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

zlib crc32/adler32 buffer length truncation (64-bit) #54485

Closed
nadeemvawda mannequin opened this issue Nov 1, 2010 · 8 comments
Closed

zlib crc32/adler32 buffer length truncation (64-bit) #54485

nadeemvawda mannequin opened this issue Nov 1, 2010 · 8 comments
Labels
extension-modules C modules in the Modules dir type-bug An unexpected behavior, bug, or error

Comments

@nadeemvawda
Copy link
Mannequin

nadeemvawda mannequin commented Nov 1, 2010

BPO 10276
Nosy @loewis, @pitrou
Files
  • zlib-checksum-truncation.diff: Calculate checksums incrementally for large buffers.
  • zlib-v2.diff: Updated fix, with test.
    		•

    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']
    @nadeemvawda
    Copy link
    Mannequin Author

    nadeemvawda mannequin commented 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.

    @nadeemvawda nadeemvawda mannequin added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Nov 1, 2010
    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Nov 1, 2010

    I find your approach fine; there isn't a need (IMO) to have the underlying functions change.

    @merwok merwok added extension-modules C modules in the Modules dir and removed stdlib Python modules in the Lib dir labels Nov 1, 2010
    @nadeemvawda
    Copy link
    Mannequin Author

    nadeemvawda mannequin commented 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
    Copy link
    Member

    pitrou commented Feb 21, 2011

    Patch looks good to me.

    @pitrou
    Copy link
    Member

    pitrou commented 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
    Copy link
    Mannequin

    python-dev mannequin commented 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
    Copy link
    Mannequin

    python-dev mannequin commented 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
    Copy link
    Mannequin Author

    nadeemvawda mannequin commented 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.

    @nadeemvawda nadeemvawda mannequin closed this as completed May 3, 2011
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @ned-deily ned-deily mentioned this issue Jun 22, 2023
    Closed
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    extension-modules C modules in the Modules dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    2 participants
    @merwok @pitrou