From ec13383cd498150578a2a57e57163302dd347b68 Mon Sep 17 00:00:00 2001 From: Miguel Mendes Date: Sat, 8 May 2021 11:52:46 +0100 Subject: [PATCH 1/3] Fix smtplib multiple CRLF injection --- Lib/smtplib.py | 6 +-- Lib/test/test_smtplib.py | 46 +++++++++++++++++++ .../2021-05-08-11-50-46.bpo-43124.2CTM6M.rst | 1 + 3 files changed, 50 insertions(+), 3 deletions(-) create mode 100644 Misc/NEWS.d/next/Security/2021-05-08-11-50-46.bpo-43124.2CTM6M.rst diff --git a/Lib/smtplib.py b/Lib/smtplib.py index 7e984e8c0fa1a1..99ad182378bcb0 100755 --- a/Lib/smtplib.py +++ b/Lib/smtplib.py @@ -367,10 +367,10 @@ def send(self, s): def putcmd(self, cmd, args=""): """Send a command to the server.""" if args == "": - str = '%s%s' % (cmd, CRLF) + str = cmd else: - str = '%s %s%s' % (cmd, args, CRLF) - self.send(str) + str = '%s %s' % (cmd, args) + self.send('%s%s' % (str.replace('\n', '\\n'), CRLF)) def getreply(self): """Get a reply from the server. diff --git a/Lib/test/test_smtplib.py b/Lib/test/test_smtplib.py index 483d747ee6a495..527061f8266715 100644 --- a/Lib/test/test_smtplib.py +++ b/Lib/test/test_smtplib.py @@ -336,6 +336,16 @@ def testEXPNNotImplemented(self): self.assertEqual(smtp.getreply(), expected) smtp.quit() + def test_issue43124_putcmd_escapes_newline(self): + # see: https://bugs.python.org/issue43124 + smtp = smtplib.SMTP(HOST, self.port, local_hostname='localhost', + timeout=support.LOOPBACK_TIMEOUT) + self.addCleanup(smtp.close) + expected = (500, b'Error: command "HELO\\NX-INJECTED" not recognized') + smtp.putcmd('helo\nX-INJECTED') + self.assertEqual(smtp.getreply(), expected) + smtp.quit() + def testVRFY(self): smtp = smtplib.SMTP(HOST, self.port, local_hostname='localhost', timeout=support.LOOPBACK_TIMEOUT) @@ -417,6 +427,42 @@ def testSendNeedingDotQuote(self): mexpect = '%s%s\n%s' % (MSG_BEGIN, m, MSG_END) self.assertEqual(self.output.getvalue(), mexpect) + def test_issue43124_escape_localhostname(self): + # see: https://bugs.python.org/issue43124 + # connect and send mail + m = 'wazzuuup\nlinetwo' + smtp = smtplib.SMTP(HOST, self.port, local_hostname='hi\nX-INJECTED', + timeout=support.LOOPBACK_TIMEOUT) + self.addCleanup(smtp.close) + smtp.sendmail("hi@me.com", "you@me.com", m) + # XXX (see comment in testSend) + time.sleep(0.01) + smtp.quit() + + debugout = smtpd.DEBUGSTREAM.getvalue() + ehlo = re.compile(r"ehlo hi\\\\nX-INJECTED", re.MULTILINE) + self.assertRegex(debugout, ehlo) + + def test_issue43124_escape_options(self): + # see: https://bugs.python.org/issue43124 + # connect and send mail + m = 'wazzuuup\nlinetwo' + smtp = smtplib.SMTP( + HOST, self.port, local_hostname='localhost', + timeout=support.LOOPBACK_TIMEOUT) + + self.addCleanup(smtp.close) + smtp.sendmail("hi@me.com", "you@me.com", m) + smtp.mail("hi@me.com", ["X-OPTION\nX-INJECTED-1", "X-OPTION2\nX-INJECTED-2"]) + # XXX (see comment in testSend) + time.sleep(0.01) + smtp.quit() + + debugout = smtpd.DEBUGSTREAM.getvalue() + expected_opts = re.compile(r"mail FROM: X-OPTION\\\\nX-INJECTED-1 X-OPTION2\\\\nX-INJECTED-2", + re.MULTILINE) + self.assertRegex(debugout, expected_opts) + def testSendNullSender(self): m = 'A test message' smtp = smtplib.SMTP(HOST, self.port, local_hostname='localhost', diff --git a/Misc/NEWS.d/next/Security/2021-05-08-11-50-46.bpo-43124.2CTM6M.rst b/Misc/NEWS.d/next/Security/2021-05-08-11-50-46.bpo-43124.2CTM6M.rst new file mode 100644 index 00000000000000..f2bd7b64037dfc --- /dev/null +++ b/Misc/NEWS.d/next/Security/2021-05-08-11-50-46.bpo-43124.2CTM6M.rst @@ -0,0 +1 @@ +Fix smtplib multiple CRLF injection From 4e0ee170be5f4cf849a1a8f9f4e589c9bdadcce2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Langa?= Date: Sun, 29 Aug 2021 12:50:29 +0200 Subject: [PATCH 2/3] Respond to R. David Murray review suggestions --- Lib/smtplib.py | 11 ++++++++--- Lib/test/test_smtplib.py | 29 +++++++++++++++++++---------- 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/Lib/smtplib.py b/Lib/smtplib.py index 99ad182378bcb0..324a1c19f12afe 100755 --- a/Lib/smtplib.py +++ b/Lib/smtplib.py @@ -367,10 +367,15 @@ def send(self, s): def putcmd(self, cmd, args=""): """Send a command to the server.""" if args == "": - str = cmd + s = cmd else: - str = '%s %s' % (cmd, args) - self.send('%s%s' % (str.replace('\n', '\\n'), CRLF)) + s = f'{cmd} {args}' + if '\r' in s or '\n' in s: + s = s.replace('\n', '\\n').replace('\r', '\\r') + raise ValueError( + f'command and arguments contain prohibited newline characters: {s}' + ) + self.send(f'{s}{CRLF}') def getreply(self): """Get a reply from the server. diff --git a/Lib/test/test_smtplib.py b/Lib/test/test_smtplib.py index 527061f8266715..9761a372517315 100644 --- a/Lib/test/test_smtplib.py +++ b/Lib/test/test_smtplib.py @@ -341,9 +341,9 @@ def test_issue43124_putcmd_escapes_newline(self): smtp = smtplib.SMTP(HOST, self.port, local_hostname='localhost', timeout=support.LOOPBACK_TIMEOUT) self.addCleanup(smtp.close) - expected = (500, b'Error: command "HELO\\NX-INJECTED" not recognized') - smtp.putcmd('helo\nX-INJECTED') - self.assertEqual(smtp.getreply(), expected) + with self.assertRaises(ValueError) as exc: + smtp.putcmd('helo\nX-INJECTED') + self.assertIn("prohibited newline characters", str(exc.exception)) smtp.quit() def testVRFY(self): @@ -434,14 +434,18 @@ def test_issue43124_escape_localhostname(self): smtp = smtplib.SMTP(HOST, self.port, local_hostname='hi\nX-INJECTED', timeout=support.LOOPBACK_TIMEOUT) self.addCleanup(smtp.close) - smtp.sendmail("hi@me.com", "you@me.com", m) + with self.assertRaises(ValueError) as exc: + smtp.sendmail("hi@me.com", "you@me.com", m) + self.assertIn( + "prohibited newline characters: ehlo hi\\nX-INJECTED", + str(exc.exception), + ) # XXX (see comment in testSend) time.sleep(0.01) smtp.quit() debugout = smtpd.DEBUGSTREAM.getvalue() - ehlo = re.compile(r"ehlo hi\\\\nX-INJECTED", re.MULTILINE) - self.assertRegex(debugout, ehlo) + self.assertNotIn("X-INJECTED", debugout) def test_issue43124_escape_options(self): # see: https://bugs.python.org/issue43124 @@ -453,15 +457,20 @@ def test_issue43124_escape_options(self): self.addCleanup(smtp.close) smtp.sendmail("hi@me.com", "you@me.com", m) - smtp.mail("hi@me.com", ["X-OPTION\nX-INJECTED-1", "X-OPTION2\nX-INJECTED-2"]) + with self.assertRaises(ValueError) as exc: + smtp.mail("hi@me.com", ["X-OPTION\nX-INJECTED-1", "X-OPTION2\nX-INJECTED-2"]) + msg = str(exc.exception) + self.assertIn("prohibited newline characters", msg) + self.assertIn("X-OPTION\\nX-INJECTED-1 X-OPTION2\\nX-INJECTED-2", msg) # XXX (see comment in testSend) time.sleep(0.01) smtp.quit() debugout = smtpd.DEBUGSTREAM.getvalue() - expected_opts = re.compile(r"mail FROM: X-OPTION\\\\nX-INJECTED-1 X-OPTION2\\\\nX-INJECTED-2", - re.MULTILINE) - self.assertRegex(debugout, expected_opts) + self.assertNotIn("X-OPTION", debugout) + self.assertNotIn("X-OPTION2", debugout) + self.assertNotIn("X-INJECTED-1", debugout) + self.assertNotIn("X-INJECTED-2", debugout) def testSendNullSender(self): m = 'A test message' From 61f28718ce854afcfb55233326adc90dd2769055 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Langa?= Date: Sun, 29 Aug 2021 12:53:58 +0200 Subject: [PATCH 3/3] Improve Blurb wording --- .../next/Security/2021-05-08-11-50-46.bpo-43124.2CTM6M.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Security/2021-05-08-11-50-46.bpo-43124.2CTM6M.rst b/Misc/NEWS.d/next/Security/2021-05-08-11-50-46.bpo-43124.2CTM6M.rst index f2bd7b64037dfc..e897d6cd3641d7 100644 --- a/Misc/NEWS.d/next/Security/2021-05-08-11-50-46.bpo-43124.2CTM6M.rst +++ b/Misc/NEWS.d/next/Security/2021-05-08-11-50-46.bpo-43124.2CTM6M.rst @@ -1 +1,2 @@ -Fix smtplib multiple CRLF injection +Made the internal ``putcmd`` function in :mod:`smtplib` sanitize input for +presence of ``\r`` and ``\n`` characters to avoid (unlikely) command injection.