Re: strcpy versus strncpy

[pedward () WEBCOM COM (pedward () WEBCOM COM)]

> Recently on the vcpp () 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.

I would never use a static buffer.

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

Yup! ;)

Here is how I do it:

int     blahprintf(char *buffer, int len, char *format, ...)
{
...

        i = vsnprintf(buffer,len,format,msg);

        if(i < 0) ret_code = FALSE;
        else buffer[i] = '\0';

...

        return ret_code;
}

In no circumstances should you EVER process data into a buffer without bounds
checking, that's why Sendmail horks so badly and there are so many crappy unix
programs out there.  If it's one thing that all new Unix programmers should learn:
Check your bounds...let us not repeat the mistakes of our predecessors (respectfully
of course).

> 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

I have successfully developed a mail server that doesn't suffer from these
problems, other people can too.

> 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.

RFC 821 defines line lengths to be 1024 bytes:

char buffer[RFC_LENGTH];

...

fgets(buffer,RFC_LENGTH,stream);

That's how my mail server does it.  Convenience is really a poor excuse for
security hole ridden software.  If it takes you 2 minutes longer to write
code that checks bounds exactly and allocates appropriately, that's 2 minutes
of time that works out to be worth over thousands of minutes in the future,
especially if the program is in wide use.

Take for example what would happen if you had a mail server with exploitables:

Hacker spends 4 hours cracking at the machine, causes a few core dumps, which
have to be cleaned up later via a Cron job.  He then gets entry, causing a
sysadmin to be paged or possibly not.  Next he goes around poking at files,
steals the passwd file (or shadow), uploads some more cracks and backdoors.
Next day the sysadmin comes in and has to change all the passwds, cleanup the
files, spend 8 hours examining every little audit trail, examining /usr/bin
and others for new executables.

Your 2 minutes saved over 720 minuts of the sysadmin's time, assuming that it
was an innocuous breakin!

--Perry

> --
> Daniel Reed <n () 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...


--
Perry Harrington        System Software Engineer    zelur xuniL  ()
http://www.webcom.com  perry.harrington () webcom com  Think Blue.  /\