fs2open.github.com icon indicating copy to clipboard operation
fs2open.github.com copied to clipboard

convert strncpy to strncpy_s

Open Goober5000 opened this issue 2 years ago • 1 comments

I have gone through the codebase and replaced most instances of strncpy with strncpy_s which always adds a null-terminator and includes some extra error checking. Quite a few of these places did not use the length properly and have been fixed.

This is based on #4321 but without the safe_strings removal.

Goober5000 avatar Sep 24 '22 00:09 Goober5000

I'd like to point out that strncpy_s, on an error such as truncation, will produce an empty dest string. This can give unpredictable, and possibly dangerous, results if such errors aren't accounted for. For something like cfile it could be dangerous to have path construction suddenly reset in the middle of the process and end up trying to act on a very unexpected path string.

strncpy_s is safer for sure, but it's not really a drop-in replacement for strncpy. Some of these places will require the addition of extra error handling or conversion to SCP_string instead.

Also to add, I am in the process of slowing converting much of the PXO related code to use SCP_string wherever possible and SDL_strlcpy() where not since it's safer than a straight strncpy but will still produce a valid, even if truncated, string. Not to tell you to avoid changes to that code, just pointing out how I'm trying to handle a similar conversion to safer strings.

notimaginative avatar Sep 26 '22 01:09 notimaginative