Re: Sudo version 1.6.3p6 now available (fwd)

[Florian Weimer <Florian.Weimer () RUS UNI-STUTTGART DE>]

Gossi The Dog <gossi () OWNED LAB6 COM> writes:

> Sudo version 1.6.3p6 is now available (ftp sites listed at the end).
> This fixes a *buffer overflow* in sudo which is a potential security
> problem.  I don't know of any exploits that currently exist but I
> suggest that you upgrade none the less.

I don't think this buffer overflow is exploitable.  Here's why:

The buffer pointed to by 'msg' is only modified in two places,
indicated by (1) and (2) below:

    for (p = msg, count = 0; count < strlen(msg) / MAXSYSLOGLEN + 1; count++) {
        if (strlen(p) > MAXSYSLOGLEN) {
            for (tmp = p + MAXSYSLOGLEN; tmp > p && *tmp != ' '; tmp--)
                ;
            if (tmp <= p)
                tmp = p + MAXSYSLOGLEN;

            save = *tmp;
/* (1) */   *tmp = '\0';

            if (count == 0)
                SYSLOG(pri, "%8.8s : %s", user_name, p);
            else
                SYSLOG(pri, "%8.8s : (command continued) %s", user_name, p);

/* (2) */   *tmp = save;

/* (3) */   for (p = tmp; *p != ' '; p++)
                ;
        } else {
/* (4) */   if (count == 0)
                SYSLOG(pri, "%8.8s : %s", user_name, p);
            else
                SYSLOG(pri, "%8.8s : (command continued) %s", user_name, p);
        }
    }

Only during the actual syslog operation, memory is erroneously
modified.  The actual position of the modification may even be beyond
the address 'msg + MAXSYSLOGLEN' because the loop (3) may push 'p' way
after the end of the string, but it is highly unlikely that a changed
single byte there will affect the syslog operation in a way which
leads to privilege escalation.  Without a very carefully crafted
command line argument, the segfault will occur at (3), because no ' '
character is found and iteration does not stop at the terminating null
character.

BTW, even the patched version of the splitting algorithm is flawed.
The choice of a break at a space shortens the length of a piece, which
means that 'msg' string may need to be broken to more than
'strlen(msg) / MAXSYSLOGLEN + 1' pieces.  In addition, the loop (3)
skips directly to the end of the log message if not space character is
found.  As a result, the message is not completely logged.

(If you wonder if the patch actually fixes the buffer overflow
problem---I'm not sure as well ;-). However, the fix on the loop (3)
ensures that during all but the last iteration of the outer loop,
we stay inside the 'msg' buffer, but during the last iteration,
'strlen(p)' is less than MAXSYSLOGLEN, so we execute the code at (4),
which is safe.)

--
Florian Weimer                    Florian.Weimer () RUS Uni-Stuttgart DE
University of Stuttgart           http://cert.uni-stuttgart.de/
RUS-CERT                          +49-711-685-5973/fax +49-711-685-5898