[Mauiusers] Big bug in latest maui (and all previous ones too probably)

Åke Sandgren ake.sandgren at hpc2n.umu.se
Thu Sep 15 05:19:39 MDT 2005


On Thu, 2005-09-15 at 09:10 +0200, Åke Sandgren wrote:
> Hi!
> 
> We just got hit by a bad buffer overflow bug in the UI part of the code.
> One user submitted 2300 jobs and when doing showq the server crashes.
> 
> The problems is a buffer overwrite causing stack thrashing emanating
> from UIProcessCommand. The local SBuffer there gets too much data
> overwriting the inparameter msocket_t *S.


As a side effect of fixing this (replacing sprintf's for snprintf's)
I discovered a nasty "bug" between the two.
sprintf and snprintf doesn't behave identically in glibc when the dest
is also used as source.

The following short example shows what i mean.
#include <stdio.h>

main()
{
    char buf[1000];
    char *p, *b;
    int s, r, a;

    b = "ARG=";

    printf("All three examples start by setting buf to \"SC=0 \"\n");

    printf("Using sprintf(buf, \"%%s%%*s%%s\", buf, a, \" \", b)\n");

    memset(buf, 0, sizeof(buf));
    r = sprintf(buf, "SC=0 ");

    s = strlen(buf) + strlen(b);
    a = 16 - (s % 16);
    r = sprintf(buf, "%s%*s%s", buf, a, " ", b);
    printf("buf = \"%s\"\n", buf);


    printf("Using snprintf(buf, size, \"%%s%%*s%%s\", buf, a, \" \",
b)\n");

    memset(buf, 0, sizeof(buf));
    r = sprintf(buf, "SC=0 ");

    s = strlen(buf) + strlen(b);
    a = 16 - (s % 16);
    r = snprintf(buf, sizeof(buf)-strlen(buf)-1, "%s%*s%s", buf, a, " ",
b);
    printf("buf = \"%s\"\n", buf);



    printf("Using snprintf(buf, size, \"%%s%%7s%%s\", buf, \" \",
b)\n");

    memset(buf, 0, sizeof(buf));
    r = sprintf(buf, "SC=0 ");

    s = strlen(buf) + strlen(b);
    r = snprintf(buf, sizeof(buf)-strlen(buf)-1, "%s%7s%s", buf, " ",
b);
    printf("buf = \"%s\"\n", buf);
}



On AIX with xlc or SUN with suncc the same result is obtained for all
cases (i.e. the expected one) but with gcc (any version) 2 different
results are obtained.


The conclusion is that you should change ALL
s(n)printf(buffer, "...", buffer, args)
to
s(n)printf(&buffer[strlen(buffer)], "...", arg2)
since it isn't defined what should really happen here and sprintf might
also break sometime in the future...

And you should also change all sprintf(buffer... to snprintf's.
There are plenty of buffer overruns luring in the maui code right now.


More information about the mauiusers mailing list