Re: strcpy versus strncpy

On Tue, 3 Mar 1998, Morten Welinder wrote:
) A recent article on BugTraq suggested that using strcpy should
) almost always be considered a bug. That's not right. It is,
) in fact, the wrong way around: strncpy is almost always a bug.
)
) True, strncpy will avoid buffer overruns, but that only proven
) that strncpy is better than incorrect use of strcpy. The problem
) is that such use of strncpy can introduce problems of its own:
 [...]
) With dynamic allocating available, there really is no execuse for
) using strncpy, with the possible exception where memory attacks
) might be a larger problem, but that should not be the case with
) argv/environ based strings.
Recently on the vcpp_at_narnia.mhv.net mailing list someone asked about
creating their own function similar to printf(), to which I gave the
minimal example:

#include <stdarg.h> // for the va_* stuff
#include <stdio.h> // for fputs() and stdout
#include <string.h> // for bzero()

#define BUFSIZE 1024 // how big is our internal buffer
#define BUFLEN (BUFSIZE-1) // how many bytes of our internal buffer
                                // can we use (BUFSIZE - 1 for the NULL
                                // terminator)

char *MySprintf(char *format, ...) {
        static char buf[BUFSIZE];
                                // this is our buffer to store the
                                // formatted string into
        va_list msg; // this is how we access the ...

        bzero(buf, BUFSIZE); // clear out our buffer
        va_start(msg, format); // then attach the ... with the format
        vsnprintf(buf, BUFLEN, format, msg);
                                // then stick the formatted string into
                                // buf (but only use BUFLEN bytes of buf,
                                // to avoid a buffer overflow)
        va_end(msg); // clean up
        return(buf); // send back a pointer to our static buffer
}

I made a note that this wasn't multi-thread safe, as calling MySprintf()
again would overwrite the static buffer for MySprintf().

If I had made this use dynamic memory, instead of a static internal
buffer, the user would then have to deal with free()'ing a section of
memory they did not allocate--my function did! If nothing else, that's
poor coding style (in my opinion), and at worse, leads to hard-to-trace
memory leaks.

So, discounting dynamic memory allocation, could you fault me for shunning
vsprintf() and instead using vsnprintf()?

 ...

int main(int argc, char **args) {
        printf("Shell login wrapper loading.\n");
        printf("\n");
        if (getuid() == 0)
                execl(MySprintf("%s.root", getenv("SHELL")), args[0], NULL);
        /* set user limits and stuff */
        execl(MySprintf("%s.user", getenv("SHELL")), args[0], NULL);
}

as a trivial example. As a not so trivial example, think of something like
sendmail, which runs forever, and uses a lot of automatic buffers. Running
as root, having a static buffer and using strcpy/sprintf/vsprintf, buffer
overflows are possibly exploitable. As any user, having a dynamic buffer
and using anything, memory starvation (or CPU starvation, in fact;
malloc() is an expensive call) is possible. Under any user, using a static
buffer with strncpy/snprintf/vsnprintf, buffer overflows are significantly
reduced (if not eliminated), resource starvation is significantly reduced
(if not eliminated), and at worse an incoming, legitimate message will be
bounced because it overflows a buffer. I believe in [one of] the SMTP
RFC[s] a maximum line length is defined for commands.

--
Daniel Reed <n_at_narnia.n.ml.org> (3CE060DD)
System administrator of narnia.n.ml.org (narnia.mhv.net [199.0.0.118])
I'm so glad to see you! I've run out of people to torment...