errordesc.cc: Correctly append a single character to a std::string

jepler · jepler · commit 1d01b831aa30 · 2017-08-21T08:35:04.000-05:00
The idiom char c = ...; _userMsg.append( &c ); is not correct C++, because it treats the address of 'c' as a NUL- terminated C string. However, this is not guaranteed. When building and testing on Debian Stretch with AddressSanitizer: ASAN_OPTIONS="detect_leaks=false" CXX="clang++" CC=clang CXXFLAGS="-fsanitize=address" LDFLAGS="-fsanitize=address" cmake .. -DSC_ENABLE_TESTING=ON -DSC_BUILD_SCHEMAS="ifc2x3;ap214e3;ap209" ASAN_OPTIONS="detect_leaks=false" make ASAN_OPTIONS="detect_leaks=false" ctest . --output-on-failure an error like the following is encountered: ==15739==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffeb2ca7621 at pc 0x00000043c943 bp 0x7ffeb2ca75d0 sp 0x7ffeb2ca6d80 READ of size 33 at 0x7ffeb2ca7621 thread T0 #0 0x43c942 in __interceptor_strlen.part.45 (/home/jepler/src/stepcode/build/bin/lazy_sdai_ap214e3+0x43c942) #1 0x7fb9056e6143 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::append(char const*) (/usr/lib/x86_64-linux-gnu/libstdc++.so.6+0x11f143) #2 0x7fb905b677c3 in ErrorDescriptor::AppendToDetailMsg(char) /home/jepler/src/stepcode/src/clutils/errordesc.cc:150:5 Address 0x7ffeb2ca7621 is located in stack of thread T0 at offset 33 in frame #0 0x7fb905b676af in ErrorDescriptor::AppendToDetailMsg(char) /home/jepler/src/stepcode/src/clutils/errordesc.cc:149 This frame has 1 object(s): [32, 33) '' <== Memory access at offset 33 overflows this variable A similar problem with AppendToUserMsg is found by inspection. After this change, all 200 tests pass under the AddressSanitizer configuration
diff --git a/src/clutils/errordesc.cc b/src/clutils/errordesc.cc
@@ -131,7 +131,7 @@ void ErrorDescriptor::PrependToUserMsg( const char * msg ) {
 }
 
 void ErrorDescriptor::AppendToUserMsg( const char c ) {
-    _userMsg.append( &c );
+    _userMsg.push_back( c );
 }
 
 void ErrorDescriptor::AppendToUserMsg( const char * msg ) {
@@ -147,7 +147,7 @@ void ErrorDescriptor::PrependToDetailMsg( const char * msg ) {
 }
 
 void ErrorDescriptor::AppendToDetailMsg( const char c ) {
-    _detailMsg.append( &c );
+    _detailMsg.push_back( c );
 }
 
 void ErrorDescriptor::AppendToDetailMsg( const char * msg ) {

Original file line number	Diff line number	Diff line change
`@@ -131,7 +131,7 @@ void ErrorDescriptor::PrependToUserMsg( const char * msg ) {`
`131`	`131`	`}`
`132`	`132`
`133`	`133`	`void ErrorDescriptor::AppendToUserMsg( const char c ) {`
`134`		`- _userMsg.append( &c );`
	`134`	`+ _userMsg.push_back( c );`
`135`	`135`	`}`
`136`	`136`
`137`	`137`	`void ErrorDescriptor::AppendToUserMsg( const char * msg ) {`
`@@ -147,7 +147,7 @@ void ErrorDescriptor::PrependToDetailMsg( const char * msg ) {`
`147`	`147`	`}`
`148`	`148`
`149`	`149`	`void ErrorDescriptor::AppendToDetailMsg( const char c ) {`
`150`		`- _detailMsg.append( &c );`
	`150`	`+ _detailMsg.push_back( c );`
`151`	`151`	`}`
`152`	`152`
`153`	`153`	`void ErrorDescriptor::AppendToDetailMsg( const char * msg ) {`