Compare commits

...

5 commits

Author SHA1 Message Date
Jeff Epler
1d01b831aa errordesc.cc: Correctly append a single character to a std::string
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
2017-08-21 08:35:04 -05:00
Jeff Epler
0d2e791e82 express/error.c: Ensure the error buffer does not overflow
On Debian Stretch, when configuring stepcode like so:
    ASAN_OPTIONS="detect_leaks=false" CXX="clang++" CXXFLAGS="-fsanitize=address" cmake ..
a fatal error would be detected:

  ==29661==ERROR: AddressSanitizer: heap-buffer-overflow on address
  0x62100001dca0 at pc 0x0000004435e3 bp 0x7ffed6d9cae0 sp 0x7ffed6d9c290

  READ of size 4001 at 0x62100001dca0 thread T0

      #0 0x4435e2 in __interceptor_strlen.part.45 (/home/jepler/src/stepcode/build/bin/schema_scanner+0x4435e2)
      #1 0x501d7b in ERRORreport_with_symbol /home/jepler/src/stepcode/src/express/error.c:413

  0x62100001dca0 is located 0 bytes to the right of 4000-byte region
  [0x62100001cd00,0x62100001dca0)

  allocated by thread T0 here:

      #0 0x4c3ae8 in __interceptor_malloc (/home/jepler/src/stepcode/build/bin/schema_scanner+0x4c3ae8)
      #1 0x5011fc in ERRORinitialize /home/jepler/src/stepcode/src/express/error.c:129

Operations on ERROR_string were unsafe, because they did not guard
against accesses beyond the end of the allocatd region.

This patch ensures that all accesses via *printf functions do respect
the end of the buffer; and encapsulates the routine for pointing
ERROR_string at the space for the next error text to start, if space is
available.

Finally, because it was found with search and replace, a stray manipulation
of ERROR_string within the print-to-file branch of the code is removed.
This stray line would have had the effect of moving ERROR_string one byte
further along at every warning-to-file, which could also have been a
cause of the problem here.
2017-08-21 08:35:04 -05:00
Mark
71fe947ff5 Merge pull request #361 from jepler/appveyor-single-thread-test
appveyor build: don't use ctest parallelism
2017-08-19 17:12:28 -04:00
Mark
beb2a595f1 Merge pull request #357 from jepler/nullptr-bool
Fix build error with g++ 6.3 (Debian Stretch)
2017-08-19 15:41:33 -04:00
Jeff Epler
0fbc3c0c84 Fix build error with g++ 6.3 (Debian Stretch)
On this platform, TEST_NULLPTR fails, even though nullptr and
nullptr_t are supported:

/home/jepler/src/stepcode/build/CMakeFiles/CMakeTmp/src.cxx:4:23:
    error: converting to 'bool' from 'std::nullptr_t'
    requires direct-initialization [-fpermissive]
 int main() {return !!f();}
                      ~^~

Subsequent to this failure, the workaround definitions in sc_nullptr.h
prevent standard C++ headers (which must refer to real nullptr) to fail.

The failure occurs because the C++ standard apparently does not state
that operator! may be used on nullptr.  Despite this, some compilers
have historically allowed it.  g++ 6.3's behavior appears to be aligned
with the standard.

As requested by @brlcad, ensure that the function 'f' is used from main,
to avoid a clever (but not nullptr-supporting) compiler from somehow
skipping 'f' altogether, creating a false positive for nullptr support.
2017-08-15 06:50:56 -05:00
3 changed files with 42 additions and 16 deletions

View file

@ -80,7 +80,7 @@ std::cout << \"1s is \"<< std::chrono::duration_cast<std::chrono::milliseconds>(
set( TEST_NULLPTR "
#include <cstddef>
std::nullptr_t f() {return nullptr;}
int main() {return !!f();}
int main() {return !(f() == f());}
" )
cmake_push_check_state()
if( UNIX )

View file

@ -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 ) {

View file

@ -67,6 +67,9 @@
#include "express/info.h"
#include "express/linklist.h"
#if defined( _WIN32 ) || defined ( __WIN32__ )
# define snprintf _snprintf
#endif
bool __ERROR_buffer_errors = false;
const char * current_filename = "stdin";
@ -112,6 +115,7 @@ static struct heap_element {
static int ERROR_with_lines = 0; /**< number of warnings & errors that have occurred with a line number */
static char * ERROR_string;
static char * ERROR_string_base;
static char * ERROR_string_end;
static bool ERROR_unsafe = false;
static jmp_buf ERROR_safe_env;
@ -119,6 +123,34 @@ static jmp_buf ERROR_safe_env;
#define error_file stderr /**< message buffer file */
static int ERROR_vprintf( const char *format, va_list ap ) {
int result = snprintf( ERROR_string, ERROR_string_end - ERROR_string, format, ap );
if(result < 0) {
ERROR_string = ERROR_string_end;
} else if(result > (ERROR_string_end - ERROR_string)) {
ERROR_string = ERROR_string_end;
} else {
ERROR_string = ERROR_string + result;
}
return result;
}
static int ERROR_printf( const char *format, ... ) {
int result;
va_list ap;
va_start( ap, format );
result = ERROR_vprintf( format, ap );
va_end( ap );
return result;
}
static void ERROR_nexterror() {
if( ERROR_string == ERROR_string_end ) {
return;
}
ERROR_string++;
}
/** Initialize the Error module */
void ERRORinitialize( void ) {
ERROR_subordinate_failed =
@ -127,6 +159,7 @@ void ERRORinitialize( void ) {
ERRORcreate( "%s, expecting %s in %s %s", SEVERITY_EXIT );
ERROR_string_base = ( char * )sc_malloc( ERROR_MAX_SPACE );
ERROR_string_end = ERROR_string_base + ERROR_MAX_SPACE;
ERROR_start_message_buffer();
@ -377,20 +410,14 @@ va_dcl {
heap[child].msg = ERROR_string;
if( what->severity >= SEVERITY_ERROR ) {
sprintf( ERROR_string, "%s:%d: --ERROR PE%03d: ", sym->filename, sym->line, what->serial );
ERROR_string += strlen( ERROR_string );
vsprintf( ERROR_string, what->message, args );
ERROR_string += strlen( ERROR_string );
*ERROR_string++ = '\n';
*ERROR_string++ = '\0';
ERROR_printf( "%s:%d: --ERROR PE%03d: ", sym->filename, sym->line, what->serial );
ERROR_vprintf( what->message, args );
ERROR_nexterror();
ERRORoccurred = true;
} else {
sprintf( ERROR_string, "%s:%d: WARNING PW%03d: ", sym->filename, sym->line, what->serial );
ERROR_string += strlen( ERROR_string );
vsprintf( ERROR_string, what->message, args );
ERROR_string += strlen( ERROR_string );
*ERROR_string++ = '\n';
*ERROR_string++ = '\0';
ERROR_printf( "%s:%d: WARNING PW%03d: ", sym->filename, sym->line, what->serial );
ERROR_vprintf( what->message, args );
ERROR_nexterror();
}
if( what->severity >= SEVERITY_EXIT ||
ERROR_string + ERROR_MAX_STRLEN > ERROR_string_base + ERROR_MAX_SPACE ||
@ -410,7 +437,6 @@ va_dcl {
ERRORoccurred = true;
} else {
fprintf( error_file, "%s:%d: WARNING PW%03d: ", sym->filename, sym->line, what->serial );
ERROR_string += strlen( ERROR_string ) + 1;
vfprintf( error_file, what->message, args );
fprintf( error_file, "\n" );
}