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

Jump to bottom

nntplib: unlimited readline() from connection #60244

Closed
tiran opened this issue Sep 25, 2012 · 23 comments
Closed

nntplib: unlimited readline() from connection #60244

tiran opened this issue Sep 25, 2012 · 23 comments
Assignees
@tiran
Labels
performance Performance or resource usage release-blocker stdlib Python modules in the Lib dir

Comments

@tiran
Copy link
Member

tiran commented Sep 25, 2012

BPO 16040
Nosy @warsaw, @birkenfeld, @terryjreedy, @larryhastings, @giampaolo, @tiran, @benjaminp, @hynek
Files
  • issue16040_py26.patch
  • issue16040_py26_v2.patch
  • issue16040_py32.patch
    		•

    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:

    assignee = 'https://github.com/tiran'
closed_at = <Date 2014-08-30.03:54:57.815>
created_at = <Date 2012-09-25.10:38:44.908>
labels = ['release-blocker', 'library', 'performance']
title = 'nntplib: unlimited readline() from connection'
updated_at = <Date 2014-10-12.07:17:06.930>
user = 'https://github.com/tiran'

    bugs.python.org fields:

    activity = <Date 2014-10-12.07:17:06.930>
actor = 'python-dev'
assignee = 'christian.heimes'
closed = True
closed_date = <Date 2014-08-30.03:54:57.815>
closer = 'terry.reedy'
components = ['Library (Lib)']
creation = <Date 2012-09-25.10:38:44.908>
creator = 'christian.heimes'
dependencies = []
files = ['31927', '31928', '32339']
hgrepos = []
issue_num = 16040
keywords = ['patch']
message_count = 23.0
messages = ['171243', '172291', '182190', '182197', '185059', '196859', '197781', '198739', '198740', '198741', '198742', '198745', '198746', '198777', '198791', '200351', '201172', '201425', '201428', '222501', '222625', '226118', '229122']
nosy_count = 12.0
nosy_names = ['barry', 'georg.brandl', 'terry.reedy', 'larry', 'giampaolo.rodola', 'christian.heimes', 'benjamin.peterson', 'Arfrever', 'nailor', 'python-dev', 'francismb', 'hynek']
pr_nums = []
priority = 'release blocker'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'resource usage'
url = 'https://bugs.python.org/issue16040'
versions = ['Python 3.1', 'Python 3.2']
    @tiran
    Copy link
    Member Author

    tiran commented Sep 25, 2012

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

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

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

    @tiran tiran added stdlib Python modules in the Lib dir performance Performance or resource usage labels Sep 25, 2012
    @hynek
    Copy link
    Member

    hynek commented Oct 7, 2012

    Any suggestions on the value for _MAXLINE or just steal the 64k from httplib?

    @tiran tiran self-assigned this Jan 20, 2013
    @tiran
    Copy link
    Member Author

    tiran commented Feb 15, 2013

    RFC 3977 specifies:

    Command lines MUST NOT exceed 512 octets, which includes
    the terminating CRLF pair.

    However NNTP also have multi-line data blocks. The RFC says nothing about the maximum length of a data line. We may need two limits here, one for command lines (2048 perhaps) and one much larger for data lines (a couple of MB?).

    Can somebody check other implementations?

    @tiran
    Copy link
    Member Author

    tiran commented Feb 15, 2013

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

    @benjaminp
    Copy link
    Contributor

    Not blocking 2.7.4 as discussed on mailing list.

    @warsaw
    Copy link
    Member

    warsaw commented Sep 3, 2013

    blocker for 2.6.9

    @warsaw
    Copy link
    Member

    warsaw commented Sep 15, 2013

    Any more thoughts on this bug w.r.t. 2.6.9? It seems that without a patch for any version of Python, and with 2.6.9 coming soon, a fix for this just won't make it into 2.6.9.

    That doesn't bother me too much, and I'm willing to just knock this off the 2.6.9 radar unless objections (accompanied by patches? :) are raised.

    @nailor
    Copy link
    Mannequin

    nailor mannequin commented Sep 30, 2013

    Regarding the implementation: all commands (even those returning multiple lines), use the same readline method.

    I've attached a patch for 2.6, working on the 2.7+ too.

    @warsaw
    Copy link
    Member

    warsaw commented Sep 30, 2013

    Looks great, thanks! I'll apply this to 2.6.9 but let others forward port it to 2.7.

    @nailor
    Copy link
    Mannequin

    nailor mannequin commented Sep 30, 2013

    The patch for 2.6 applies cleanly on 2.7 too and the tests pass there

    @nailor
    Copy link
    Mannequin

    nailor mannequin commented Sep 30, 2013

    Did a slight change to the patch, making the too long line to look like a valid line so that it does not raise a NNTPProtocolError otherwise. Thanks to Barry for catching this :)

    I also wonder if there should be data error risen instead? Current docstrings of the errors are not that well fit.

    @warsaw
    Copy link
    Member

    warsaw commented Sep 30, 2013

    On Sep 30, 2013, at 09:43 PM, Jyrki Pulliainen wrote:

    I also wonder if there should be data error risen instead? Current docstrings
    of the errors are not that well fit.

    I guess a data error makes the least nonsense here, so I'll change it over to
    that. I'm happy to entertain other thoughts (except for introducing a new
    exception of course) before 2.6.9 final.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Sep 30, 2013

    New changeset 731abf7834c4 by Barry Warsaw in branch '2.6':

    New changeset 36680a7c0e22 by Barry Warsaw in branch '2.7':

    @Arfrever
    Copy link
    Mannequin

    Arfrever mannequin commented Oct 1, 2013

    New changeset 731abf7834c4 by Barry Warsaw in branch '2.6':

    New changeset 36680a7c0e22 by Barry Warsaw in branch '2.7':

    s/lenght/length/ in new comment in Lib/nntplib.py

    @warsaw
    Copy link
    Member

    warsaw commented Oct 1, 2013

    On Oct 01, 2013, at 01:44 PM, Arfrever Frehtes Taifersar Arahesis wrote:

    s/lenght/length/ in new comment in Lib/nntplib.py

    Fixed, thanks.

    @larryhastings
    Copy link
    Contributor

    Ping. Please fix before "beta 1".

    @nailor
    Copy link
    Mannequin

    nailor mannequin commented Oct 24, 2013

    ...and here's a patch for 3.2

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Oct 27, 2013

    New changeset fc88bd80d925 by Georg Brandl in branch '3.3':
    Issue bpo-16040: CVE-2013-1752: nntplib: Limit maximum line lengths to 2048 to
    http://hg.python.org/cpython/rev/fc88bd80d925

    @birkenfeld
    Copy link
    Member

    Also merged to default.

    @francismb
    Copy link
    Mannequin

    francismb mannequin commented Jul 7, 2014

    Just a small detail on the patches, they seem to have a typo
    (lenght vs. length) on the line:

    > reading arbitrary lenght lines. RFC 3977 limits NNTP line length to

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jul 9, 2014

    New changeset 5be778fec115 by Berker Peksag in branch '3.4':
    Issues bpo-21948 and bpo-16040: Fix typos.
    http://hg.python.org/cpython/rev/5be778fec115

    New changeset 051cc4f60384 by Berker Peksag in branch 'default':
    Issues bpo-21948 and bpo-16040: Merge with 3.4.
    http://hg.python.org/cpython/rev/051cc4f60384

    @terryjreedy
    Copy link
    Member

    3.1 is finished and Georg decided to skip 3.2.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Oct 12, 2014

    New changeset 985bda4edf9d by Georg Brandl in branch '3.2':
    bpo-16040: fix unlimited read from connection in nntplib.
    https://hg.python.org/cpython/rev/985bda4edf9d

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees

    @tiran tiran

    Labels
    performance Performance or resource usage release-blocker stdlib Python modules in the Lib dir
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    7 participants
    @hynek @birkenfeld @warsaw @benjaminp @tiran @larryhastings @terryjreedy