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 module is not completly 64-bit safe #62494

Closed
vstinner opened this issue Jun 24, 2013 · 13 comments
Closed

zlib module is not completly 64-bit safe #62494

vstinner opened this issue Jun 24, 2013 · 13 comments
Labels
extension-modules C modules in the Modules dir type-bug An unexpected behavior, bug, or error

Comments

@vstinner
Copy link
Member

BPO 18294
Nosy @loewis, @vstinner, @serhiy-storchaka
Files
  • zlib_64bit.patch
  • zlib_64bit-2.patch
  • zlib_64bit-3.patch
  • zlib_64bit-4.patch
    		•

    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 2013-11-21.21:38:50.757>
created_at = <Date 2013-06-24.21:04:42.921>
labels = ['extension-modules', 'type-bug']
title = 'zlib module is not completly 64-bit safe'
updated_at = <Date 2014-01-03.11:27:16.910>
user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2014-01-03.11:27:16.910>
actor = 'python-dev'
assignee = 'none'
closed = True
closed_date = <Date 2013-11-21.21:38:50.757>
closer = 'python-dev'
components = ['Extension Modules']
creation = <Date 2013-06-24.21:04:42.921>
creator = 'vstinner'
dependencies = []
files = ['30693', '30699', '32710', '32715']
hgrepos = []
issue_num = 18294
keywords = ['patch']
message_count = 13.0
messages = ['191799', '191800', '191802', '191852', '191855', '202977', '202978', '203437', '203453', '203460', '203674', '203675', '207222']
nosy_count = 5.0
nosy_names = ['loewis', 'vstinner', '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/issue18294'
versions = ['Python 2.7', 'Python 3.3', 'Python 3.4']
    @vstinner
    Copy link
    Member Author

    Attached patch fixes different compiler warnings on Windows x64 in the zlib module. The module raises OverflowError if some values are longer than INT_MAX, but not all parameters are checked.

    @vstinner
    Copy link
    Member Author

    Related issue: bpo-9566.

    @vstinner
    Copy link
    Member Author

    The changeset 931e1bc090f6 fixes warnings in adler32 and crc32.

    @serhiy-storchaka serhiy-storchaka added extension-modules C modules in the Modules dir type-bug An unexpected behavior, bug, or error labels Jun 25, 2013
    @vstinner
    Copy link
    Member Author

    @serhiy: Oh, thanks for your review. My patch was far from being perfect, a review was necessary! (I hesitated to commit it directly.)

    Here is a new patch using unsigned int is most places, parsing arguments with "I" format, and explicit cast to (size_t) to not compare unsigned with signed.

    @vstinner
    Copy link
    Member Author

    "Here is a new patch using unsigned int is most places, parsing
    arguments with "I" format, and explicit cast to (size_t) to not
    compare unsigned with signed."

    Oh oh, it's still wrong. The "I" parser format does not check for
    integer overflow. _PyBytes_Resize() also uses "Py_DECREF(retVal);
    retVal = NULL;" on failure, instead of Py_CLEAR(retVal), whereas
    _PyBytes_Resize(&retVal, ...) sets retVal to NULL...

    @vstinner
    Copy link
    Member Author

    Ping myself.

    @vstinner
    Copy link
    Member Author

    The "I" parser format does not check for integer overflow.

    The "O" format can be used with _PyLong_AsInt() instead.

    @vstinner
    Copy link
    Member Author

    zlib_64bit-3.patch: updated patch which fixes also the PyArg_ParseTuple() convert for C int and C unsigned int.

    @vstinner
    Copy link
    Member Author

    zlib_64bit-4.patch:

    • fix usage of argument clinic, add a uint_converter
    • fix zlib_Decompress_decompress(): use an unsigned int, not an int (remove int_converter())
    • fix two bugs and unit tests

    @serhiy-storchaka
    Copy link
    Member

    LGTM.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 21, 2013

    New changeset f947fe289db8 by Victor Stinner in branch 'default':
    Close bpo-18294: Fix the zlib module to make it 64-bit safe
    http://hg.python.org/cpython/rev/f947fe289db8

    @python-dev python-dev mannequin closed this as completed Nov 21, 2013
    @vstinner
    Copy link
    Member Author

    LGTM.

    I changed the "uint" parser to raise a ValueError instead of an OverflowError, so the unit test doesn't need to be adapted.

    Thanks again Serhiy for your review :)

    I doesn't want to backport the fix to Python 2.7 or 3.3, it's more intrusive than what I expected. I opened the issue to fix compiler warnings, it's not like an user complained about a crash.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jan 3, 2014

    New changeset 0cca6c5513d2 by Victor Stinner in branch 'default':
    Issue bpo-18294: Fix uint_converter() in zlibmodule.c, fix the "> UINT_MAX" check
    http://hg.python.org/cpython/rev/0cca6c5513d2

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    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
    @vstinner @serhiy-storchaka