Issue #16038: CVE-2013-1752: ftplib: Limit amount of data read by

limiting the call to readline().  Original patch by Michał
Jastrzębski and Giampaolo Rodola.

diff --git a/Lib/ftplib.py b/Lib/ftplib.py
index a62218b333b18269ff780d70a7092626e43c74d0..c98290ce85e996f2e10516d88f94094e06c9a78f 100644 (file)
--- a/Lib/ftplib.py
+++ b/Lib/ftplib.py
@@ -55,6 +55,8 @@ MSG_OOB = 0x1                           # Process data out of band
 
 # The standard FTP server control port
 FTP_PORT = 21
+# The sizehint parameter passed to readline() calls
+MAXLINE = 8192
 
 
 # Exception raised when an error or invalid response is received
@@ -101,6 +103,7 @@ class FTP:
     debugging = 0
     host = ''
     port = FTP_PORT
+    maxline = MAXLINE
     sock = None
     file = None
     welcome = None
@@ -180,7 +183,9 @@ class FTP:
     # Internal: return one line from the server, stripping CRLF.
     # Raise EOFError if the connection is closed
     def getline(self):
-        line = self.file.readline()
+        line = self.file.readline(self.maxline + 1)
+        if len(line) > self.maxline:
+            raise Error("got more than %d bytes" % self.maxline)
         if self.debugging > 1:
             print '*get*', self.sanitize(line)
         if not line: raise EOFError
@@ -432,7 +437,9 @@ class FTP:
         conn = self.transfercmd(cmd)
         fp = conn.makefile('rb')
         while 1:
-            line = fp.readline()
+            line = fp.readline(self.maxline + 1)
+            if len(line) > self.maxline:
+                raise Error("got more than %d bytes" % self.maxline)
             if self.debugging > 2: print '*retr*', repr(line)
             if not line:
                 break
@@ -485,7 +492,9 @@ class FTP:
         self.voidcmd('TYPE A')
         conn = self.transfercmd(cmd)
         while 1:
-            buf = fp.readline()
+            buf = fp.readline(self.maxline + 1)
+            if len(buf) > self.maxline:
+                raise Error("got more than %d bytes" % self.maxline)
             if not buf: break
             if buf[-2:] != CRLF:
                 if buf[-1] in CRLF: buf = buf[:-1]
@@ -710,7 +719,9 @@ else:
             fp = conn.makefile('rb')
             try:
                 while 1:
-                    line = fp.readline()
+                    line = fp.readline(self.maxline + 1)
+                    if len(line) > self.maxline:
+                        raise Error("got more than %d bytes" % self.maxline)
                     if self.debugging > 2: print '*retr*', repr(line)
                     if not line:
                         break
@@ -748,7 +759,9 @@ else:
             conn = self.transfercmd(cmd)
             try:
                 while 1:
-                    buf = fp.readline()
+                    buf = fp.readline(self.maxline + 1)
+                    if len(buf) > self.maxline:
+                        raise Error("got more than %d bytes" % self.maxline)
                     if not buf: break
                     if buf[-2:] != CRLF:
                         if buf[-1] in CRLF: buf = buf[:-1]
@@ -905,7 +918,9 @@ class Netrc:
         fp = open(filename, "r")
         in_macro = 0
         while 1:
-            line = fp.readline()
+            line = fp.readline(self.maxline + 1)
+            if len(line) > self.maxline:
+                raise Error("got more than %d bytes" % self.maxline)
             if not line: break
             if in_macro and line.strip():
                 macro_lines.append(line)

diff --git a/Lib/test/test_ftplib.py b/Lib/test/test_ftplib.py
index e6aaca5376bb5e93774b0223328dfd5f7b515983..40600155417903532818ad3879313e3e5270a205 100644 (file)
--- a/Lib/test/test_ftplib.py
+++ b/Lib/test/test_ftplib.py
@@ -65,6 +65,7 @@ class DummyFTPHandler(asynchat.async_chat):
         self.last_received_data = ''
         self.next_response = ''
         self.rest = None
+        self.next_retr_data = RETR_DATA
         self.push('220 welcome')
 
     def collect_incoming_data(self, data):
@@ -189,7 +190,7 @@ class DummyFTPHandler(asynchat.async_chat):
             offset = int(self.rest)
         else:
             offset = 0
-        self.dtp.push(RETR_DATA[offset:])
+        self.dtp.push(self.next_retr_data[offset:])
         self.dtp.close_when_done()
         self.rest = None
 
@@ -203,6 +204,11 @@ class DummyFTPHandler(asynchat.async_chat):
         self.dtp.push(NLST_DATA)
         self.dtp.close_when_done()
 
+    def cmd_setlongretr(self, arg):
+        # For testing. Next RETR will return long line.
+        self.next_retr_data = 'x' * int(arg)
+        self.push('125 setlongretr ok')
+
 
 class DummyFTPServer(asyncore.dispatcher, threading.Thread):
 
@@ -558,6 +564,20 @@ class TestFTPClass(TestCase):
         # IPv4 is in use, just make sure send_epsv has not been used
         self.assertEqual(self.server.handler.last_received_cmd, 'pasv')
 
+    def test_line_too_long(self):
+        self.assertRaises(ftplib.Error, self.client.sendcmd,
+                          'x' * self.client.maxline * 2)
+
+    def test_retrlines_too_long(self):
+        self.client.sendcmd('SETLONGRETR %d' % (self.client.maxline * 2))
+        received = []
+        self.assertRaises(ftplib.Error,
+                          self.client.retrlines, 'retr', received.append)
+
+    def test_storlines_too_long(self):
+        f = StringIO.StringIO('x' * self.client.maxline * 2)
+        self.assertRaises(ftplib.Error, self.client.storlines, 'stor', f)
+
 
 class TestIPv6Environment(TestCase):
 

diff --git a/Misc/NEWS b/Misc/NEWS
index b0b4f104296c782e1a814a825d48f153e58acec8..312bfe1f57034dc01f09d8415b78087a247980da 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -37,6 +37,10 @@ Core and Builtins
 Library
 -------
 
+- Issue #16038: CVE-2013-1752: ftplib: Limit amount of data read by
+  limiting the call to readline().  Original patch by Michał
+  Jastrzębski and Giampaolo Rodola.
+
 - Issue #19276: Fixed the wave module on 64-bit big-endian platforms.
 
 - Issue #18458: Prevent crashes with newer versions of libedit.  Its readline