classification
Title: zlib module is not completly 64-bit safe
Type: behavior Stage: resolved
Components: Extension Modules Versions: Python 3.4, Python 3.3, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: haypo, loewis, nadeem.vawda, python-dev, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2013-06-24 21:04 by haypo, last changed 2014-01-03 11:27 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
zlib_64bit.patch haypo, 2013-06-24 21:04 review
zlib_64bit-2.patch haypo, 2013-06-25 12:18 review
zlib_64bit-3.patch haypo, 2013-11-19 22:22 review
zlib_64bit-4.patch haypo, 2013-11-20 01:06 review
Messages (13)
msg191799 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2013-06-24 21:04
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.
msg191800 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2013-06-24 21:05
Related issue: #9566.
msg191802 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2013-06-24 21:06
The changeset 931e1bc090f6 fixes warnings in adler32 and crc32.
msg191852 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2013-06-25 12:18
@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.
msg191855 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2013-06-25 12:31
"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...
msg202977 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2013-11-15 23:29
Ping myself.
msg202978 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2013-11-15 23:30
> The "I" parser format does not check for integer overflow.

The "O" format can be used with _PyLong_AsInt() instead.
msg203437 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2013-11-19 22:22
zlib_64bit-3.patch: updated patch which fixes also the PyArg_ParseTuple() convert for C int and C unsigned int.
msg203453 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2013-11-20 01:06
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
msg203460 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-11-20 07:13
LGTM.
msg203674 - (view) Author: Roundup Robot (python-dev) Date: 2013-11-21 21:38
New changeset f947fe289db8 by Victor Stinner in branch 'default':
Close #18294: Fix the zlib module to make it 64-bit safe
http://hg.python.org/cpython/rev/f947fe289db8
msg203675 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2013-11-21 21:44
> 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.
msg207222 - (view) Author: Roundup Robot (python-dev) Date: 2014-01-03 11:27
New changeset 0cca6c5513d2 by Victor Stinner in branch 'default':
Issue #18294: Fix uint_converter() in zlibmodule.c, fix the "> UINT_MAX" check
http://hg.python.org/cpython/rev/0cca6c5513d2
History
Date User Action Args
2014-01-03 11:27:16python-devsetmessages: + msg207222
2013-11-21 21:44:31hayposetmessages: + msg203675
2013-11-21 21:38:50python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg203674

resolution: fixed
stage: patch review -> resolved
2013-11-20 07:13:39serhiy.storchakasetmessages: + msg203460
2013-11-20 01:06:25hayposetfiles: + zlib_64bit-4.patch

messages: + msg203453
2013-11-19 22:22:27hayposetfiles: + zlib_64bit-3.patch

messages: + msg203437
2013-11-15 23:30:12hayposetmessages: + msg202978
2013-11-15 23:29:46hayposetmessages: + msg202977
2013-06-25 12:31:30hayposetmessages: + msg191855
2013-06-25 12:18:14hayposetfiles: + zlib_64bit-2.patch

messages: + msg191852
2013-06-25 08:01:45serhiy.storchakasetversions: + Python 2.7, Python 3.3
nosy: + nadeem.vawda

components: + Extension Modules
type: behavior
stage: patch review
2013-06-24 21:06:02hayposetmessages: + msg191802
2013-06-24 21:05:01hayposetmessages: + msg191800
2013-06-24 21:04:42haypocreate