Repository navigation
zlib.Decompress.decompress() retains pointer to input buffer without acquiring reference to it #60615
Description
| BPO | 16411 |
|---|---|
| Nosy | @serhiy-storchaka |
| Files |
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 2012-11-11.02:37:51.405>
created_at = <Date 2012-11-05.00:29:24.235>
labels = ['type-bug']
title = 'zlib.Decompress.decompress() retains pointer to input buffer without acquiring reference to it'
updated_at = <Date 2012-11-11.02:37:51.335>
user = 'https://bugs.python.org/nadeemvawda'bugs.python.org fields:
activity = <Date 2012-11-11.02:37:51.335>
actor = 'nadeem.vawda'
assignee = 'none'
closed = True
closed_date = <Date 2012-11-11.02:37:51.405>
closer = 'nadeem.vawda'
components = []
creation = <Date 2012-11-05.00:29:24.235>
creator = 'nadeem.vawda'
dependencies = []
files = ['27889', '27891']
hgrepos = []
issue_num = 16411
keywords = ['patch']
message_count = 5.0
messages = ['174853', '174889', '175310', '175311', '175312']
nosy_count = 3.0
nosy_names = ['nadeem.vawda', 'python-dev', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue16411'
versions = ['Python 2.7', 'Python 3.2', 'Python 3.3', 'Python 3.4']
Activity
nadeemvawda commentedon Nov 5, 2012
When calling zlib.Decompress.decompress() with a max_length argument,
if the input data is not full consumed, the next_in pointer in the
z_stream struct are left pointing into the data object, but the
decompressor does not hold a reference to this object. This same
pointer is reused (perhaps unintentionally) if flush() is called
without calling decompress() again.
If the data object gets deallocated between the calls to decompress()
and to flush(), zlib will then try to access this deallocated memory,
and most likely return bogus output (or segfault). See the attached
script for a demonstration.
I see two potential solutions:
Set avail_in to zero in flush(), so that it does not try to use
leftover data (or whatever is else where that data used to be).
Have decompress() check if there is leftover data, and if so,
save a reference to the object until a) we consume the rest of
the data in flush(), or b) discard it in a subsequent call to
decompress().
Solution 2 would be less disruptive to code that depends on the existing
behavior (in non-pathological cases), but I'm don't like the maintenance
burden of adding yet another thing to keep track of to the decompressor
state. The PyZlib_objdecompress function is complex enough as it is, and
we can expect more bugs like this to creep in the more we cram additional
logic into it. So I'm more in favor of solution 1.
Any thoughts?
serhiy-storchaka commentedon Nov 5, 2012
The decompressor does not hold a reference to the data object, but it holds a reference to the data. It's the unconsumed_tail attribute.
The patch is simple.
python-dev commentedon Nov 11, 2012
New changeset c3828831861c by Nadeem Vawda in branch '2.7':
Issue bpo-16411: Fix a bug where zlib.decompressobj().flush() might try to access previously-freed memory.
http://hg.python.org/cpython/rev/c3828831861c
New changeset a1db815d0829 by Nadeem Vawda in branch '3.2':
Issue bpo-16411: Fix a bug where zlib.decompressobj().flush() might try to access previously-freed memory.
http://hg.python.org/cpython/rev/a1db815d0829
New changeset a7934fe2927e by Nadeem Vawda in branch '3.3':
Issue bpo-16411: Fix a bug where zlib.decompressobj().flush() might try to access previously-freed memory.
http://hg.python.org/cpython/rev/a7934fe2927e
New changeset d63c751e9f01 by Nadeem Vawda in branch 'default':
Issue bpo-16411: Fix a bug where zlib.decompressobj().flush() might try to access previously-freed memory.
http://hg.python.org/cpython/rev/d63c751e9f01
python-dev commentedon Nov 11, 2012
New changeset be40a10d553a by Nadeem Vawda in branch '2.7':
Fix typo in backporting fix of issue bpo-16411 to 2.7.
http://hg.python.org/cpython/rev/be40a10d553a
nadeemvawda commentedon Nov 11, 2012
Ah, that's much nicer than either of my ideas. Patch committed. Thanks!