Skip to content

Commit

Permalink
bpo-43124: Fix smtplib multiple CRLF injection (GH-25987)
Browse files Browse the repository at this point in the history
Co-authored-by: Łukasz Langa <lukasz@langa.pl>
  • Loading branch information
miguendes and ambv authored Aug 29, 2021
1 parent 3fc5d84 commit 0897253
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 3 deletions.
11 changes: 8 additions & 3 deletions Lib/smtplib.py
Original file line number Diff line number Diff line change
Expand Up @@ -367,10 +367,15 @@ def send(self, s):
def putcmd(self, cmd, args=""):
"""Send a command to the server."""
if args == "":
str = '%s%s' % (cmd, CRLF)
s = cmd
else:
str = '%s %s%s' % (cmd, args, CRLF)
self.send(str)
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.
Expand Down
55 changes: 55 additions & 0 deletions Lib/test/test_smtplib.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
with self.assertRaises(ValueError) as exc:
smtp.putcmd('helo\nX-INJECTED')
self.assertIn("prohibited newline characters", str(exc.exception))
smtp.quit()

def testVRFY(self):
smtp = smtplib.SMTP(HOST, self.port, local_hostname='localhost',
timeout=support.LOOPBACK_TIMEOUT)
Expand Down Expand Up @@ -417,6 +427,51 @@ 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)
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()
self.assertNotIn("X-INJECTED", debugout)

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)
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()
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'
smtp = smtplib.SMTP(HOST, self.port, local_hostname='localhost',
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Made the internal ``putcmd`` function in :mod:`smtplib` sanitize input for
presence of ``\r`` and ``\n`` characters to avoid (unlikely) command injection.

0 comments on commit 0897253

Please sign in to comment.