classification
Title: xmlrpc: gzip_decode has unlimited read()
Type: resource usage Stage: resolved
Components: Library (Lib) Versions: Python 3.4, Python 3.3, Python 3.1, Python 3.2, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: christian.heimes Nosy List: Arfrever, Jim.Jewett, barry, benjamin.peterson, christian.heimes, doko, flox, georg.brandl, larry, loewis, matejcik, python-dev, schmir, serhiy.storchaka, vadmium
Priority: release blocker Keywords: patch

Created on 2012-09-25 10:52 by christian.heimes, last changed 2014-12-06 01:36 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
xmlrpc_gzip_27.patch christian.heimes, 2013-01-20 15:14
xmlrpc_gzip_27_parameter.patch doko, 2014-12-02 12:39 review
xmlrpc_gzip_27_parameter.patch doko, 2014-12-02 12:54 review
Messages (16)
msg171246 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2012-09-25 10:52
The xmlrpc client library is the only stdlib module that has a gzip decompression handler for compressed HTTP streams. The gzip_decode() function decompresses HTTP bodies that are compressed and sent with Accept-Encoding: x-gzip.

A malicious server can send a specially prepared HTTP request that can consume lots of memory. For example 1 GB of \0 bytes is less than 1 MB of gzip data.

Suggestion:
The gzip_decode() should only decode a sane amount of bytes (for example 50 MB) and raise an exception when more data is to be read.
msg171247 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2012-09-25 10:58
Also see #15955

According to Nadeem it's not (easily) possible to detect how large the output is going to be.
msg180296 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2013-01-20 15:14
The attached patch adds a limitation to xmlrpclib.gzip_decode().
msg182087 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2013-02-14 11:24
IMHO the patch should also limit the maximum amount of read bytes in Transport.parse_response(). Do you agree?
msg182093 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-02-14 12:10
I think instead of global variable it will be better to add an optional parameter for gzip_decode() (with a sane default value) and related functions. Or at least in additional to it.
msg182138 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2013-02-15 12:53
+1 for a keyword argument

I also have to add a limit to GzipDecodedResponse().

Python 2.6 and 3.1 are not affected by the issue. The problematic code was added in 2.7 and 3.2.
msg182203 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2013-02-16 00:08
CVE-2013-1753 gzip bomb and unbound read DoS vulnerabilities in Python's xmlrpc library
msg185061 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2013-03-23 14:46
Not blocking 2.7.4 as discussed on mailing list.
msg196857 - (view) Author: Barry A. Warsaw (barry) * (Python committer) Date: 2013-09-03 18:32
blocker for 2.6.9
msg200345 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2013-10-19 01:20
Ping.  Can we get this fixed before beta 1?
msg207176 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2014-01-02 16:56
Demoting this from release blocker: apparently, the release-blocking property was only intended for 2.6.9, which has been released.
msg213976 - (view) Author: Jim Jewett (Jim.Jewett) Date: 2014-03-18 15:00
I'm putting it back to release blocker, because 3.3 should decide whether to fix it/call it security/remove itself from the list.

The patch contains several small changes.  I like the spelling fix (gsip -> gzip) in a test method, but otherwise, I prefer the alternative solution of an additional function parameter with a default.

I would prefer that the marker for "no limit" be None, rather than -1, 0, or anything less than 0.

I also don't see the point of raising a too-much-data ValueError *after* decoding.  While that *might* mean we set the default too low, all we would really know for sure is that there would be a bug in gzip.GzipFile().read -- and ValueError suggests otherwise.
msg231989 - (view) Author: Matthias Klose (doko) * (Python committer) Date: 2014-12-02 12:39
updated patch to use an optional parameter "max_decode".
msg231992 - (view) Author: Matthias Klose (doko) * (Python committer) Date: 2014-12-02 12:54
document the new exception
msg232231 - (view) Author: Roundup Robot (python-dev) Date: 2014-12-06 01:17
New changeset d50096708b2d by Benjamin Peterson in branch '2.7':
add a default limit for the amount of data xmlrpclib.gzip_decode will return (closes #16043)
https://hg.python.org/cpython/rev/d50096708b2d
msg232235 - (view) Author: Roundup Robot (python-dev) Date: 2014-12-06 01:36
New changeset a0368f81af9a by Benjamin Peterson in branch '3.2':
add a default limit for the amount of data xmlrpclib.gzip_decode will return (closes #16043)
https://hg.python.org/cpython/rev/a0368f81af9a

New changeset 4a9418c6f8ae by Benjamin Peterson in branch '3.3':
merge 3.2 (#16043)
https://hg.python.org/cpython/rev/4a9418c6f8ae

New changeset 6b83e21c8679 by Benjamin Peterson in branch '3.4':
merge 3.3 (#16043)
https://hg.python.org/cpython/rev/6b83e21c8679

New changeset 6f002c4741e2 by Benjamin Peterson in branch 'default':
merge 3.4 (#16043)
https://hg.python.org/cpython/rev/6f002c4741e2
History
Date User Action Args
2014-12-06 01:36:14python-devsetmessages: + msg232235
2014-12-06 01:17:14python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg232231

resolution: fixed
stage: patch review -> resolved
2014-12-02 12:54:20dokosetfiles: + xmlrpc_gzip_27_parameter.patch

messages: + msg231992
2014-12-02 12:39:17dokosetfiles: + xmlrpc_gzip_27_parameter.patch
nosy: + doko
messages: + msg231989

2014-03-26 15:56:22matejciksetnosy: + matejcik
2014-03-18 15:00:45Jim.Jewettsetpriority: critical -> release blocker
nosy: + Jim.Jewett
messages: + msg213976

2014-01-02 16:56:38loewissetpriority: release blocker -> critical
nosy: + loewis
messages: + msg207176

2013-10-19 01:20:09larrysetmessages: + msg200345
2013-09-15 19:46:57Arfreversetversions: + Python 3.1
2013-09-15 15:45:15barrysetversions: - Python 2.6
2013-09-03 18:32:44barrysetpriority: critical -> release blocker

messages: + msg196857
2013-06-17 06:34:45vadmiumsetnosy: + vadmium
2013-03-23 14:46:10benjamin.petersonsetpriority: release blocker -> critical

messages: + msg185061
2013-02-22 23:32:27Arfreversetnosy: + Arfrever
2013-02-22 13:00:14floxsetnosy: + flox
2013-02-20 22:27:27barrysetnosy: + barry

versions: + Python 2.6
2013-02-16 00:08:01christian.heimessetmessages: + msg182203
2013-02-15 12:53:50christian.heimessetmessages: + msg182138
2013-02-14 12:10:37serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg182093
2013-02-14 11:24:29christian.heimessetmessages: + msg182087
2013-02-04 17:12:49christian.heimessetpriority: critical -> release blocker
nosy: + georg.brandl, larry, benjamin.peterson
2013-01-20 15:14:03christian.heimessetfiles: + xmlrpc_gzip_27.patch
priority: normal -> critical
dependencies: - gzip, bz2, lzma: add option to limit output size

assignee: christian.heimes
versions: + Python 3.4
keywords: + patch
messages: + msg180296
stage: patch review
2012-09-25 10:58:41christian.heimessetdependencies: + gzip, bz2, lzma: add option to limit output size
messages: + msg171247
2012-09-25 10:53:04schmirsetnosy: + schmir
2012-09-25 10:52:06christian.heimescreate