global buffer overflow, Windows environment

Question

I've asked a question about buffer overflow detection few days ago ( sprintf buffer global data overflow - how to detect it, Windows ) and problem can by only solved by cppcheck with standard function ( not secure _s version ).

I went deeper and changed code from

#include <stdio.h>
char buffer[2];
void main()
{
  sprintf(buffer,"12345");
}

to

#include <stdio.h>
void f( char *b )
{
   sprintf(b,"12345");
}

char buffer[2];
void main()
{
   f( buffer );
}

Visual studio 2012 /RTC can handle stack allocated buffer overflow - during runtime, but global data stays undetected.

I guess it is not possible to make deep analysis using cppcheck and this problem is not detected by cppcheck-1.64. Additionally I have tried to use clang with AddressSanitizer ( Windows ) also without good results.

Is is possible to prevent such problems under Windows ( free tool preferably ), if not maybe some linux tool can help?

Answer 1

The answer is a bit late i know, but it could still help in similar cases.
Cppcheck is constantly improved and now able to detect this issue.
The latest version of Cppcheck (version 1.86 at the moment) outputs this error message when the second code example is analyzed:

$ ./cppcheck global_buffer_overflow.c
Checking global_buffer_overflow.c ...
[global_buffer_overflow.c:10] -> [global_buffer_overflow.c:4]: (error) Buffer is accessed out of bounds: buffer

I am not sure if it already worked in version 1.85, but it definitely does not work with 1.84 or older versions. I guess if you hide the global buffer even better Cppcheck is eventually no longer able to detect the issue. Value flow analysis is somewhat complex and needs some resources (memory, time, CPU).

Answer 2

Instead of using sprintf, you should consider using snprintf. Its prototype is (contained in the header stdio.h) -

int snprintf(char *str, size_t size, const char *format, ...);

The function writes at most size bytes including the terminating null byte to the buffer pointed to by str. Therefore, you should change your function f signature to take the length of the buffer as well. Also, note that the signature of main should be one of the following -

int main(void);
int main(int argc, char *argv[]);

I suggest the following changes -

#include <stdio.h>

void f(char *b, size_t len) {
   sprintf(b, len, "12345");
}

char buffer[2];
int main(void) {
   f(buffer, sizeof buffer);
   return 0;
}

Answer 3

These are the some of reasons due to which we should avoid to global variable in our program. I do not think there are such tools exists which would report about the global variable(data segment) corruption. There is good set of tools available for detecting the stack and heap segment memory corruption/overrun.

So to avoid such scenario, we should use minimize the usage of global variable in our program. If it is not possible, we should try to use defensive programming approach by yourself while using these. Just to illustrate this your program can be re-written in the following way which would avoid the global memory corruption scenario in your program.

#include <cstdio>

char buffer[2];


void f( char *b, size_t sz)
{
 // Now we have protected our global variable from overrun by
 // using the size information passed by caller.So even though
 // client "12345" has been passed, it would just copy 12.
 strncpy(b,"12345",sz);
}


int main() {
  size_t tmp = sizeof(buffer)/sizeof(buffer[0]);
  f( buffer, tmp);
  return 0;
}