Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bpo-32981: Fix catastrophic backtracking vulns #5955

Merged

Conversation

@davisjam
Copy link
Contributor

@davisjam davisjam commented Mar 1, 2018

Fix catastrophic backtracking vulns disclosed by email.

  • CVE-2018-1060
  • CVE-2018-1061

https://bugs.python.org/issue32981

@davisjam davisjam requested a review from python/email-team as a code owner Mar 1, 2018
@the-knights-who-say-ni
Copy link

@the-knights-who-say-ni the-knights-who-say-ni commented Mar 1, 2018

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA (this might be simply due to a missing "GitHub Name" entry in your b.p.o account settings). This is necessary for legal reasons before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

Thanks again to your contribution and we look forward to looking at it!

@davisjam
Copy link
Contributor Author

@davisjam davisjam commented Mar 1, 2018

I have now signed the CLA.

@davisjam
Copy link
Contributor Author

@davisjam davisjam commented Mar 1, 2018

Because this is a security issue no issue was previously created to track it. Perhaps someone might create one now?

[Update: I created bpo-32981.]

@davisjam
Copy link
Contributor Author

@davisjam davisjam commented Mar 1, 2018

Travis failure:

Fixing Python file whitespace ... 2 files:
  Lib/test/test_difflib.py
  Lib/test/test_poplib.py

Took a look, I can't see what's wrong. Is there a command (or flag to configure) I can run on my end to get a more verbose error message?

@gvanrossum
Copy link
Member

@gvanrossum gvanrossum commented Mar 1, 2018

That error comes from Tools/scripts/patchcheck.py.

@davisjam davisjam force-pushed the davisjam:TestAndFixCatastrophicBacktracking branch from 6c9b1ef to 38bdbdd Mar 2, 2018
@davisjam
Copy link
Contributor Author

@davisjam davisjam commented Mar 2, 2018

Thank you. Fixed.

@davisjam davisjam changed the title Fix catastrophic backtracking vulns bpo-32981: Fix catastrophic backtracking vulns Mar 2, 2018
@davisjam
Copy link
Contributor Author

@davisjam davisjam commented Mar 2, 2018

I created bpo-32981 for this problem.

I skimmed through the git log. Commit messages for security seem to follow the 'XXX (CVE-...)' format rather than the 'bpo-XXX: ...' format. Let me know if I should change the commit messages though.

Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Please add a news entry and add your name into Misc/ACKS.

@@ -1083,7 +1083,7 @@ def _qformat(self, aline, bline, atags, btags):

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):

This comment has been minimized.

@serhiy-storchaka

serhiy-storchaka Mar 2, 2018
Member

Use non-capturing group: (?:#\s*).

This comment has been minimized.

@davisjam

davisjam Mar 2, 2018
Author Contributor

Fixed.

def test_is_line_junk_true(self):
for line in ['#', ' ', ' #', '# ', ' # ', '']:
is_junk = difflib.IS_LINE_JUNK(line)
self.assertEqual(is_junk, True)

This comment has been minimized.

@serhiy-storchaka

serhiy-storchaka Mar 2, 2018
Member

Why not assertTrue()?

This comment has been minimized.

@davisjam

davisjam Mar 2, 2018
Author Contributor

Fixed this and others, thanks.

elapsed = end - start

self.assertEqual(is_junk, False)
self.assertEqual(elapsed.seconds, 0)

This comment has been minimized.

@serhiy-storchaka

serhiy-storchaka Mar 2, 2018
Member

Timing test is very fragile. It can fail randomly when tests are paused for some reasons. I suggest to remove the timing test and increase the size of input so that it would take a hour with non-fixed code.

This comment has been minimized.

@davisjam

davisjam Mar 2, 2018
Author Contributor

Then the test will pass if you wait long enough. Since the test suite already takes a long time to run and does so in parallel (so one hanging test might not be visible), this seems undesirable to me. Thoughts?

This comment has been minimized.

@davisjam

davisjam Mar 2, 2018
Author Contributor

Same thing in Lib/test/test_poplib.py

This comment has been minimized.

@serhiy-storchaka

serhiy-storchaka Mar 2, 2018
Member

If the test takes too long time for running it will be failed on buildbots due to total timeout.

@davisjam davisjam force-pushed the davisjam:TestAndFixCatastrophicBacktracking branch 2 times, most recently from 1a03bf9 to 83bf67c Mar 2, 2018
@davisjam
Copy link
Contributor Author

@davisjam davisjam commented Mar 2, 2018

@serhiy-storchaka Thank you! I think I have addressed all of your comments so far.

@davisjam davisjam force-pushed the davisjam:TestAndFixCatastrophicBacktracking branch from 83bf67c to 539f390 Mar 2, 2018
Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Thank you James. Here yet few comments.

@@ -0,0 +1,3 @@
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.

This comment has been minimized.

@serhiy-storchaka

serhiy-storchaka Mar 2, 2018
Member

Add "Patch by your name."

This comment has been minimized.

@davisjam

davisjam Mar 2, 2018
Author Contributor

Done.

def test_is_character_junk_true(self):
for char in [' ', '\t']:
is_junk = difflib.IS_CHARACTER_JUNK(char)
self.assertEqual(is_junk, True)

This comment has been minimized.

@serhiy-storchaka

serhiy-storchaka Mar 2, 2018
Member

assertTrue()?

def test_is_line_junk_true(self):
for line in ['#', ' ', ' #', '# ', ' # ', '']:
is_junk = difflib.IS_LINE_JUNK(line)
self.assertTrue(is_junk)

This comment has been minimized.

@serhiy-storchaka

serhiy-storchaka Mar 2, 2018
Member

Why not just

self.assertTrue(difflib.IS_LINE_JUNK(line))

?

And you can add the second argument of assertTrue() for getting more information if the test fails:

self.assertTrue(difflib.IS_LINE_JUNK(line), rept(line))

But don't do this in test_is_line_junk_REDOS.

# At this length, evil input makes each apop call take
# on the order of milliseconds instead of microseconds.
evil_welcome = bytes('+OK' + ('<' * 500000), self.client.encoding)
self.client.welcome = evil_welcome

This comment has been minimized.

@serhiy-storchaka

serhiy-storchaka Mar 2, 2018
Member

You can use a special helper swap_attr() for temporary setting the attribute:

with test.support.swap_attr(self.client, 'welcome', evil_welcome):

This comment has been minimized.

@davisjam

davisjam Mar 2, 2018
Author Contributor

Ah, nice, thanks for the tip.

except poplib.error_proto:
threw = True
# The evil welcome is invalid, so apop should throw.
self.assertTrue(threw)

This comment has been minimized.

@serhiy-storchaka

serhiy-storchaka Mar 2, 2018
Member

Why not use assertRaises()?

This comment has been minimized.

@davisjam

davisjam Mar 2, 2018
Author Contributor

The reason for the poor choices of asserts is that I didn't know what asserts are available and made do with what I could find. I really appreciate your help.

This comment has been minimized.

@serhiy-storchaka

serhiy-storchaka Mar 2, 2018
Member

https://docs.python.org/3/library/unittest.html#test-cases

assertRaises() can be used as a function:

self.assertRaises(poplib.error_proto, self.client.apop, 'foo', 'dummypassword')

or as a context manager:

with self.assertRaises(poplib.error_proto):
    self.client.apop('foo', 'dummypassword')

This comment has been minimized.

@davisjam

davisjam Mar 2, 2018
Author Contributor

Yup I have at last found the documentation for unittest.

# 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 = bytes('+OK' + ('<' * 500000), self.client.encoding)

This comment has been minimized.

@serhiy-storchaka

serhiy-storchaka Mar 2, 2018
Member

What is the advantage over just b'+OK' + b'<' * 500000?

@@ -10,6 +10,7 @@
import os
import errno
import threading
import datetime

This comment has been minimized.

@serhiy-storchaka

serhiy-storchaka Mar 2, 2018
Member

No longer used.

@@ -3,6 +3,7 @@
import unittest
import doctest
import sys
import datetime

This comment has been minimized.

@serhiy-storchaka

serhiy-storchaka Mar 2, 2018
Member

No longer used.

@davisjam davisjam force-pushed the davisjam:TestAndFixCatastrophicBacktracking branch from 539f390 to 71db0e4 Mar 2, 2018
davisjam and others added 3 commits Feb 25, 2018
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.

Co-authored-by: Tim Peters <tim.peters@gmail.com>
Co-authored-by: Christian Heimes <christian@python.org>
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>
@davisjam davisjam force-pushed the davisjam:TestAndFixCatastrophicBacktracking branch from 71db0e4 to 7bf9559 Mar 2, 2018
@davisjam
Copy link
Contributor Author

@davisjam davisjam commented Mar 2, 2018

The latest version uses 1-million character attack strings. These take at least 3 minutes to evaluate on the master branch. I didn't want to wait for the full timeout.


def test_is_line_junk_false(self):
for line in ['##', ' ##', '## ', 'abc ', 'abc #', 'abc # ', 'Mr. Moose is up!']:
self.assertFalse(difflib.IS_LINE_JUNK(line), 'should not be junk: {}'.format(line))

This comment has been minimized.

@serhiy-storchaka

serhiy-storchaka Mar 2, 2018
Member

The length of this line exceeds the 79 character limit allowed by PEP 8.

And since testing strings contain spaces it is hard to distinguish the following cases:

should not be junk: abc #

and

should not be junk: abc # 

This is why I suggested repr().

This comment has been minimized.

@davisjam

davisjam Mar 2, 2018
Author Contributor

The length of this line exceeds the 79 character limit allowed by PEP 8.

Didn't realize this was being enforced, there are other lines over this limit as well. Will do.

This is why I suggested repr().

Got it, I misread that as rept earlier and couldn't understand what you meant.

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, 'arthur', 'kingofbritons')

This comment has been minimized.

@serhiy-storchaka

serhiy-storchaka Mar 2, 2018
Member

Since this line is too long, a context manager form of assertRaises() would be more preferable.

@serhiy-storchaka
Copy link
Member

@serhiy-storchaka serhiy-storchaka commented Mar 2, 2018

And please make your changes as new commits rather of editing old commits. This would make reviewing easier.

@davisjam
Copy link
Contributor Author

@davisjam davisjam commented Mar 2, 2018

@serhiy-storchaka Acknowledged. I believe I've addressed all of the concerns you've raised.

@davisjam
Copy link
Contributor Author

@davisjam davisjam commented Mar 2, 2018

Should I rebase or will these be squashed during merge anyway?

@serhiy-storchaka
Copy link
Member

@serhiy-storchaka serhiy-storchaka commented Mar 2, 2018

Will be squashed during merge.

@benjaminp benjaminp merged commit 0e6c8ee into python:master Mar 4, 2018
4 checks passed
4 checks passed
bedevere/issue-number Issue number 32981 found
Details
bedevere/news News entry found in Misc/NEWS.d
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@miss-islington
Copy link
Contributor

@miss-islington miss-islington commented Mar 4, 2018

Thanks @davisjam for the PR, and @benjaminp for merging it 🌮🎉.. I'm working now to backport this PR to: 2.7, 3.6, 3.7.
🐍🍒🤖

@bedevere-bot
Copy link

@bedevere-bot bedevere-bot commented Mar 4, 2018

@benjaminp: Please replace # with GH- in the commit message next time. Thanks!

miss-islington added a commit to miss-islington/cpython that referenced this pull request Mar 4, 2018
* 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>
(cherry picked from commit 0e6c8ee)

Co-authored-by: Jamie Davis <davisjam@vt.edu>
@bedevere-bot
Copy link

@bedevere-bot bedevere-bot commented Mar 4, 2018

GH-5969 is a backport of this pull request to the 3.7 branch.

@miss-islington
Copy link
Contributor

@miss-islington miss-islington commented Mar 4, 2018

Sorry, @davisjam and @benjaminp, I could not cleanly backport this to 3.6 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 0e6c8ee2358a2e23117501826c008842acb835ac 3.6

@miss-islington
Copy link
Contributor

@miss-islington miss-islington commented Mar 4, 2018

Sorry, @davisjam and @benjaminp, I could not cleanly backport this to 2.7 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 0e6c8ee2358a2e23117501826c008842acb835ac 2.7

benjaminp added a commit that referenced this pull request Mar 4, 2018
* 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>.
(cherry picked from commit 0e6c8ee)
@bedevere-bot
Copy link

@bedevere-bot bedevere-bot commented Mar 4, 2018

GH-5970 is a backport of this pull request to the 2.7 branch.

benjaminp added a commit that referenced this pull request Mar 4, 2018
* 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>.
(cherry picked from commit 0e6c8ee)
@bedevere-bot
Copy link

@bedevere-bot bedevere-bot commented Mar 4, 2018

GH-5971 is a backport of this pull request to the 3.6 branch.

benjaminp added a commit that referenced this pull request Mar 4, 2018
* 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>
Co-authored-by: Jamie Davis <davisjam@vt.edu>
(cherry picked from commit 0e6c8ee)
benjaminp added a commit that referenced this pull request Mar 4, 2018
* 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>.
(cherry picked from commit 0e6c8ee)
benjaminp added a commit that referenced this pull request Mar 4, 2018
* 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>.
(cherry picked from commit 0e6c8ee)
ned-deily added a commit to ned-deily/cpython that referenced this pull request Mar 8, 2018
* 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>.
(cherry picked from commit 0e6c8ee)
ned-deily added a commit to ned-deily/cpython that referenced this pull request Mar 8, 2018
* 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>.
larryhastings added a commit that referenced this pull request Mar 11, 2018
* 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>.
larryhastings added a commit that referenced this pull request Mar 11, 2018
* 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>.
(cherry picked from commit 0e6c8ee)
yahya-abou-imran added a commit to yahya-abou-imran/cpython that referenced this pull request Nov 2, 2018
* 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

7 participants