--------------------------------------------------------------
Security Report : integer overflows in Putty's sftp client
by Gael Delalleau  <gael.delalleau+moz@m4x.org>  (dec. 2004)
--------------------------------------------------------------

Note: these bugs may affect Putty's pscp too.


Overview
--------
Two integer overflows were identified in Putty's sftp client "psftp".
A malicious sftp server can overwrite the heap with arbitrary data,
thus crashing the client or possibly executing arbitrary code with
the rights of the user running psftp.


Technical Details
-----------------

Tests were done on a Mandrake Linux system with Putty 0.56 compiled
from source code.

BUG #1
------
In sftp.c:

The following function is called from scp_sftp_listdir(char *dirname),
from scp_get_sink_action in scp.c and from psftp.c in sftp_cmd_ls():

struct fxp_names *fxp_readdir_recv(struct sftp_packet *pktin,
                                   struct sftp_request *req) {
        ret->nnames = sftp_pkt_getuint32(pktin);               << 32 bits value from remote >>
        ret->names = snewn(ret->nnames, struct fxp_name);      << INT OVERFLOW => ALLOCATED BUFFER TOO SMALL >>
        for (i = 0; i < ret->nnames; i++) {
		 << HEAP CORRUPTION BELOW THIS LINE >>
            char *str;
            int len;
            sftp_pkt_getstring(pktin, &str, &len);
            ret->names[i].filename = mkstr(str, len);   <<--- pointer to arbitrary data taken from packet
            sftp_pkt_getstring(pktin, &str, &len);
            ret->names[i].longname = mkstr(str, len);   <<--- pointer to arbitrary data taken from packet
            ret->names[i].attrs = sftp_pkt_getattrs(pktin);  <<--- arbitrary data taken from packet

	}

The snewn() macro is defined as:
  #define snewn(n, type) ((type *)smalloc((n)*sizeof(type)))
where smalloc() is equivalent to malloc().

Since sizeof (struct fxp_name) is 40 bytes, we can trigger the integer
overflow by supplying a value of 0x06666667 for ret->nnames. This value
is supplied in a readdir() packet by the sftp server, after the client
has issue a 'ls' command. A buffer of 0x06666667*40 = 24 bytes is
allocated, then the loop copies values from the packet to the heap,
overwriting heap control structure and data.


BUG #2
------
In sftp.c:

The sftp_pkt_getstring() function takes the length of the string from
the packet as a signed integer. Thus, *length can be set to an arbitrary
value. If *length is negative, the check [1] is passed, *p is set and
pkt->savedpos is decremented by *length.

static void sftp_pkt_getstring(struct sftp_packet *pkt,
                               char **p, int *length)
{                              
    *p = NULL;
    if (pkt->length - pkt->savedpos < 4)
        return;
    *length = GET_32BIT(pkt->data + pkt->savedpos);     <<--- arbitrary value
    pkt->savedpos += 4;
    if (pkt->length - pkt->savedpos < *length)          [1]
        return;                                         <<--- check passed if length < 0
    *p = pkt->data + pkt->savedpos;
    pkt->savedpos += *length;
}

This bug leads to an exploitable scenario in fxp_open_recv():
        sftp_pkt_getstring(pktin, &hstring, &len);   <<-- we control len
	...
	handle = snew(struct fxp_handle);
        handle->hstring = mkstr(hstring, len);       <<-- mkstr() heap corruption if len == -1
        handle->hlen = len;
        sftp_pkt_free(pktin);
        return handle;

where mkstr is:

static char *mkstr(char *s, int len)
{
    char *p = snewn(len + 1, char);      <<-- len + 1 = 0 if len = -1 
    memcpy(p, s, len);                   <<-- heap corruption with arbitrary data
    p[len] = '\0';
    return p;
}

A 0-bytes buffer is allocated, then the memcpy() will try to copy 4Gb of
data on the heap. 


Exploitation
------------
Arbitrary code execution was achieved in the past on other applications with
similar bugs, at least on some systems. The problem is usually the crash
we experience due to the big copy loop ultimately writing into non-allocated
memory.

BUG #1 is likely exploitable because there are memory management calls inside
the copy loop (calls to mkstr => newn => safemalloc => libc's malloc ). These
calls make use of the heap we just trashed, BEFORE any crash due to bad memory
writes. This may be used to achieve exploitation. I got crashes into
mallopt() during my tests:

psftp> ls
Listing directory /home/admin
Segmentation fault (core dumped)

[gael@localhost putty-0.56]$ gdb unix/psftp core.13361
(...)
Program terminated with signal 11, Segmentation fault.
#0  0x27f61baf in mallopt () from /lib/i686/libc.so.6
(gdb) bt
#0  0x27f61baf in mallopt () from /lib/i686/libc.so.6
#1  0x27f60cb8 in malloc () from /lib/i686/libc.so.6
#2  0x0804be43 in safemalloc (size=60) at ../misc.c:341
#3  0x08055684 in mkstr (s=0x80b6c94 "drwxr-xr-x    2 admin    admin        4096 Feb 12  2004 tmp",
    len=59) at ../sftp.c:380
#4  0x0805625e in fxp_readdir_recv (pktin=0x80a9b28, req=0x80b0450) at ../sftp.c:954
#5  0x0804f1fc in sftp_cmd_ls (cmd=0x0) at ../psftp.c:251
#6  0x08051220 in do_sftp (mode=0, modeflags=1, batchfile=0x0) at ../psftp.c:1557
#7  0x08051ee6 in psftp_main (argc=4, argv=0x5d2779f4) at ../psftp.c:2095
#8  0x0807d72c in main (argc=4, argv=0x5d2779f4) at ../unix/uxsftp.c:406
#9  0x27f04c57 in __libc_start_main () from /lib/i686/libc.so.6


BUG #2 may be harder to exploit, maybe easier on 64 bit systems.

Because of lack of time and interest into this client-side vulnerability
I did not investigate further.



- End of Security Report -

Gael Delalleau