Re: ssh-1.2.26 buffer overflow patch

From: Joel Eriksson (na98jenat_private)
Date: Wed Nov 04 1998 - 04:50:49 PST

  • Next message: Casper Dik: "Re: another /usr/dt/bin/dtappgather feature!"

    On Sun, 1 Nov 1998, Andy Church wrote:
    
    > [login.c:538]
    > log_msg("putuserattr S_LASTTTY %.900s failed: %.100s", /*...*/);
    >
    > [log-server.c:130]
    > void log_msg(const char *fmt, ...)
    > {
    >   char buf[1024];
    >   /*...*/
    >   vsprintf(buf, fmt, args);
    >   /*...*/
    > }
    >
    > Granted, I haven't checked whether this is exploitable, but I could never
    > call that secure.  (Count the bytes.)
    
    I don't think there's any reason of not using snprintf, but, in this
    particular case they seem to be rather well protected anyway. I have
    noticed the line you mention above, and a few others that woke my
    interest. But since the whole log_msg-call reads:
    
            log_msg("putuserattr S_LASTTTY %.900s failed: %.100s",
                ttyname, strerror(errno));
    
    It means that strerror() must return a string with a length around 100
    bytes *and* ttyname should be around 900 bytes, I have not checked if that
    is possible, but I certainly don't think so.
    
    There are a few other places that made me wonder, like when lines like:
    
              log_msg("Connection for %.200s not allowed from %s\n",
                      user, get_canonical_hostname());
    
    were used, when tracing back to get_canonical_hostname() and then back to
    get_remote_hostname() it seems as the name is limited to 255 bytes anyway.
    
    In auth-kerberos.c there are frequent use of %s in the log_msg()-calls and
    it sure looks dangerous.. In the places where I have tracked the origin of
    the strings supplied their length have been checked *before* log_msg() is
    used. I think that IBM's statements about not having found any exploitable
    overflows in ssh-1.2.26 can probably be trusted, but since the functions
    for logging uses vsprintf to a buffer on 1024 bytes it certainly is very
    possible to introduce a severe securityhole if not being careful. It
    certainly seems a lot easier to me to use vsnprintf's instead and limit
    the length to 1023 bytes instead of having to track back every use of the
    functions to be certain no securityproblem exists.
    
    >   --Andy Church                    | If Bell Atlantic really is the heart
    >     achurchat_private         | of communication, then it desperately
    >     http://achurch.dragonfire.net/ | needs a quadruple bypass.
    
    /Joel Eriksson
    



    This archive was generated by hypermail 2b30 : Fri Apr 13 2001 - 14:22:04 PDT