python / cpython

The Python programming language
https://www.python.org
Other
63.38k stars 30.35k forks source link

imaplib: unlimited readline() from connection #60243

Closed tiran closed 10 years ago

tiran commented 12 years ago
BPO 16039
Nosy @warsaw, @akuchling, @birkenfeld, @vstinner, @larryhastings, @giampaolo, @tiran, @benjaminp, @bitdancer
PRs
  • python/cpython#11120
  • Files
  • imaplib.issue16039.patch
  • imaplib.txt
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields: ```python assignee = 'https://github.com/tiran' closed_at = created_at = labels = ['library', 'performance'] title = 'imaplib: unlimited readline() from connection' updated_at = user = 'https://github.com/tiran' ``` bugs.python.org fields: ```python activity = actor = 'vstinner' assignee = 'christian.heimes' closed = True closed_date = closer = 'georg.brandl' components = ['Library (Lib)'] creation = creator = 'christian.heimes' dependencies = [] files = ['29254', '31778'] hgrepos = [] issue_num = 16039 keywords = ['patch'] message_count = 29.0 messages = ['171242', '182193', '182196', '183096', '185054', '196860', '197823', '198299', '198300', '198301', '200346', '201426', '201427', '207218', '207224', '207227', '207231', '207233', '207239', '207254', '207255', '207256', '207257', '227925', '227927', '227928', '227931', '331692', '331698'] nosy_count = 12.0 nosy_names = ['barry', 'akuchling', 'georg.brandl', 'vstinner', 'larry', 'giampaolo.rodola', 'christian.heimes', 'benjamin.peterson', 'Arfrever', 'r.david.murray', 'python-dev', 'Emil.Lind'] pr_nums = ['11120'] priority = None resolution = 'fixed' stage = 'resolved' status = 'closed' superseder = None type = 'resource usage' url = 'https://bugs.python.org/issue16039' versions = ['Python 2.7'] ```

    tiran commented 12 years ago

    This bug is similar to bpo-16037 and a modified copy of bpo-16038.

    The imaplib module doesn't limit the amount of read data in its call to readline(). An erroneous or malicious IMAP server can trick the imaplib module to consume large amounts of memory.

    Suggestion: The imaplib module should be modified to use limited readline() with _MAXLINE like the httplib module.

    tiran commented 11 years ago

    RFC 3501 and 2060 (IMAP 4rev1) don't specify a line length

    RFC 2683 says:

    A client should limit the length of the command lines it generates to approximately 1000 octets.

    For its part, a server should allow for a command line of at least 8000 octets.

    Some config files and code have values between 2k and 64k, usually around 8k to 10k, e.g.

    UW and Panda IMAP have a limit of 10,000 octets which is far more than what anything is ever likely to use.

    tiran commented 11 years ago

    CVE-2013-1752 Unbound readline() DoS vulnerabilities in Python stdlib

    4a502687-f3db-48f0-b770-12a1309c168c commented 11 years ago

    I'm uploading my first patch. Heavily based on the related issues for ftplib and poplib. Need help with review and a few questions...

    Q1: Is the error Exception the right way to handle the "breach" (disconnects client?) or is there a better way? Like a 'BAD' response...

    Q2: I'm not sure how to best modify the test_imaplib for this patch. I'm guessing a make_server where the client gets MAXLINE+1 bytes of data and validates exception. But it's above my abilities right now...

    I welcome any input, thanks.

    note: patch seems to apply to 2.7, 3.2, 3.3, 3.4

    benjaminp commented 11 years ago

    Not blocking 2.7.4 as discussed on mailing list.

    warsaw commented 11 years ago

    blocker for 2.6.9

    akuchling commented 11 years ago

    Updated version of the patch against 2.6 that adds a test. Thanks for the fix, Emil!

    warsaw commented 11 years ago

    Looks good for 2.6. The NEWS file hunk doesn't apply, but I'll fix that when I commit this to 2.6.

    1762cc99-3127-4a62-9baf-30c3d0f51ef7 commented 11 years ago

    New changeset 4190568ceda0 by Barry Warsaw in branch '2.6':

    warsaw commented 11 years ago

    Since the merge 2.6 -> 2.7 did not apply cleanly, and had other problems. I null merged the 2.6 changes. I'll leave it to Benjamin to work out whatever patches 2.7 needs.

    larryhastings commented 11 years ago

    Ping. Please fix before "beta 1".

    1762cc99-3127-4a62-9baf-30c3d0f51ef7 commented 11 years ago

    New changeset 4b0364fc5711 by Georg Brandl in branch '3.3': Issue bpo-16039: CVE-2013-1752: Change use of readline in imaplib module to limit http://hg.python.org/cpython/rev/4b0364fc5711

    birkenfeld commented 11 years ago

    Also merged to default.

    vstinner commented 10 years ago

    Why is this issue still open? The issue was fixed in Python 2.6.9. Why is the issue a release blocker? The issue was also fixed in the future Python 3.4 (in default).

    bitdancer commented 10 years ago

    Presumably because it has not been fixed in 2.7.

    vstinner commented 10 years ago

    "Since the merge 2.6 -> 2.7 did not apply cleanly, and had other problems. I null merged the 2.6 changes. I'll leave it to Benjamin to work out whatever patches 2.7 needs."

    So Benjamin, is there a reason to not fix this security vulnerability in Python 2.7?

    benjaminp commented 10 years ago

    There's no reason not to fix it assuming the patch is good...

    bitdancer commented 10 years ago

    Applied to 2.7 in dd906f4ab923.

    bitdancer commented 10 years ago

    And we're getting test failures in the SSL version of the test. No similar failure reports in the tracker, and the same test has been running on the Python3 branch for a while now.

    1762cc99-3127-4a62-9baf-30c3d0f51ef7 commented 10 years ago

    New changeset d7ae948d9eee by R David Murray in branch '2.7': bpo-16039/bpo-20118: temporarily skip failing imaplib SSL test. http://hg.python.org/cpython/rev/d7ae948d9eee

    vstinner commented 10 years ago

    Reopen, a test is failing.

    bitdancer commented 10 years ago

    I opened a new issue for the failing test: bpo-20118, so I don't see a reason to keep this open.

    vstinner commented 10 years ago

    "I opened a new issue for the failing test: bpo-20118, so I don't see a reason to keep this open."

    Ok, I wasn't aware of this issue.

    1762cc99-3127-4a62-9baf-30c3d0f51ef7 commented 10 years ago

    New changeset 5d1c03316af7 by Georg Brandl in branch '3.2': Issue bpo-16039: CVE-2013-1752: Change use of readline in imaplib module to limit https://hg.python.org/cpython/rev/5d1c03316af7

    vstinner commented 10 years ago

    New changeset 5d1c03316af7 by Georg Brandl in branch '3.2': Issue bpo-16039: CVE-2013-1752: Change use of readline in imaplib module to limit https://hg.python.org/cpython/rev/5d1c03316af7

    I'm not sure that this change is correct, the test failed on Windows. Or maybe, it's just an issue with test test?

    http://buildbot.python.org/all/builders/AMD64%20Windows7%20SP1%203.x/builds/5168/steps/test/logs/stdio

    \====================================================================== ERROR: test_connect (test.test_smtpnet.SmtpSSLTest) ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\test_smtpnet.py", line 16, in test_connect
        server = smtplib.SMTP_SSL(self.testServer, self.remotePort)
      File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\smtplib.py", line 862, in __init__
        SMTP.__init__(self, host, port, local_hostname, timeout)
      File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\smtplib.py", line 260, in __init__
        (code, msg) = self.connect(host, port)
      File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\smtplib.py", line 321, in connect
        (code, msg) = self.getreply()
      File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\smtplib.py", line 367, in getreply
        line = self.file.readline(_MAXLINE + 1)
    TypeError: readline() takes exactly 1 positional argument (2 given)
    birkenfeld commented 10 years ago

    Let me check that.

    80036ac5-bb84-4d39-8416-02cd8e51707d commented 10 years ago

    This error is rather related to issue bpo-16042, not issue bpo-16039.

    vstinner commented 5 years ago

    New changeset 16d63202af35dadd652a5e3eae687ea709e95b11 by Victor Stinner in branch '2.7': bpo-16039: CVE-2013-1752: Limit imaplib.IMAP4_SSL.readline() (GH-11120) https://github.com/python/cpython/commit/16d63202af35dadd652a5e3eae687ea709e95b11

    vstinner commented 5 years ago

    I added imaplib.IMAP4_SSL.readline() to my python-security website:

    https://python-security.readthedocs.io/vuln/cve-2013-1752_cve-2013-1752_limit_imaplib.imap4_ssl.readline.html

    I'm now waiting for a Python 2.7.16 release.