Repository navigation
zlib crc32/adler32 buffer length truncation (64-bit) #54485
Description
| BPO | 10276 |
|---|---|
| Nosy | @loewis, @pitrou |
| 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 2011-05-03.21:17:34.451>
created_at = <Date 2010-11-01.09:46:03.865>
labels = ['extension-modules', 'type-bug']
title = 'zlib crc32/adler32 buffer length truncation (64-bit)'
updated_at = <Date 2016-05-27.02:41:08.931>
user = 'https://bugs.python.org/nadeemvawda'bugs.python.org fields:
activity = <Date 2016-05-27.02:41:08.931>
actor = 'martin.panter'
assignee = 'none'
closed = True
closed_date = <Date 2011-05-03.21:17:34.451>
closer = 'nadeem.vawda'
components = ['Extension Modules']
creation = <Date 2010-11-01.09:46:03.865>
creator = 'nadeem.vawda'
dependencies = []
files = ['19453', '20543']
hgrepos = []
issue_num = 10276
keywords = ['patch']
message_count = 8.0
messages = ['120114', '120116', '127159', '128959', '128977', '135036', '135042', '135072']
nosy_count = 5.0
nosy_names = ['loewis', 'pitrou', 'nadeem.vawda', 'sdaoden', 'python-dev']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue10276'
versions = ['Python 2.7']
Activity
nadeemvawda commented on Nov 1, 2010
zlib.crc32() and zlib.adler32() in Modules/zlibmodule.c don't handle buffers of >=4GB correctly. The length of a Py_buffer is of type Py_ssize_t, while the C zlib functions take length as an unsigned integer. This means that on a 64-bit build, the buffer length gets silently truncated to 32 bits, which results in incorrect output for large inputs.
Attached is a patch that fixes this by computing the checksum incrementally, using small-enough chunks of the buffer.
A better fix might be to have Modules/zlib/crc32.c use 64-bit lengths. I tried this, but I couldn't get it to work. It seems that if the system already has zlib installed, Python will link against the existing version instead of compiling its own.
Testing this might be a bit tricky. Allocating a 4+GB regular buffer isn't practical. Using a memory-mapped file would work, but I'm not sure having a unit test create a multi-gigabyte file is a great thing to do.
loewis commented on Nov 1, 2010
I find your approach fine; there isn't a need (IMO) to have the underlying functions change.
nadeemvawda commented on Jan 26, 2011
Here is an update patch, which corrects a typo in the previous patch, and adds a test to test_zlib.
The test uses a memory-mapped sparse file, so it gets skipped on systems without mmap. The alternative would be to allocate a 4+GB buffer of ordinary memory, causes heavy swapping on my machine (4GB of RAM). The test also gets skipped on 32-bit builds, where the address space is too small for this bug to arise.
I'm not sure whether the test can count on the created file actually being sparse, so I had the test require the 'largefile' resource, to be on the safe side.
pitrou commented on Feb 21, 2011
Patch looks good to me.
pitrou commented on Feb 21, 2011
Thank you for the patch! Committed in r88460 (3.3) and r88461 (3.2).
2.7 would need more surgery in order for this to be fixed, see bpo-8651 and bpo-8650.
python-dev commented on May 3, 2011
New changeset f43213129ba8 by Victor Stinner in branch '2.7':
Issue bpo-10276: test_zlib checks that inputs of 2 GB are handled correctly by
http://hg.python.org/cpython/rev/f43213129ba8
python-dev commented on May 3, 2011
New changeset dd58f8072216 by Victor Stinner in branch '2.7':
Issue bpo-10276: Fix test_zlib, m may be undefined in the finally block
http://hg.python.org/cpython/rev/dd58f8072216
nadeemvawda commented on May 3, 2011
Changeset a0681e7a6ded fixes this bug for 2.7 - too-large buffers cause
an OverflowError during argument parsing, so there is no possibility of
truncation happening.