8000 [3.4] bpo-32981: Fix catastrophic backtracking vulns (GH-5955) (#6035) · python/cpython@942cc04 · GitHub
[go: up one dir, main page]
Skip to content

Commit 942cc04

Browse files
ned-deilytim-one
authored and
larryhastings
committed
[3.4] bpo-32981: Fix catastrophic backtracking vulns (GH-5955) (#6035)
* 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>.
1 parent f584ecd commit 942cc04

File tree

6 files changed

+39
-4
lines changed

6 files changed

+39
-4
lines changed

‎Lib/difflib.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1084,7 +1084,7 @@ def _qformat(self, aline, bline, atags, btags):
10841084

10851085
import re
10861086

1087-
def IS_LINE_JUNK(line, pat=re.compile(r"\s*#?\s*$").match):
1087+
def IS_LINE_JUNK(line, pat=re.compile(r"\s*(?:#\s*)?$").match):
10881088
r"""
10891089
Return 1 for ignorable line: iff `line` is blank or contains a single '#'.
10901090

‎Lib/poplib.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,7 @@ def rpop(self, user):
304304
return self._shortcmd('RPOP %s' % user)
305305

306306

307-
timestamp = re.compile(br'\+OK.*(<[^>]+>)')
307+
timestamp = re.compile(br'\+OK.[^<]*(<.*>)')
308308

309309
def apop(self, user, password):
310310
"""Authorisation

‎Lib/test/test_difflib.py

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -286,13 +286,33 @@ def test_range_format_context(self):
286286
self.assertEqual(fmt(3,6), '4,6')
287287
self.assertEqual(fmt(0,0), '0')
288288

289+
class TestJunkAPIs(unittest.TestCase):
290+
def test_is_line_junk_true(self):
291+
for line in ['#', ' ', ' #', '# ', ' # ', '']:
292+
self.assertTrue(difflib.IS_LINE_JUNK(line), repr(line))
293+
294+
def test_is_line_junk_false(self):
295+
for line in ['##', ' ##', '## ', 'abc ', 'abc #', 'Mr. Moose is up!']:
296+
self.assertFalse(difflib.IS_LINE_JUNK(line), repr(line))
297+
298+
def test_is_line_junk_REDOS(self):
299+
evil_input = ('\t' * 1000000) + '##'
300+
self.assertFalse(difflib.IS_LINE_JUNK(evil_input))
301+
302+
def test_is_character_junk_true(self):
303+
for char in [' ', '\t']:
304+
self.assertTrue(difflib.IS_CHARACTER_JUNK(char), repr(char))
305+
306+
def test_is_character_junk_false(self):
307+
for char in ['a', '#', '\n', '\f', '\r', '\v']:
308+
self.assertFalse(difflib.IS_CHARACTER_JUNK(char), repr(char))
289309

290310
def test_main():
291311
difflib.HtmlDiff._default_prefix = 0
292312
Doctests = doctest.DocTestSuite(difflib)
293313
run_unittest(
294314
TestWithAscii, TestAutojunk, TestSFpatches, TestSFbugs,
295-
TestOutputFormat, Doctests)
315+
TestOutputFormat, TestJunkAPIs, Doctests)
296316

297317
if __name__ == '__main__':
298318
test_main()

‎Lib/test/test_poplib.py

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -294,9 +294,19 @@ def test_noop(self):
294294
def test_rpop(self):
295295
self.assertOK(self.client.rpop('foo'))
296296

297-
def test_apop(self):
297+
def test_apop_normal(self):
298298
self.assertOK(self.client.apop('foo', 'dummypassword'))
299299

300+
def test_apop_REDOS(self):
301+
# Replace welcome with very long evil welcome.
302+
# NB The upper bound on welcome length is currently 2048.
303+
# At this length, evil input makes each apop call take
304+
# on the order of milliseconds instead of microseconds.
305+
evil_welcome = b'+OK' + (b'<' * 1000000)
306+
with test_support.swap_attr(self.client, 'welcome', evil_welcome):
307+
# The evil welcome is invalid, so apop should throw.
308+
self.assertRaises(poplib.error_proto, self.client.apop, 'a', 'kb')
309+
300310
def test_top(self):
301311
expected = (b'+OK 116 bytes',
302312
[b'From: postmaster@python.org', b'Content-Type: text/plain',

‎Misc/ACKS

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,7 @@ Kushal Das
319319
Jonathan Dasteel
320320
Pierre-Yves David
321321
A. Jesse Jiryu Davis
322+
Jamie (James C.) Davis
322323
Merlijn van Deen
323324
John DeGood
324325
Ned Deily

‎Misc/NEWS.d/next/Security/2018-03-02-10-24-52.bpo-32981.O_qDyj.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Regexes in difflib and poplib were vulnerable to catastrophic backtracking.
2+
These regexes formed potential DOS vectors (REDOS). They have been
3+
refactored. This resolves CVE-2018-1060 and CVE-2018-1061.
4+
Patch by Jamie Davis.

0 commit comments

Comments
 (0)
0