bpo-32981: Fix catastrophic backtracking vulns (#5955)

* Prevent low-grade poplib REDOS (CVE-2018-1060)

The regex to test a mail server's timestamp is susceptible to
catastrophic backtracking on long evil responses from the server.

Happily, the maximum length of malicious inputs is 2K thanks
to a limit introduced in the fix for CVE-2013-1752.

A 2KB evil response from the mail server would result in small slowdowns
(milliseconds vs. microseconds) accumulated over many apop calls.
This is a potential DOS vector via accumulated slowdowns.

Replace it with a similar non-vulnerable regex.

The new regex is RFC compliant.
The old regex was non-compliant in edge cases.

* Prevent difflib REDOS (CVE-2018-1061)

The default regex for IS_LINE_JUNK is susceptible to
catastrophic backtracking.
This is a potential DOS vector.

Replace it with an equivalent non-vulnerable regex.

Also introduce unit and REDOS tests for difflib.

Co-authored-by: Tim Peters <tim.peters@gmail.com>
Co-authored-by: Christian Heimes <christian@python.org>
This commit is contained in:
Jamie Davis 2018-03-04 00:33:32 -05:00 committed by Benjamin Peterson
parent 13cfd57dcf
commit 0e6c8ee235
6 changed files with 39 additions and 4 deletions

View File

@ -1083,7 +1083,7 @@ class Differ:
import re import re
def IS_LINE_JUNK(line, pat=re.compile(r"\s*#?\s*$").match): def IS_LINE_JUNK(line, pat=re.compile(r"\s*(?:#\s*)?$").match):
r""" r"""
Return 1 for ignorable line: iff `line` is blank or contains a single '#'. Return 1 for ignorable line: iff `line` is blank or contains a single '#'.

View File

@ -308,7 +308,7 @@ class POP3:
return self._shortcmd('RPOP %s' % user) return self._shortcmd('RPOP %s' % user)
timestamp = re.compile(br'\+OK.*(<[^>]+>)') timestamp = re.compile(br'\+OK.[^<]*(<.*>)')
def apop(self, user, password): def apop(self, user, password):
"""Authorisation """Authorisation

View File

@ -466,13 +466,33 @@ class TestBytes(unittest.TestCase):
list(generator(*args)) list(generator(*args))
self.assertEqual(msg, str(ctx.exception)) self.assertEqual(msg, str(ctx.exception))
class TestJunkAPIs(unittest.TestCase):
def test_is_line_junk_true(self):
for line in ['#', ' ', ' #', '# ', ' # ', '']:
self.assertTrue(difflib.IS_LINE_JUNK(line), repr(line))
def test_is_line_junk_false(self):
for line in ['##', ' ##', '## ', 'abc ', 'abc #', 'Mr. Moose is up!']:
self.assertFalse(difflib.IS_LINE_JUNK(line), repr(line))
def test_is_line_junk_REDOS(self):
evil_input = ('\t' * 1000000) + '##'
self.assertFalse(difflib.IS_LINE_JUNK(evil_input))
def test_is_character_junk_true(self):
for char in [' ', '\t']:
self.assertTrue(difflib.IS_CHARACTER_JUNK(char), repr(char))
def test_is_character_junk_false(self):
for char in ['a', '#', '\n', '\f', '\r', '\v']:
self.assertFalse(difflib.IS_CHARACTER_JUNK(char), repr(char))
def test_main(): def test_main():
difflib.HtmlDiff._default_prefix = 0 difflib.HtmlDiff._default_prefix = 0
Doctests = doctest.DocTestSuite(difflib) Doctests = doctest.DocTestSuite(difflib)
run_unittest( run_unittest(
TestWithAscii, TestAutojunk, TestSFpatches, TestSFbugs, TestWithAscii, TestAutojunk, TestSFpatches, TestSFbugs,
TestOutputFormat, TestBytes, Doctests) TestOutputFormat, TestBytes, TestJunkAPIs, Doctests)
if __name__ == '__main__': if __name__ == '__main__':
test_main() test_main()

View File

@ -308,9 +308,19 @@ class TestPOP3Class(TestCase):
def test_rpop(self): def test_rpop(self):
self.assertOK(self.client.rpop('foo')) self.assertOK(self.client.rpop('foo'))
def test_apop(self): def test_apop_normal(self):
self.assertOK(self.client.apop('foo', 'dummypassword')) self.assertOK(self.client.apop('foo', 'dummypassword'))
def test_apop_REDOS(self):
# Replace welcome with very long evil welcome.
# NB The upper bound on welcome length is currently 2048.
# At this length, evil input makes each apop call take
# on the order of milliseconds instead of microseconds.
evil_welcome = b'+OK' + (b'<' * 1000000)
with test_support.swap_attr(self.client, 'welcome', evil_welcome):
# The evil welcome is invalid, so apop should throw.
self.assertRaises(poplib.error_proto, self.client.apop, 'a', 'kb')
def test_top(self): def test_top(self):
expected = (b'+OK 116 bytes', expected = (b'+OK 116 bytes',
[b'From: postmaster@python.org', b'Content-Type: text/plain', [b'From: postmaster@python.org', b'Content-Type: text/plain',

View File

@ -356,6 +356,7 @@ Jonathan Dasteel
Pierre-Yves David Pierre-Yves David
A. Jesse Jiryu Davis A. Jesse Jiryu Davis
Jake Davis Jake Davis
Jamie (James C.) Davis
Ratnadeep Debnath Ratnadeep Debnath
Merlijn van Deen Merlijn van Deen
John DeGood John DeGood

View File

@ -0,0 +1,4 @@
Regexes in difflib and poplib were vulnerable to catastrophic backtracking.
These regexes formed potential DOS vectors (REDOS). They have been
refactored. This resolves CVE-2018-1060 and CVE-2018-1061.
Patch by Jamie Davis.