How could the following code be adjusted to prevent a buffer overflow?

Question

void updateConfigParams( void ) {
    char buffer [512];
    int i = 0;

while (( c = readFromWireless ()) != NULL) 
{
    buffer [ i ] = c;
    i += 1; 
} 
writeConfigParams ( buffer );
}

I'm only getting to grips with buffer overflow so please could somebody show me how the following code needs to be changed to prevent a buffer overflow and also explain how the new code stops the writing past the end of the buffer.

Answer 1

You need to add buffer size check at start of while loop if(i == sizeof(buffer)) break;

and add \0 at the end of buffer if writeConfigParams expects a string

Answer 2

You Could add the following if Statement to check the Buffer Overflow condition. after incrementing the variable i

i+=1;
if(i > 512)
    break;

Once the i reaches the buffer limit you will exit the loop.

Answer 3

Buffer overflow is when you try to write beyond the memory address allocated for the buffer.

In your case you have allocated 512 bytes, so your code should make sure that you never de-reference beyond buffer + 511. In other words

buffer [i] // i should never exceed 511

Your code should have if checks to stop taking input once your index counter reaches equal to the size of buffer.

Answer 4

void updateConfigParams( void ) {
    char buffer [512];
    int i = 0;

    while (i < 512 && ( c = readFromWireless ()) != NULL) {
        buffer [ i++ ] = c;
    } 
    writeConfigParams ( buffer );
}

Answer 5

I would do something along the following lines:

bool
updateConfigParams( void ) {
  char buffer[512] = {};
  int i = 0;

  while( (c = readFromWireless()) != NULL ) {
    if( i == sizeof(buffer) - 1 ) {
      warn("readFromWireless exceeded %zu byte limit", sizeof(buffer));
      return false;
    } 
    buffer[i++] = c;
  }
  writeConfigParams( buffer );
  return true;
}

Depending on the state of your program, it might be more appropriate simply to call err(3). Important points are:

• When writing to an array, always ensure you're in bounds.
• When collecting input into an array, always be prepared for input that exceeds the array's size. What to do with input you can't accept is application-dependent.
• When internal storage is exceeded by an action the programmer can't prevent at compile time -- such as a wireless device sending "too much" data -- inform the user and the program. Above, the program emits a message, and the function returns an error status.
• Partial input is usually suspect and should not be accepted.

Edit: pursuant to a comment, I added initialization to buffer. Since writeConfigParams takes no length parameter, perhaps it accepts a NUL-terminated string.