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

httplib: header parsing is unlimited #60241

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

httplib: header parsing is unlimited #60241

tiran opened this issue Sep 25, 2012 · 31 comments
Assignees
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 16037
Nosy @warsaw, @birkenfeld, @terryjreedy, @pitrou, @larryhastings, @tiran, @benjaminp, @berkerpeksag, @Lukasa
Files
  • issue16037_py27.patch
  • issue16037_py32.patch
  • issue16037_py26.patch
  • issue16037_py27_v2.patch
  • issue16037_py32_v2.patch
  • issue16037_py32_v3.patch
  • issue16037_py27_v3.diff
  • 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-09-30.12:50:04.031>
    created_at = <Date 2012-09-25.10:25:22.415>
    labels = ['release-blocker', 'library', 'performance']
    title = 'httplib: header parsing is unlimited'
    updated_at = <Date 2014-09-30.13:17:53.186>
    user = 'https://github.com/tiran'

    bugs.python.org fields:

    activity = <Date 2014-09-30.13:17:53.186>
    actor = 'berker.peksag'
    assignee = 'christian.heimes'
    closed = True
    closed_date = <Date 2014-09-30.12:50:04.031>
    closer = 'georg.brandl'
    components = ['Library (Lib)']
    creation = <Date 2012-09-25.10:25:22.415>
    creator = 'christian.heimes'
    dependencies = []
    files = ['29201', '29203', '31581', '31582', '31583', '32358', '36214']
    hgrepos = []
    issue_num = 16037
    keywords = ['patch']
    message_count = 31.0
    messages = ['171240', '171250', '171251', '171258', '182194', '182803', '182805', '185055', '187276', '196862', '196898', '198610', '198618', '198619', '198620', '198621', '200349', '201162', '201255', '201424', '201429', '213240', '222210', '224568', '224802', '224803', '226078', '226079', '226110', '226112', '227890']
    nosy_count = 14.0
    nosy_names = ['barry', 'georg.brandl', 'terry.reedy', 'pitrou', 'larry', 'christian.heimes', 'benjamin.peterson', 'Arfrever', 'BreamoreBoy', 'nailor', 'python-dev', 'berker.peksag', 'puppet', 'Lukasa']
    pr_nums = []
    priority = 'release blocker'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'resource usage'
    url = 'https://bugs.python.org/issue16037'
    versions = ['Python 3.2']

    @tiran
    Copy link
    Member Author

    tiran commented Sep 25, 2012

    The httplib module / package can read arbitrary amounts of data from its socket when it's parsing the HTTP header. This may lead to issues when a user connects to a broken HTTP server or something that isn't a HTTP at all. The issue can be broken up into two parts: parsing the HTTP status line parsing and parsing the remaining HTTP headers.

    Reading and parsing of the HTTP status line is already limited in Python 3.x. Python 2.7 and lower may read arbitrary amounts of bytes from the socket until it finds a newline char. The small patch below is a backport of the Python 3.x behavior to 2.7:

    --- a/Lib/httplib.py
    +++ b/Lib/httplib.py
    @@ -362,7 +362,9 @@

         def _read_status(self):
             # Initialize with Simple-Response defaults
    -        line = self.fp.readline()
    +        line = self.fp.readline(_MAXLINE + 1)
    +        if len(line) > _MAXLINE:
    +            raise LineTooLong("header line")
             if self.debuglevel > 0:
                 print "reply:", repr(line)
             if not line:

    Both Python 2 and Python 3 accept an unlimited count of HTTP headers with a maximum length of 64k each. As headers are accumulated in an list it may consume lots of memory. I suggest that we limit the maximum amount of HTTP header lines to a sane value. How does 100 sound to you?

    @tiran tiran added stdlib Python modules in the Lib dir performance Performance or resource usage labels Sep 25, 2012
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Sep 25, 2012

    New changeset 8a22a2804a66 by Christian Heimes in branch '2.7':
    Issue bpo-16037: Limit httplib's _read_status() function to work around broken
    http://hg.python.org/cpython/rev/8a22a2804a66

    @tiran
    Copy link
    Member Author

    tiran commented Sep 25, 2012

    The readline() limitation in _read_status() was added at some point in the 3.2 line. Python 3.1 has an unlimited readline().

    @pitrou
    Copy link
    Member

    pitrou commented Sep 25, 2012

    100 headers sounds more than enough for everybody.

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

    tiran commented Feb 15, 2013

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

    @nailor
    Copy link
    Mannequin

    nailor mannequin commented Feb 23, 2013

    Here's a patch that limits the headers to 100. If more than _MAXHEADERS headers are read, this raises exception TooMuchHeaders.

    The patch is for 2.7, I'll cook one for 3.2 too.

    @nailor
    Copy link
    Mannequin

    nailor mannequin commented Feb 23, 2013

    ...and here's the patch for 3.2

    @benjaminp
    Copy link
    Contributor

    Not blocking 2.7.4 as discussed on mailing list.

    @BreamoreBoy
    Copy link
    Mannequin

    BreamoreBoy mannequin commented Apr 18, 2013

    Patches LGTM but I suggest TooManyHeaders instead of TooMuchHeaders. I've tried the 3.2 patch against the latest default repo on Windows Vista and it applies cleanly. All tests passed so looks as if this could be committed.

    @warsaw
    Copy link
    Member

    warsaw commented Sep 3, 2013

    blocker for 2.6.9

    @nailor
    Copy link
    Mannequin

    nailor mannequin commented Sep 4, 2013

    Reworded TooMuch to TooMany and made a patch for 2.6 too (2.7 didn't apply cleanly there)

    @Arfrever Arfrever mannequin changed the title httplib: header parsing is not delimited httplib: header parsing is not unlimited Sep 15, 2013
    @warsaw
    Copy link
    Member

    warsaw commented Sep 29, 2013

    As we discussed in other issues regarding the similar problem, I don't really want to introduce a new exception in a point release of 2.6. Is there any reason not to just raise HTTPException with the error message text? Code that has to work across multiple 2.6.X versions won't be able to import the new exception, and thus cannot rely on it anyway.

    If you agree, I'll make that change when I apply this patch.

    @nailor
    Copy link
    Mannequin

    nailor mannequin commented Sep 29, 2013

    I'm fine with not introducing a new exception for 2.6 (or any other version for that matter), so go for it :)

    @warsaw
    Copy link
    Member

    warsaw commented Sep 29, 2013

    I'm just going to go ahead and commit this patch to 2.6 with the change I mentioned. Does anything else need to be done for 2.6?

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Sep 29, 2013

    New changeset 582e5072ff89 by Barry Warsaw in branch '2.6':

    @warsaw
    Copy link
    Member

    warsaw commented Sep 29, 2013

    Thanks!

    @Arfrever Arfrever mannequin changed the title httplib: header parsing is not unlimited httplib: header parsing is unlimited Sep 29, 2013
    @larryhastings
    Copy link
    Contributor

    Ping. Please fix before "beta 1".

    @nailor
    Copy link
    Mannequin

    nailor mannequin commented Oct 24, 2013

    Patch for py32 applies cleanly on 3.4 too, this should be good to go

    @nailor
    Copy link
    Mannequin

    nailor mannequin commented Oct 25, 2013

    Third version of the 3.2 patch, this time with documentation of the exception TooManyHeaders

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Oct 27, 2013

    New changeset e445d02e5306 by Georg Brandl in branch '3.3':
    Issue bpo-16037: HTTPMessage.readheaders() raises an HTTPException when more than
    http://hg.python.org/cpython/rev/e445d02e5306

    @birkenfeld
    Copy link
    Member

    Also merged to default.

    @Lukasa
    Copy link
    Mannequin

    Lukasa mannequin commented Mar 12, 2014

    I presume Barry's disinclination to merge this to 2.6 with a new exception applies equally to 2.7, which is why this hasn't been merged to 2.7 yet?

    I'm happy to review an updated 2.7 patch that raises an HTTPException if that's what we need to keep this moving.

    @BreamoreBoy
    Copy link
    Mannequin

    BreamoreBoy mannequin commented Jul 3, 2014

    Is any further work needed on this and similar issues bpo-16038, bpo-16040, bpo-16041, bpo-16042 and bpo-16043 ?

    @puppet
    Copy link
    Mannequin

    puppet mannequin commented Aug 2, 2014

    Updated the patch for 2.7 to raise HTTPException instead of a new Exception.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Aug 5, 2014

    New changeset 5e310c6a8520 by Berker Peksag in branch '2.7':
    Issue bpo-16037: HTTPMessage.readheaders() raises an HTTPException when more
    http://hg.python.org/cpython/rev/5e310c6a8520

    @berkerpeksag
    Copy link
    Member

    Thanks for the patches Jyrki and Daniel.

    @terryjreedy
    Copy link
    Member

    Looking further, already fixed in 3.x

    @Arfrever
    Copy link
    Mannequin

    Arfrever mannequin commented Aug 29, 2014

    Python 3.2 still receives security fixes.

    @Arfrever Arfrever mannequin reopened this Aug 29, 2014
    @terryjreedy
    Copy link
    Member

    This was never discussed as a security issue. Why do you think it is? Users wasting their *own* time is different for wasting the time of a remote server in a DoS attack.

    @pitrou
    Copy link
    Member

    pitrou commented Aug 30, 2014

    A server can include a HTTP client. It's actually quite common these days, given the number of services which are exposed as REST APIs.
    Now, unless Georg plans to do a new 3.2 release some day, it's not very useful to discuss the inclusion of the fix in 3.2.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Sep 30, 2014

    New changeset deee87d61436 by Georg Brandl in branch '3.2':
    Issue bpo-16037: HTTPMessage.readheaders() raises an HTTPException when more than
    https://hg.python.org/cpython/rev/deee87d61436

    @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
    Labels
    performance Performance or resource usage release-blocker stdlib Python modules in the Lib dir
    Projects
    None yet
    Development

    No branches or pull requests

    8 participants