Can this code contain buffer overflows?

Question

These days I have read about buffer overflow attacks, and actually I can't say that I have understand the big picture, I have some doubts in my mind.

So to kill my doubts the question arises, if my program is written in C and all of code used to get input or to copy/merge buffers, checks for bounds, can buffer overflow occur? Or saying directly, is input (wherever that comes) the only method that an attacker can use to cause buffer overflow?

For example, consider the following code:

int main(){
    int size = 15;
    char buf[size];
    fgets(buf, size , stdin);
    printf("%s",buf);}

Is susceptible to buffer overflows? Thank you!:)

Answer 1

Actually guys there is an error in the code, and there could be a potential security problem, coding like that in certain applications! In short checking returns values matter.

Whilst it may be argued that his program is indeed safe, the bigger picture is about the pattern on the code, and ensuring the assumed invariants of the code, which is that buf, contains a NULL terminated string between 0 and 14 bytes long.

From man page :

The fgets() function shall read bytes from stream into the array pointed to by s, until n-1 bytes are read, or a is read and transferred to s, or an end-of-file condition is encountered. The string is then terminated with a null byte.

RETURN VALUE

Upon successful completion, fgets() shall return s. If the stream is at end-of-file, the end-of-file indicator for the stream shall be set and fgets() shall return a null pointer. If a read error occurs, the error indicator for the stream shall be set, fgets() shall return a null pointer, [CX] [Option Start] and shall set errno to indicate the error.

Arranging for an error condition, may mean no NULL may be appended to the string and the buffer is automatically allocated, so printf(3) may leak information.. think about Heardbleed.

As chux points out initialising the automatically allocated buffer buf[0] = '\0';, or declaring buf statically so it's system initialised to 0, ought not be relied upon as in event of error, the state of buf is undefined.

So a check on the return value of fgets is necessary. So something more like :

{ 
  char *s;
  if ((s = fgets( buf, sizeof buf, stdin)) {
      puts( s);
  }
}

Here's a link to an article on secure programming, which may be of interest http://www.dwheeler.com/secure-programs/