Potential security problem with mtr

From: Viktor Fougstedt (viktorat_private)
Date: Fri Mar 03 2000 - 07:42:24 PST

  • Next message: Joel Klecker: "Re: [XFree86 3.3.6] fix for race conditions in xterm logfile"

    Hi.
    
    One of my users asked me to install mtr, most adequately described as
    a GUI:ed combination of traceroute and ping. I thought it looked cool,
    and had a closer look.
    
    In this mail follows a warning about a potential security problem with
    this program if installed as suggested. No exploit has been confirmed
    to exist, but deficiencies in the code make one trivial to write under
    certain circumstances.
    
    Affected: All known versions of mtr installed setuid-root, on most
    UNIX-dialects except HPUX.
    
    mtr-0.28 seems to be a standard package in some Linux distributions,
    but it is not known whether it is installed setuid-root.
    
    Outline: Since mtr must open a raw socket, it needs root privileges,
    and must be setuid-root if ordinary users are going to be able to use
    it. Contrary to what the mtr documentation claims, mtr does not
    properly drop root privileges. If it is installed setuid-root, which
    the documentation suggests, the entire program effectively runs as
    root, including all parts of Gtk, Gdk, glib and curses that the
    program uses.
    
    The authors have been contacted, but no reply has been received. The
    latest version is from Aug 19 1999, and I am uncertain whether mtr is
    still being actively developed.
    
    Recommendation: Do not run mtr setuid-root until patched. A patch is
    supplied at the bottom of this message.
    
    Details:
    
    According to the file called SECURITY in mtr's source distribution,
    mtr opens a raw socket pair, and then drops root privileges. Thus, no
    Gtk/curses/mtr code should ever execute with more privileges than the
    access to the raw sockets.
    
    Unfortunately, mtr only does a seteuid(), and not a full setuid() when
    attempting to drop root privs. seteuid() _only_ affects the effective
    uid of the process, and the saved uid is therefore still 0 after this
    call. When a process has saved uid 0, it may issue a setuid(0) to
    regain full root privileges (i.e. real and effective uid 0). A
    malicious user that manages to take control over mtr (perhaps through
    gtk or curses) can thus execute arbitrary code as root, by simply
    calling setuid(0) first.
    
    Had setuid() been used instead of seteuid(), the saved uid would also
    have been affected, and a call to setuid(0) would fail, i.e. the
    process would not be able to regain root privileges.
    
    This behaviour of seteuid() is well documented in, for example,
    Advanced Programming in the UNIX Environment by W. Richard Stevens, as
    well as Solaris' manpages, and has been confirmed practically.
    
    Taking control over mtr is a question of, for example, finding a
    buffer overrun in mtr, Gtk, Gdk, glib or curses/ncurses. I suspect
    that not many sysadmins have scrutinized the involved library code for
    potential security holes, since it is not expected to be run with any
    special privileges by ordinary users, and mtr claims to drop root
    privs.
    
    Since the saved uid survives across fork() and exec(), any buffer
    overrun or similar bug in mtr is just as bad as if mtr had never done
    the seteuid() at all.
    
    The mtr code uses setuid() on HPUX, which according to the comments in
    the mtr code doesn't have the seteuid() call. It does seteuid() on all
    other systems though. It is unclear why the mtr authors favoured
    seteuid() before setuid() on platforms that have it.
    
    The mtr developers have been contacted on the address supplied with
    the code, but no reply has been received.
    
    The remedy to this problem is very simple: the call to seteuid()
    should be replaced with a call to setuid(). Apply the following diff to mtr.c
    in the mtr distribution.
    
    ----------------------------------------
    161c161
    <   if(seteuid(getuid())) {
    ---
    >   if(setuid(getuid())) {
    ----------------------------------------
    
    I hope that any further development of mtr will include this patch in
    the distribution.
    
    
    /Viktor...
    
    --|     Viktor Fougstedt, system administrator at dtek.chalmers.se     |--
    --|                http://www.dtek.chalmers.se/~viktor/                |--
    --| ...soon we'll be sliding down the razor blade of life. /Tom Lehrer |--
    



    This archive was generated by hypermail 2b30 : Fri Apr 13 2001 - 15:38:49 PDT