Why is there a buffer overflow in this code?

Question

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?"

There are three errors in the above fragment. The only line without an error is the line with } — wildplasser
– wildplasser, Commented May 25, 2014 at 17:32
1) strncpy() is always wrong. 2) the > compare is wrong (off by one), the currentItem.filename[sizeof(currentItem.filename)] = '\0'; is wrong too (off by one) — wildplasser
– wildplasser, Commented May 25, 2014 at 17:35

templatetypedef · Accepted Answer · 2014-05-25 17:42:49Z

2

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!

answered May 25, 2014 at 17:42

templatetypedef

376k112 gold badges954 silver badges1.1k bronze badges

Sign up to request clarification or add additional context in comments.

Collectives™ on Stack Overflow

Why is there a buffer overflow in this code?

1 Answer 1

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

Comments

Your Answer

Sign up or log in

Post as a guest

Related