Rita caught a weird bug. It was weird, in part, because there hadn’t been any code changes in their label printing application for ages. Yet, there was a sudden new bug. Labels were printing with what was obviously unicode garbage. Interestingly, the application definitely supported unicode- there had been a huge effort a few years back to move the C++ code from char
s to wchar
s.
Rita started debugging, and confirmed that when the label text was populated, memory stored correct values. By the time the data was printed, it no longer did. Obviously, there was something wrong with memory management- something was touching the end of the string and throwing off the output. That was an easy enough bug to make in C++, but tracing through 7,000 lines of label printing code to figure out where things got thrown off was more of a chore, especially with the… “friendly” variable naming convention the original developer had used.
wchar_t* xyz = new wchar_t(wcslen(abc));
That doesn’t look like much, does it? xyz
is a pointer to a wchar_t
, a pretty typical way to represent a string, and then we initialize it to point to a new wchar_t
with a size equivalent to the length of an input string, right?
Of course not. There’s an important difference between new wchar_t(10)
and new wchar_t[10]
. The latter makes a block of memory capable of holding ten wide characters. The former makes a single wide character and initializes it with the value 10
. So xyz
points to a single character, but this is C++. If you disable enough warnings, there’s nothing that will stop you from abusing that pointer and running off the end of memory.
The real WTF though, is that this wasn’t a bug introduced back when they switched to wchar
. Going back through the source control history, Rita found the previous version:
char* xyz = new char(strlen(abc));
It had the same bug. The bug had gone undetected for all this time because the message string had been short enough that nothing else was touching that section of memory. It was only after a user had recently tried to add a slightly longer message that the buffer overrun started causing problems.