CODEWITHSUNDEEP
cbuffer-overflow
CodingHater
CodingHater

Reputation: 51

Why is there a buffer overflow in this code?

As part of a problem, I have a struct which contains char isGood and char filename[32]

There is a function that checks if a file contains the string I'm a virus and return the value in isGood.

Then there is the following code:

strncpy(currentItem.filename, argv[i], sizeof(currentItem.filename));
if (strlen(argv[i]) > sizeof(currentItem.filename)) {
    currentItem.filename[sizeof(currentItem.filename)] = '\0';
}

This code causes an error that a file with a name larger than 32 characters will always return "OK" even when the file contains the string I'm a virus.

Why does this happen? Why changing the currentItem.filename changes the value of currentItem.isGood?? It seems to be related to the strncpy because the next question I have to answer says "What is the proper way of ending a string copied with strncpy?"

Upvotes: 0

Views: 129

Answers (1)

templatetypedef
templatetypedef

Reputation: 373082

This line of code is your buffer overflow:

currentItem.filename[sizeof(currentItem.filename)] = '\0';

The value of sizeof(currentItem.filename) is the length of the buffer. If you write at that index, you're writing one spot past the end of the array, causing the overflow.

To fix this, write

currentItem.filename[sizeof(currentItem.filename) - 1] = '\0';

More generally, rather than using strncpy, you may want to use strlcpy, a compiler extension that most compilers support. It's like strncpy, except that it always puts in a null terminator. This means that you never have to worry about adding your own terminator instead.

Hope this helps!

Upvotes: 2

Related Questions