Brad Bishop | 004d499 | 2018-10-02 23:54:45 +0200 | [diff] [blame] | 1 | From 6d7ef39198856395edd62ef143bfcfaaf2ed6e25 Mon Sep 17 00:00:00 2001 |
| 2 | From: Ned Deily <nad@python.org> |
| 3 | Date: Sun, 11 Mar 2018 14:29:05 -0400 |
| 4 | Subject: [PATCH] [3.5] bpo-32981: Fix catastrophic backtracking vulns |
| 5 | (GH-5955) (#6034) |
| 6 | |
| 7 | * Prevent low-grade poplib REDOS (CVE-2018-1060) |
| 8 | |
| 9 | The regex to test a mail server's timestamp is susceptible to |
| 10 | catastrophic backtracking on long evil responses from the server. |
| 11 | |
| 12 | Happily, the maximum length of malicious inputs is 2K thanks |
| 13 | to a limit introduced in the fix for CVE-2013-1752. |
| 14 | |
| 15 | A 2KB evil response from the mail server would result in small slowdowns |
| 16 | (milliseconds vs. microseconds) accumulated over many apop calls. |
| 17 | This is a potential DOS vector via accumulated slowdowns. |
| 18 | |
| 19 | Replace it with a similar non-vulnerable regex. |
| 20 | |
| 21 | The new regex is RFC compliant. |
| 22 | The old regex was non-compliant in edge cases. |
| 23 | |
| 24 | * Prevent difflib REDOS (CVE-2018-1061) |
| 25 | |
| 26 | The default regex for IS_LINE_JUNK is susceptible to |
| 27 | catastrophic backtracking. |
| 28 | This is a potential DOS vector. |
| 29 | |
| 30 | Replace it with an equivalent non-vulnerable regex. |
| 31 | |
| 32 | Also introduce unit and REDOS tests for difflib. |
| 33 | |
| 34 | Co-authored-by: Tim Peters <tim.peters@gmail.com> |
| 35 | Co-authored-by: Christian Heimes <christian@python.org>. |
| 36 | (cherry picked from commit 0e6c8ee2358a2e23117501826c008842acb835ac) |
| 37 | CVE: CVE-2018-1061 |
| 38 | CVE: CVE-2018-1060 |
| 39 | Upstream-Status: Backport [https://github.com/python/cpython/commit/937ac1fe069a4dc8471dff205f553d82e724015b] |
| 40 | Signed-off-by: Sinan Kaya <okaya@kernel.org> |
| 41 | --- |
| 42 | Lib/difflib.py | 2 +- |
| 43 | Lib/poplib.py | 2 +- |
| 44 | Lib/test/test_difflib.py | 22 ++++++++++++++++++- |
| 45 | Lib/test/test_poplib.py | 12 +++++++++- |
| 46 | Misc/ACKS | 1 + |
| 47 | .../2018-03-02-10-24-52.bpo-32981.O_qDyj.rst | 4 ++++ |
| 48 | 6 files changed, 39 insertions(+), 4 deletions(-) |
| 49 | create mode 100644 Misc/NEWS.d/next/Security/2018-03-02-10-24-52.bpo-32981.O_qDyj.rst |
| 50 | |
| 51 | diff --git a/Lib/difflib.py b/Lib/difflib.py |
| 52 | index 076bbac01d..b4ec335056 100644 |
| 53 | --- a/Lib/difflib.py |
| 54 | +++ b/Lib/difflib.py |
| 55 | @@ -1083,7 +1083,7 @@ class Differ: |
| 56 | |
| 57 | import re |
| 58 | |
| 59 | -def IS_LINE_JUNK(line, pat=re.compile(r"\s*#?\s*$").match): |
| 60 | +def IS_LINE_JUNK(line, pat=re.compile(r"\s*(?:#\s*)?$").match): |
| 61 | r""" |
| 62 | Return 1 for ignorable line: iff `line` is blank or contains a single '#'. |
| 63 | |
| 64 | diff --git a/Lib/poplib.py b/Lib/poplib.py |
| 65 | index 516b6f060d..2437ea0e27 100644 |
| 66 | --- a/Lib/poplib.py |
| 67 | +++ b/Lib/poplib.py |
| 68 | @@ -308,7 +308,7 @@ class POP3: |
| 69 | return self._shortcmd('RPOP %s' % user) |
| 70 | |
| 71 | |
| 72 | - timestamp = re.compile(br'\+OK.*(<[^>]+>)') |
| 73 | + timestamp = re.compile(br'\+OK.[^<]*(<.*>)') |
| 74 | |
| 75 | def apop(self, user, password): |
| 76 | """Authorisation |
| 77 | diff --git a/Lib/test/test_difflib.py b/Lib/test/test_difflib.py |
| 78 | index ab9debf8e2..b6c8a7dd5b 100644 |
| 79 | --- a/Lib/test/test_difflib.py |
| 80 | +++ b/Lib/test/test_difflib.py |
| 81 | @@ -466,13 +466,33 @@ class TestBytes(unittest.TestCase): |
| 82 | list(generator(*args)) |
| 83 | self.assertEqual(msg, str(ctx.exception)) |
| 84 | |
| 85 | +class TestJunkAPIs(unittest.TestCase): |
| 86 | + def test_is_line_junk_true(self): |
| 87 | + for line in ['#', ' ', ' #', '# ', ' # ', '']: |
| 88 | + self.assertTrue(difflib.IS_LINE_JUNK(line), repr(line)) |
| 89 | + |
| 90 | + def test_is_line_junk_false(self): |
| 91 | + for line in ['##', ' ##', '## ', 'abc ', 'abc #', 'Mr. Moose is up!']: |
| 92 | + self.assertFalse(difflib.IS_LINE_JUNK(line), repr(line)) |
| 93 | + |
| 94 | + def test_is_line_junk_REDOS(self): |
| 95 | + evil_input = ('\t' * 1000000) + '##' |
| 96 | + self.assertFalse(difflib.IS_LINE_JUNK(evil_input)) |
| 97 | + |
| 98 | + def test_is_character_junk_true(self): |
| 99 | + for char in [' ', '\t']: |
| 100 | + self.assertTrue(difflib.IS_CHARACTER_JUNK(char), repr(char)) |
| 101 | + |
| 102 | + def test_is_character_junk_false(self): |
| 103 | + for char in ['a', '#', '\n', '\f', '\r', '\v']: |
| 104 | + self.assertFalse(difflib.IS_CHARACTER_JUNK(char), repr(char)) |
| 105 | |
| 106 | def test_main(): |
| 107 | difflib.HtmlDiff._default_prefix = 0 |
| 108 | Doctests = doctest.DocTestSuite(difflib) |
| 109 | run_unittest( |
| 110 | TestWithAscii, TestAutojunk, TestSFpatches, TestSFbugs, |
| 111 | - TestOutputFormat, TestBytes, Doctests) |
| 112 | + TestOutputFormat, TestBytes, TestJunkAPIs, Doctests) |
| 113 | |
| 114 | if __name__ == '__main__': |
| 115 | test_main() |
| 116 | diff --git a/Lib/test/test_poplib.py b/Lib/test/test_poplib.py |
| 117 | index bceeb93ad1..799e403652 100644 |
| 118 | --- a/Lib/test/test_poplib.py |
| 119 | +++ b/Lib/test/test_poplib.py |
| 120 | @@ -300,9 +300,19 @@ class TestPOP3Class(TestCase): |
| 121 | def test_rpop(self): |
| 122 | self.assertOK(self.client.rpop('foo')) |
| 123 | |
| 124 | - def test_apop(self): |
| 125 | + def test_apop_normal(self): |
| 126 | self.assertOK(self.client.apop('foo', 'dummypassword')) |
| 127 | |
| 128 | + def test_apop_REDOS(self): |
| 129 | + # Replace welcome with very long evil welcome. |
| 130 | + # NB The upper bound on welcome length is currently 2048. |
| 131 | + # At this length, evil input makes each apop call take |
| 132 | + # on the order of milliseconds instead of microseconds. |
| 133 | + evil_welcome = b'+OK' + (b'<' * 1000000) |
| 134 | + with test_support.swap_attr(self.client, 'welcome', evil_welcome): |
| 135 | + # The evil welcome is invalid, so apop should throw. |
| 136 | + self.assertRaises(poplib.error_proto, self.client.apop, 'a', 'kb') |
| 137 | + |
| 138 | def test_top(self): |
| 139 | expected = (b'+OK 116 bytes', |
| 140 | [b'From: postmaster@python.org', b'Content-Type: text/plain', |
| 141 | diff --git a/Misc/ACKS b/Misc/ACKS |
| 142 | index 1a35aad66c..72c5d740bd 100644 |
| 143 | --- a/Misc/ACKS |
| 144 | +++ b/Misc/ACKS |
| 145 | @@ -341,6 +341,7 @@ Kushal Das |
| 146 | Jonathan Dasteel |
| 147 | Pierre-Yves David |
| 148 | A. Jesse Jiryu Davis |
| 149 | +Jamie (James C.) Davis |
| 150 | Merlijn van Deen |
| 151 | John DeGood |
| 152 | Ned Deily |
| 153 | diff --git a/Misc/NEWS.d/next/Security/2018-03-02-10-24-52.bpo-32981.O_qDyj.rst b/Misc/NEWS.d/next/Security/2018-03-02-10-24-52.bpo-32981.O_qDyj.rst |
| 154 | new file mode 100644 |
| 155 | index 0000000000..9ebabb44f9 |
| 156 | --- /dev/null |
| 157 | +++ b/Misc/NEWS.d/next/Security/2018-03-02-10-24-52.bpo-32981.O_qDyj.rst |
| 158 | @@ -0,0 +1,4 @@ |
| 159 | +Regexes in difflib and poplib were vulnerable to catastrophic backtracking. |
| 160 | +These regexes formed potential DOS vectors (REDOS). They have been |
| 161 | +refactored. This resolves CVE-2018-1060 and CVE-2018-1061. |
| 162 | +Patch by Jamie Davis. |
| 163 | -- |
| 164 | 2.19.0 |
| 165 | |