Commit c1143539 authored by Georg Brandl's avatar Georg Brandl

#16042: CVE-2013-1752: smtplib fix for unlimited readline() from socket

parent f580d5b6
...@@ -62,6 +62,7 @@ SMTP_PORT = 25 ...@@ -62,6 +62,7 @@ SMTP_PORT = 25
SMTP_SSL_PORT = 465 SMTP_SSL_PORT = 465
CRLF = "\r\n" CRLF = "\r\n"
bCRLF = b"\r\n" bCRLF = b"\r\n"
_MAXLINE = 8192 # more than 8 times larger than RFC 821, 4.5.3
OLDSTYLE_AUTH = re.compile(r"auth=(.*)", re.I) OLDSTYLE_AUTH = re.compile(r"auth=(.*)", re.I)
...@@ -364,7 +365,7 @@ class SMTP: ...@@ -364,7 +365,7 @@ class SMTP:
self.file = self.sock.makefile('rb') self.file = self.sock.makefile('rb')
while 1: while 1:
try: try:
line = self.file.readline() line = self.file.readline(_MAXLINE + 1)
except socket.error as e: except socket.error as e:
self.close() self.close()
raise SMTPServerDisconnected("Connection unexpectedly closed: " raise SMTPServerDisconnected("Connection unexpectedly closed: "
...@@ -374,6 +375,8 @@ class SMTP: ...@@ -374,6 +375,8 @@ class SMTP:
raise SMTPServerDisconnected("Connection unexpectedly closed") raise SMTPServerDisconnected("Connection unexpectedly closed")
if self.debuglevel > 0: if self.debuglevel > 0:
print('reply:', repr(line), file=stderr) print('reply:', repr(line), file=stderr)
if len(line) > _MAXLINE:
raise SMTPResponseException(500, "Line too long.")
resp.append(line[4:].strip(b' \t\r\n')) resp.append(line[4:].strip(b' \t\r\n'))
code = line[:3] code = line[:3]
# Check that the error code is syntactically correct. # Check that the error code is syntactically correct.
......
...@@ -21,8 +21,13 @@ class MockFile: ...@@ -21,8 +21,13 @@ class MockFile:
""" """
def __init__(self, lines): def __init__(self, lines):
self.lines = lines self.lines = lines
def readline(self): def readline(self, limit=-1):
return self.lines.pop(0) + b'\r\n' result = self.lines.pop(0) + b'\r\n'
if limit >= 0:
# Re-insert the line, removing the \r\n we added.
self.lines.insert(0, result[limit:-2])
result = result[:limit]
return result
def close(self): def close(self):
pass pass
......
...@@ -569,6 +569,33 @@ class BadHELOServerTests(unittest.TestCase): ...@@ -569,6 +569,33 @@ class BadHELOServerTests(unittest.TestCase):
HOST, self.port, 'localhost', 3) HOST, self.port, 'localhost', 3)
@unittest.skipUnless(threading, 'Threading required for this test.')
class TooLongLineTests(unittest.TestCase):
respdata = b'250 OK' + (b'.' * smtplib._MAXLINE * 2) + b'\n'
def setUp(self):
self.old_stdout = sys.stdout
self.output = io.StringIO()
sys.stdout = self.output
self.evt = threading.Event()
self.sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
self.sock.settimeout(15)
self.port = support.bind_port(self.sock)
servargs = (self.evt, self.respdata, self.sock)
threading.Thread(target=server, args=servargs).start()
self.evt.wait()
self.evt.clear()
def tearDown(self):
self.evt.wait()
sys.stdout = self.old_stdout
def testLineTooLong(self):
self.assertRaises(smtplib.SMTPResponseException, smtplib.SMTP,
HOST, self.port, 'localhost', 3)
sim_users = {'Mr.A@somewhere.com':'John A', sim_users = {'Mr.A@somewhere.com':'John A',
'Ms.B@xn--fo-fka.com':'Sally B', 'Ms.B@xn--fo-fka.com':'Sally B',
'Mrs.C@somewhereesle.com':'Ruth C', 'Mrs.C@somewhereesle.com':'Ruth C',
...@@ -885,7 +912,8 @@ class SMTPSimTests(unittest.TestCase): ...@@ -885,7 +912,8 @@ class SMTPSimTests(unittest.TestCase):
def test_main(verbose=None): def test_main(verbose=None):
support.run_unittest(GeneralTests, DebuggingServerTests, support.run_unittest(GeneralTests, DebuggingServerTests,
NonConnectingTests, NonConnectingTests,
BadHELOServerTests, SMTPSimTests) BadHELOServerTests, SMTPSimTests,
TooLongLineTests)
if __name__ == '__main__': if __name__ == '__main__':
test_main() test_main()
...@@ -50,6 +50,9 @@ Core and Builtins ...@@ -50,6 +50,9 @@ Core and Builtins
Library Library
------- -------
- Issue #16042: CVE-2013-1752: smtplib: Limit amount of data read by
limiting the call to readline(). Original patch by Christian Heimes.
- Issue #20317: ExitStack.__exit__ could create a self-referential loop if an - Issue #20317: ExitStack.__exit__ could create a self-referential loop if an
exception raised by a cleanup operation already had its context set exception raised by a cleanup operation already had its context set
correctly (for example, by the @contextmanager decorator). The infinite correctly (for example, by the @contextmanager decorator). The infinite
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment