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

Created on 2012-09-25 10:52 by christian.heimes, last changed 2014-03-26 15:56 by matejcik.

Files
File name Uploaded Description Edit
xmlrpc_gzip_27.patch christian.heimes, 2013-01-20 15:14
Messages (12)
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.
History
Date User Action Args
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