This tweet trolls me this XmasEve, on many difference levels.
What we see here is a classic buffer overflow, where an input string is indiscriminately copied into a buffer that may not be big enough to hold all the input. It's even more "classic" in that it's part of a debug/log statement, where most such bugs lurk.
So the fix is to check the length fo the destination buffer. One option is "strncpy()". It's the wrong solution. It'll prevent the buffer from overflowing, but doesn't handle the error correctly. It'll leave the string without a nul-terminator, which can lead to other vulns.
The 'strncpy()' is often chosen because it's part of the C standard library and works everywhere. So even though it a poor solution, it at least improves the situation and doesn't require fancy portability problems.
This problem has been known for decades -- time enough to add a correct solution to the C standard library. So why wasn't this done? Well, it was, sorta, but foundered on the rock of politics.
The C11 standard included fixes, in this case, a function "strcpy_s()" as a good replacement that does a length check and properly NUL terminates the string on error. However, it wasn't only defined as an OPTIONAL feature, in Annex K, rather than mandatory.
That's fine, even though it's optional, most people would've adopted it.
Except it came from Microsoft. This made it politically toxic. So Unix libraries, especially glibc at the heart of Systemd/Linux, have refused to adopt it.
Except it came from Microsoft. This made it politically toxic. So Unix libraries, especially glibc at the heart of Systemd/Linux, have refused to adopt it.
Instead, in the non-Microsoft world, "strlcpy()" has become the standard that everyone seems to support. Unfortunately, Microsoft is just as much a jerk about the whole thing and refuses to support strlcpy(). Microsoft prefers C++, and refuses to do much of anything with their C.
But there's yet a third solution, -D_FORTIFY_SOURCE. This tells the gcc/clang compilers to automatically add such checks, without changing the source code in the first place.
It's not perfect, but it would've worked in this case. If you can calculate the length of sizeof(SaveFileName), then the compiler can also detect the length of the buffer automatically, and will automatically insert a replacement function here that checks the length.
Thus, the original bug shouldn't even have been a bug, because the source code should've been compiled with FORTIFY_SOURCE to begin with. There's really no excuse NOT to compiler your code with that option in the modern world.
As for C++, that's also a solution to security problems. It comes with proper container classes to hold things that includes bounds checking, so you get a lot fewer buffer overflow bugs in C++ than C. This is a big reason why Microsoft prefers C++.
Safety is by far the thing that needs most to be "fixed" in C. Those doing "systems" programming are abandoning it for Rust for this reason. I don't like Rust, it's going the way of C++ by constantly "improving" the language out of control.
