Probably one of the worst false friends out there is the amazing Wcsncpy and all its family. Each time I see code using (and abusing) of wcsncpy a cold fear run through my whole back. Believe me, run away from it and all its family: wcsncpy(the father), strncpy(the mather), and all its sons. Whenever possible.

The worst part of wcsncpy is that it acts as one of those worst friends who are BSing in your back and you’re not noticing at all, until you begin discovering odd behaviors in your code. Even more, wcsncpy is such an amazing false friend that it probably won’t show bugs in the lines of code where it comfortly lives but, when (often) misused, will throw random bugged behaviors in any part of your program.

This wcsncpy family was sold at its time as the evolution of the wcscpy one. Did you notice the missing “n”?
The “non-n” Wcspcy, strcpy, and all its sons were letting you to copy a C string pointed by source into the array pointed by destination. However this “non-n” family were soon the main responsibles of tons of buffer overflows. This “non-n” family is blindly (stubbornish) copying from destination to source without caring at all to stop when the destination buffer is full. So if the destination array is smaller than the source array, or if you forget to end your source array with a string terminator, you will end corrupting memory sit next to your destination array memory addresses. And these cases were happening zillion of times.

wcscpy (wchar_t* destination, const wchar_t* source);

You can see how wcscpy was receiving just the start address of source and destination as params so it doesn’t have a way to know when it is overflowing the destination array. It just stops copying when it finds the Null terminator in the source buffer (good luck if you forgot to append to it)

So if the problem is that the function doesn’t know when to stop, why not creating a family of functions which receives an extra param “n-um” telling how many characters must be written to the destination buffer? Wow! Amazing idea! So the wcsncpy family was created.

wcsncpy (wchar_t* destination, const wchar_t* source, size_t num);

 

And then Developers were starting to feel safer! Long life to Wcsncpy(they said)! It’s our savor!
You can see in tons of threads recommending you to use the “n” family because it is much safer than the “non-n” one.

Then why? Why this “n” family was introducing much more corruptions?

First because the “non-n” one was pretty well-known to be an overflow lover so each time it was being used, developers were being really cautious and double checking. However when they started to use the “n” family they got much more relaxed, believing in the new “num” param as their own Super-num-an, but it was in a galaxy far-far away from that.

On the other hand Num was, and is, totally misinterpreted by developers.
Some thought that it was the maximum Num-ber of characters to be copied from source to destiny believing also that “wcsncpy” will stop when the destination buffer is full (magically maybe?). But nope, NUM is the Number of characters to be copied (ignoring the length of the source and destiny). And the stubborn “n” function will really copy NUM characters to destination, even if that means overflowing it. Next line, used by these kind of developers, will corrupt memory by overflowing destination, if destination is a 20 wchar array.

wcsncpy (destination, source, MAX_PATH);

Let me highlight one hipercommun pitfall. The old “non-n” family was using the Null terminator of the source string to STOP copying. So when the “n” family was introduced a lot of devs thought that the “num” param was explicitly defining the number of characters BUT that if the Null terminator was found before copying such “num” characters the function will inmediatly stop copying there. Not the case of the “n-family”.

Some others thought that the Num param was the size of the destination but in bytes. So if destination is a 20 wchar array the following line will corrupt their memory due overflow ( sizeo(WCHAR) * 20 = 40): .

wcsncpy (destination, source, sizeof(WCHAR) * 20);

Sometimes the corruption is even more difficult to track, something like:


wcsncpy (destination, source, SOURCESIZE);

Here it is pretty obvious, but if you name the variable SOURCESIZE as SIZE, you will have a nice corruption in front of your eyes (if source is bigger than destination), and you not noticing it at all.

And finally, copypasta, since the name is tedious to write, and there are 3 params being received copy and pasting is a new way to introduce bugs.

wcsncpy (destination, source, numdestination);
wcsncpy (destination2, source2, numdestination);
wcsncpy (destination3, source3, numdestination);

Oops, he forgot to modify the last param. Potential overflows there.

This patch I made for ReactOS shows you how easy you can fall in the trap, when the num param (Usize) is being calculated in the parent function and then thrown to the helper function. Our Coverity  scans were an amazing help to find this flaw.

So if you used this lovely function, please check twice. Your non-corrupted memory deserves it 😉

 

There are currently no comments.
www.000webhost.com