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.

Sign up for GitHub

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

CGI DOS vulnerability via long post list #79047

Closed
matthewbelisle-wf mannequin opened this issue Oct 1, 2018 · 11 comments
Closed

CGI DOS vulnerability via long post list #79047

matthewbelisle-wf mannequin opened this issue Oct 1, 2018 · 11 comments
Labels
3.7 (EOL) end of life 3.8 only security fixes stdlib Python modules in the Lib dir type-security A security issue

Comments

@matthewbelisle-wf
Copy link
Mannequin

matthewbelisle-wf mannequin commented Oct 1, 2018

BPO 34866
Nosy @vstinner, @miss-islington, @tirkarthi, @matthewbelisle-wf
PRs
  • bpo-34866: Adding max_num_fields to cgi.FieldStorage #9660
  • [3.7] bpo-34866: Adding max_num_fields to cgi.FieldStorage (GH-9660) #9965
  • [3.6] bpo-34866: Adding max_num_fields to cgi.FieldStorage (GH-9660) #9966
  • [2.7] bpo-34866: Adding max_num_fields to cgi.FieldStorage (GH-9660) #9969
    		•
    Files
  • example.py
    		•

    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 = None
closed_at = <Date 2018-10-30.21:30:20.920>
created_at = <Date 2018-10-01.21:23:27.958>
labels = ['type-security', '3.8', '3.7', 'library']
title = 'CGI DOS vulnerability via long post list'
updated_at = <Date 2018-10-30.21:30:20.918>
user = 'https://github.com/matthewbelisle-wf'

    bugs.python.org fields:

    activity = <Date 2018-10-30.21:30:20.918>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2018-10-30.21:30:20.920>
closer = 'vstinner'
components = ['Library (Lib)']
creation = <Date 2018-10-01.21:23:27.958>
creator = 'Matthew Belisle'
dependencies = []
files = ['47861']
hgrepos = []
issue_num = 34866
keywords = ['patch']
message_count = 11.0
messages = ['326831', '327476', '328036', '328037', '328038', '328401', '328402', '328950', '328951', '328953', '328954']
nosy_count = 4.0
nosy_names = ['vstinner', 'miss-islington', 'xtreak', 'Matthew Belisle']
pr_nums = ['9660', '9965', '9966', '9969']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'security'
url = 'https://bugs.python.org/issue34866'
versions = ['Python 2.7', 'Python 3.6', 'Python 3.7', 'Python 3.8']
    @matthewbelisle-wf
    Copy link
    Mannequin Author

    matthewbelisle-wf mannequin commented Oct 1, 2018

    Copied from email to security@python.org:

    I have been doing memory profiling on a few python web frameworks and I noticed this issue in the cgi.FieldStorage class.

    $ python example.py
Memory used: 523935744 bytes

    The problem is there is no easy way to limit the number of MiniFieldStorage objects created by FieldStorage, so it goes unchecked in many frameworks like pyramid, pylons, webapp2, and flask. The end result is that on these frameworks, a 9MB request body (gzipped down to 9KB) can chew up ~500MB of memory on the server which is enough to effectively DOS it. The obvious way to prevent this currently is to check the content-length header and fail if it exceeds some value. But that solution has a major shortcoming because many frameworks want to allow large payloads, sometimes up to 10MB, as long as they contain a reasonable number of fields.

    After talking with the security@python.org
    team and pylons dev team about it, we think the best solution is to add a max_num_fields param to the FieldStorage class, defaulting to None, which throws an error if max_num_fields is exceeded.

    @matthewbelisle-wf matthewbelisle-wf mannequin added 3.8 only security fixes 3.7 (EOL) end of life stdlib Python modules in the Lib dir type-security A security issue labels Oct 1, 2018
    @matthewbelisle-wf
    Copy link
    Mannequin Author

    matthewbelisle-wf mannequin commented Oct 10, 2018

    Sorry, looks like I forgot to attach example.py. Attaching now.

    @miss-islington
    Copy link
    Contributor

    New changeset 2091448 by Miss Islington (bot) (matthewbelisle-wf) in branch 'master':
    bpo-34866: Adding max_num_fields to cgi.FieldStorage (GH-9660)
    2091448

    @miss-islington
    Copy link
    Contributor

    New changeset a66f279 by Miss Islington (bot) in branch '3.7':
    bpo-34866: Adding max_num_fields to cgi.FieldStorage (GH-9660)
    a66f279

    @miss-islington
    Copy link
    Contributor

    New changeset 322a914 by Miss Islington (bot) in branch '3.6':
    bpo-34866: Adding max_num_fields to cgi.FieldStorage (GH-9660)
    322a914

    @vstinner
    Copy link
    Member

    2091448

    This commit adds a new max_num_fields=None parameter to FieldStorage, parse_qs() and parse_qsl(): you must update the documentation in Doc/library/ as well.

    @vstinner
    Copy link
    Member

    For 3.7 an 3.6 changes, you have to specify the minor Python version (3.7.x and 3.6.x) in which the change has been introduce. Same comment for Python 2.7.

    @vstinner
    Copy link
    Member

    New changeset bc6f74a by Victor Stinner (matthewbelisle-wf) in branch '2.7':
    bpo-34866: Add max_num_fields to cgi.FieldStorage (GH-9660) (GH-9969)
    bc6f74a

    @vstinner
    Copy link
    Member

    I suggest to not add the new parameter to 3.4 and 3.5 branches, even if it's a security fix. The fix requires to *use* the parameter, and I don't expect applications on Python 3.4 and 3.5 to be modified to use it.

    @matthewbelisle-wf
    Copy link
    Mannequin Author

    matthewbelisle-wf mannequin commented Oct 30, 2018

    That makes sense Victor, I agree. Thanks for merging those PRs.

    @vstinner
    Copy link
    Member

    Thanks Matthew Belisle for the nice security counter-measure!

    @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
    No one assigned
    Labels
    3.7 (EOL) end of life 3.8 only security fixes stdlib Python modules in the Lib dir type-security A security issue
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    2 participants
    @vstinner @miss-islington