| commit | author | age | ||
| f63228 | 1 | |
| CS | 2 | # HG changeset patch |
| 3 | # User Serhiy Storchaka <storchaka@gmail.com> | |
| 4 | # Date 1382277427 -10800 | |
| 5 | # Node ID 44ac81e6d584758ee56a865a7c18d82505be0643 | |
| 6 | # Parent 625ece68d79a27d376889579c414ed4b2d8a2649 | |
| 7 | Issue #16038: CVE-2013-1752: ftplib: Limit amount of data read by | |
| 8 | limiting the call to readline(). Original patch by Michał | |
| 9 | Jastrzębski and Giampaolo Rodola. | |
| 10 | ||
| 11 | diff --git a/Lib/ftplib.py b/Lib/ftplib.py | |
| 12 | --- a/Lib/ftplib.py | |
| 13 | +++ b/Lib/ftplib.py | |
| 14 | @@ -55,6 +55,8 @@ MSG_OOB = 0x1 | |
| 15 | ||
| 16 | # The standard FTP server control port | |
| 17 | FTP_PORT = 21 | |
| 18 | +# The sizehint parameter passed to readline() calls | |
| 19 | +MAXLINE = 8192 | |
| 20 | ||
| 21 | ||
| 22 | # Exception raised when an error or invalid response is received | |
| 23 | @@ -101,6 +103,7 @@ class FTP: | |
| 24 | debugging = 0 | |
| 25 | host = '' | |
| 26 | port = FTP_PORT | |
| 27 | + maxline = MAXLINE | |
| 28 | sock = None | |
| 29 | file = None | |
| 30 | welcome = None | |
| 31 | @@ -180,7 +183,9 @@ class FTP: | |
| 32 | # Internal: return one line from the server, stripping CRLF. | |
| 33 | # Raise EOFError if the connection is closed | |
| 34 | def getline(self): | |
| 35 | - line = self.file.readline() | |
| 36 | + line = self.file.readline(self.maxline + 1) | |
| 37 | + if len(line) > self.maxline: | |
| 38 | + raise Error("got more than %d bytes" % self.maxline) | |
| 39 | if self.debugging > 1: | |
| 40 | print '*get*', self.sanitize(line) | |
| 41 | if not line: raise EOFError | |
| 42 | @@ -432,7 +437,9 @@ class FTP: | |
| 43 | conn = self.transfercmd(cmd) | |
| 44 | fp = conn.makefile('rb') | |
| 45 | while 1: | |
| 46 | - line = fp.readline() | |
| 47 | + line = fp.readline(self.maxline + 1) | |
| 48 | + if len(line) > self.maxline: | |
| 49 | + raise Error("got more than %d bytes" % self.maxline) | |
| 50 | if self.debugging > 2: print '*retr*', repr(line) | |
| 51 | if not line: | |
| 52 | break | |
| 53 | @@ -485,7 +492,9 @@ class FTP: | |
| 54 | self.voidcmd('TYPE A') | |
| 55 | conn = self.transfercmd(cmd) | |
| 56 | while 1: | |
| 57 | - buf = fp.readline() | |
| 58 | + buf = fp.readline(self.maxline + 1) | |
| 59 | + if len(buf) > self.maxline: | |
| 60 | + raise Error("got more than %d bytes" % self.maxline) | |
| 61 | if not buf: break | |
| 62 | if buf[-2:] != CRLF: | |
| 63 | if buf[-1] in CRLF: buf = buf[:-1] | |
| 64 | @@ -710,7 +719,9 @@ else: | |
| 65 | fp = conn.makefile('rb') | |
| 66 | try: | |
| 67 | while 1: | |
| 68 | - line = fp.readline() | |
| 69 | + line = fp.readline(self.maxline + 1) | |
| 70 | + if len(line) > self.maxline: | |
| 71 | + raise Error("got more than %d bytes" % self.maxline) | |
| 72 | if self.debugging > 2: print '*retr*', repr(line) | |
| 73 | if not line: | |
| 74 | break | |
| 75 | @@ -748,7 +759,9 @@ else: | |
| 76 | conn = self.transfercmd(cmd) | |
| 77 | try: | |
| 78 | while 1: | |
| 79 | - buf = fp.readline() | |
| 80 | + buf = fp.readline(self.maxline + 1) | |
| 81 | + if len(buf) > self.maxline: | |
| 82 | + raise Error("got more than %d bytes" % self.maxline) | |
| 83 | if not buf: break | |
| 84 | if buf[-2:] != CRLF: | |
| 85 | if buf[-1] in CRLF: buf = buf[:-1] | |
| 86 | @@ -905,7 +918,9 @@ class Netrc: | |
| 87 | fp = open(filename, "r") | |
| 88 | in_macro = 0 | |
| 89 | while 1: | |
| 90 | - line = fp.readline() | |
| 91 | + line = fp.readline(self.maxline + 1) | |
| 92 | + if len(line) > self.maxline: | |
| 93 | + raise Error("got more than %d bytes" % self.maxline) | |
| 94 | if not line: break | |
| 95 | if in_macro and line.strip(): | |
| 96 | macro_lines.append(line) | |
| 97 | diff --git a/Lib/test/test_ftplib.py b/Lib/test/test_ftplib.py | |
| 98 | --- a/Lib/test/test_ftplib.py | |
| 99 | +++ b/Lib/test/test_ftplib.py | |
| 100 | @@ -65,6 +65,7 @@ class DummyFTPHandler(asynchat.async_cha | |
| 101 | self.last_received_data = '' | |
| 102 | self.next_response = '' | |
| 103 | self.rest = None | |
| 104 | + self.next_retr_data = RETR_DATA | |
| 105 | self.push('220 welcome') | |
| 106 | ||
| 107 | def collect_incoming_data(self, data): | |
| 108 | @@ -189,7 +190,7 @@ class DummyFTPHandler(asynchat.async_cha | |
| 109 | offset = int(self.rest) | |
| 110 | else: | |
| 111 | offset = 0 | |
| 112 | - self.dtp.push(RETR_DATA[offset:]) | |
| 113 | + self.dtp.push(self.next_retr_data[offset:]) | |
| 114 | self.dtp.close_when_done() | |
| 115 | self.rest = None | |
| 116 | ||
| 117 | @@ -203,6 +204,11 @@ class DummyFTPHandler(asynchat.async_cha | |
| 118 | self.dtp.push(NLST_DATA) | |
| 119 | self.dtp.close_when_done() | |
| 120 | ||
| 121 | + def cmd_setlongretr(self, arg): | |
| 122 | + # For testing. Next RETR will return long line. | |
| 123 | + self.next_retr_data = 'x' * int(arg) | |
| 124 | + self.push('125 setlongretr ok') | |
| 125 | + | |
| 126 | ||
| 127 | class DummyFTPServer(asyncore.dispatcher, threading.Thread): | |
| 128 | ||
| 129 | @@ -558,6 +564,20 @@ class TestFTPClass(TestCase): | |
| 130 | # IPv4 is in use, just make sure send_epsv has not been used | |
| 131 | self.assertEqual(self.server.handler.last_received_cmd, 'pasv') | |
| 132 | ||
| 133 | + def test_line_too_long(self): | |
| 134 | + self.assertRaises(ftplib.Error, self.client.sendcmd, | |
| 135 | + 'x' * self.client.maxline * 2) | |
| 136 | + | |
| 137 | + def test_retrlines_too_long(self): | |
| 138 | + self.client.sendcmd('SETLONGRETR %d' % (self.client.maxline * 2)) | |
| 139 | + received = [] | |
| 140 | + self.assertRaises(ftplib.Error, | |
| 141 | + self.client.retrlines, 'retr', received.append) | |
| 142 | + | |
| 143 | + def test_storlines_too_long(self): | |
| 144 | + f = StringIO.StringIO('x' * self.client.maxline * 2) | |
| 145 | + self.assertRaises(ftplib.Error, self.client.storlines, 'stor', f) | |
| 146 | + | |
| 147 | ||
| 148 | class TestIPv6Environment(TestCase): | |
| 149 | ||
