While that's indeed a bug, for it to be a security vulnerability, wouldn't there also have to be a security boundary involved? Specifically, mapName is always either "w1000b.png" or "w1000.png", so the only way to trigger the buffer overflow would be through the HOME environment variable. But if an attacker can run commands as you with arbitrary environment variables, aren't you already pwned? What would anyone gain by running your program and exploiting it to do something, rather than just doing the thing directly? https://devblogs.microsoft.com/oldnewthing/20060508-22/?p=31...
Assuming that code is actually present in your app, env vars can hold more than 255 characters. Easy buffer overflow to trigger. Use length-bounded copies and concats...
That's just off the top of my head; I've not written in C in a while.
Yeah, that is not a helpful attitude to take when it comes to this sort of thing. If nothing else, a super-long home path can crash your app and leave your user scratching their head. In other words, this is a bug (as is the fact that paths are not necessarily limited to 255 characters in the first place; see the PATH_MAX constant, I think it is?).
As to what could be accomplished with an overflow? I don't know; I'm not in security, and I don't sit around thinking of possible uses for various bugs when it comes to compromising systems.
Perhaps the most important thing to realize, though, is that you're distributing software publicly. Your security situation may not be the same as your user's security situation. Assumptions should not be made.
As long as you’re fixing that bug, you should do it right. If the return value from snprintf if more than 256 but less than a few GB then you should malloc a buffer big enough to hold the string and then call snprintf again with the new buffer. Only if that or malloc fails would you print an error. (It’s really a shame that the C standard library requires so many extra steps to do things correctly; this ought to be way easier.)
Basically, any path longer than 256 characters for `mapFilename` would cause a buffer overrun.
An unprivileged app could run your app (say, with more privileges), with a very long `HOME` environment path, causing a buffer overflow, and potentially exploit it to use your app's privileges to do more stuff than it was supposed to.
Basically, you should never use strcpy and strcat and but use the secure alternatives like strcpy_s and strcat_s, even when you know the source buffer would never exceed the destination size.
Sure, but since there's no enforced standard for how privileges are configured on a system, there's always the possibility that your app to be the only escape ticket.
You can dismiss that possibility of course. But, as a general habit, it's best to use secure alternatives instead of mulling over probabilities every other line.
As a positive side-effect, the change would make your app not crash on systems with long HOME env paths.:)
I see you already addressed it but here let me give a scenario.
Say the program was installed and set so the user didn't have privs to modify the executable (so an attacker couldn't just change it to do what they want).
A buffer overflow could allow an attacker to gain control flow of the program and feed bogus data to the user allowing them to scrub their presence from the map.