homepage

There are several places where the result of PyLong_AsLong() assigned to variable of type int. It can cause unknown issues on platforms with sizeof(int) != sizeof(long). All 140 cases of PyLong_AsLong() usage should be checked.

Getting a C int out of a Python int is currently a bit awkward. What do you think about adding a PyLong_AsInt counterpart to PyLong_AsLong? (The alternative is to use PyLong_AsLong and repeat the same overflow detection code in many places.)

In the Objects subdirectory (which is all I've looked at so far), I see issues in:
- fileobject.c (PyObject_AsFileDescriptor)
- structseq.c (PyLong_AsLong return value used as a Py_ssize_t; probably safe, but it would be better to use PyLong_AsSsize_t).
- unicodeobject.c (one place where result assigned to something of type ssize_t, one where result assigned to something of type int).

Here is a patch (for 3.3). I added private _PyLong_AsInt and where possible I 
have tried to use the appropriate function.

I was assigned this to itself to show that I working on the issue. Now I free up place for someone with committing privileges.

Looks good, in general.
_PyLong_AsInt should return -1 on overflow in all cases; at the moment, it looks like it doesn't do that for values that overflow an int but not a long.

I added some comments on Rietveld.
Apart from the bug in _PyLong_AsInt mentioned above, the patch mostly looks good. But there are many changes, some of which will have user-visible effects, and I think it's somewhat risky to make all these changes in bugfix releases (which now includes Python 3.3).
Perhaps there could be a smaller patch aimed at the bugfix releases? Ideally, each fix there should be backed by a regression test.

Thank you for review and for found bugs. I again checked the patch, corrected 
the errors and dubious places. Also I corrected the error in 
Modules/grpmodule.c (in other places gid_t parsed as signed long) and the 
possible behaviour change in Modules/_io/_iomodule.c, reversed the changes in 
Modules/pyexpat.c. If some changes cause you have doubts, you can cherry-pick 
only the most obvious fixes.
> Ideally, each fix there should be backed by a regression test.
Unfortunately I have only platforms where sizeof(int) == sizeof(long) == 
sizeof(size_t).

Added patches for 2.7 and 3.2.

Here is a tests for most of fixed overflows. Some errors are difficult or impossible to reproduce.

Here are minimized patches. All included changes have tests (i.e. overflow bugs 
are observable). I drop grp module changes even with tests, because there are 
other issues for this (issue2005, issue4591). New tests for fcntl added (they 
test PyObject_AsFileDescriptor).
If patches look good in general, I going first commit to 3.4, and if this will 
not break any buildbot, then I'll commit the rest patches. After that I'll 
continue with not included changes (if I can write tests for them).

New changeset 13e2e44db99d by Serhiy Storchaka in branch 'default':
Issue #15989: Fix several occurrences of integer overflow
http://hg.python.org/cpython/rev/13e2e44db99d 

Changeset 525407d89277: Fix test_socket broken in previous commit (changeset 13e2e44db99d).

New changeset 974ace29ee2d by Serhiy Storchaka in branch '3.2':
Issue #15989: Fix several occurrences of integer overflow
http://hg.python.org/cpython/rev/974ace29ee2d
New changeset 8f10c9eae183 by Serhiy Storchaka in branch '3.3':
Issue #15989: Fix several occurrences of integer overflow
http://hg.python.org/cpython/rev/8f10c9eae183 

New changeset d544873d62e9 by Serhiy Storchaka in branch '2.7':
Issue #15989: Fix several occurrences of integer overflow
http://hg.python.org/cpython/rev/d544873d62e9 

Several 2.7 buildbots are failing.
> Unfortunately I have only platforms where sizeof(int) == sizeof(long) == 
sizeof(size_t).
You can use your cpython ssh key to login to all snakebite buildbot
machines. They are tagged with [SB].
http://mail.python.org/pipermail/python-dev/2012-September/121651.html 

New changeset a78ebf9aed06 by Serhiy Storchaka in branch '2.7':
Ensure that width and precision in string formatting test have type int, not long.
http://hg.python.org/cpython/rev/a78ebf9aed06 

Thank you for point, Stefan. And thanks Trent for his project.

New changeset ee93a89b4e0f by Serhiy Storchaka in branch '2.7':
Issue #15989: Fix possible integer overflow in str formatting as in unicode formatting.
http://hg.python.org/cpython/rev/ee93a89b4e0f 

Sqlite module part extracted to issue17073.

Here are remnants of initial patch which were not committed. There are no tests.

Are the fixes in the final patch waiting for someone to write tests, or is the intent to commit them without tests after a final review because tests are too difficult to write?

It would be good to write tests, but for some tests it is difficult (if possible). At least I did not see ways how to do this.

@serhiy - I'm a little confused about the state of this patch. It seems like you need more review?

Yes, it would be good if other's pair of eyes will look on the patch.

It looks fine to me, for whatever thats worth. I think you should commit it.

New changeset d51a82f68a70 by Serhiy Storchaka in branch 'default':
Issue #15989: Fixed some scarcely probable integer overflows.
https://hg.python.org/cpython/rev/d51a82f68a70 

One of changes to Modules/_io/_iomodule.c is no longer actual since issue21679.
Change to Modules/selectmodule.c is no longer actual since issue23485 (f54bc2c52dfd).
The rest changes were committed to 3.6 only.

Isn't Python-ast.c a generated file?

New changeset e53df7955192 by Serhiy Storchaka in branch 'default':
Make asdl_c.py to generate Python-ast.c changed in issue #15989.
https://hg.python.org/cpython/rev/e53df7955192 

> Isn't Python-ast.c a generated file?
Good catch Eric.