Replacing strcpy with strncpy

Question

Let's say I have some legacy code which was written using unsafe calls to C STL functions like strcpy. We all know that strcpy is unsafe because it leaves a program vulnerable to buffer overflow issues. Let's say I want to replace all calls to strcpy with calls to strncpy. A technique for replacing all calls to strcpy(dest, src) would involve calling strncpy with the parameters (dest, src, length of dest - 1) and then terminating dest with \0. I know the issue with this is that we don't always know the length of dest because it may be a pointer to memory allocated on the heap.

Let's assume I can figure out the length of dest at each of these call sites. I could replace all calls to strcpy with calls to strncpy which will guarantee that my program is immune to buffer overflow attacks (at least from improper use of strcpy). However, this approach may silently truncate data and alter program behavior in an undesirable way. Is this a better approach than detecting truncation and aborting the program? Or is it better to allow the truncation but also log it?

I'm asking from the perspective of someone who is interested in developing an automated method for patching legacy code. Does anyone have any thoughts on how to best approach this issue?

Answer 1

We all know that strcpy is unsafe because it leaves a program vulnerable to buffer overflow issues.

This is not a fault of strcpy in the slightest: it is up to the programmers to ensure that the string is going to fit into their buffer, for example, by calling strlen before copying, or ensuring that the string that comes in cannot possibly be longer than their buffer.

Let's say I want to replace all calls to strcpy with calls to strncpy

You shouldn't do that, unless you are working with fixed-size strings: remember that strncpy not only copies up to the terminating null, but also fills the rest of the string with null bytes. If you are looking for a "modern replacement" of strcpy, consider using strlcpy instead.

This approach may silently truncate data and alter program behavior in an undesirable way. Is this a better approach than detecting truncation and aborting the program? Or is it better to allow the truncation but also log it?

This is entirely up to you. It depends on the place in your design where such truncation would happen: if it happens in the code that sends authentication information to a web service, you would be better off stopping the process right then; if it happens in the code that writes a trace message into a log, it is probably OK to ignore the issue, or log it and go on. Sadly, you cannot decide it automatically, because a certain level of program understanding is required.