morgant / mlvwm

Macintosh-like Virtual Window Manager (official repo)
http://www2u.biglobe.ne.jp/~y-miyata/mlvwm.html
275 stars 8 forks source link

Replace sprintf with snprintf #6

Closed morgant closed 3 years ago

morgant commented 3 years ago

Much of the code still uses sprintf(), but should use snprintf() instead to prevent potential buffer overflows.

morgant commented 3 years ago

I have converted all the sprintf() calls that used string formats to snprintf() calls. I didn't convert any of the calls using numerical formats as it was explained to me that there wasn't a buffer overrun issue with those (please correct me if wrong).

I also see a couple strcat() calls that should probably be replaced with snprintf() calls.

morgant commented 3 years ago

I was tempted to replace the strcat() calls with strlcat() from OpenBSD, but I didn't want to include another dependency on non-OpenBSD platforms, so went with snprintf(). This is how fvwm on OpenBSD does it and mlvwm was originally based on fvwm, so it feels right.

Further discussion of the options, including OpenBSD's strlcat() can be found in Efficient string copying and concatenation in C.

morgant commented 3 years ago

I haven't run into any issues with my — admittedly light — testing so far, so l merged in these changes.