From 2585e1e48abb3013abeb8a1fe9dccb5f79ac4091 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sun, 20 Oct 2013 16:57:07 +0300 Subject: [PATCH] =?utf8?q?Issue=20#16038:=20CVE-2013-1752:=20ftplib:=20Lim?= =?utf8?q?it=20amount=20of=20data=20read=20by=20limiting=20the=20call=20to?= =?utf8?q?=20readline().=20=20Original=20patch=20by=20Micha=C5=82=20Jastrz?= =?utf8?q?=C4=99bski=20and=20Giampaolo=20Rodola.?= MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit --- Lib/ftplib.py | 27 +++++++++++++++++++++------ Lib/test/test_ftplib.py | 22 +++++++++++++++++++++- Misc/NEWS | 4 ++++ 3 files changed, 46 insertions(+), 7 deletions(-) diff --git a/Lib/ftplib.py b/Lib/ftplib.py index a62218b333..c98290ce85 100644 --- 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 e6aaca5376..4060015541 100644 --- 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 b0b4f10429..312bfe1f57 100644 --- 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 -- 2.50.1