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.
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
Possible integer overflow of PyLong_AsLong() results #60193
Comments
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:
|
Here is a patch (for 3.3). I added private _PyLong_AsInt and where possible I |
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
Unfortunately I have only platforms where sizeof(int) == sizeof(long) == |
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 If patches look good in general, I going first commit to 3.4, and if this will |
New changeset 13e2e44db99d by Serhiy Storchaka in branch 'default': |
Changeset 525407d89277: Fix test_socket broken in previous commit (changeset 13e2e44db99d). |
New changeset 974ace29ee2d by Serhiy Storchaka in branch '3.2': New changeset 8f10c9eae183 by Serhiy Storchaka in branch '3.3': |
New changeset d544873d62e9 by Serhiy Storchaka in branch '2.7': |
Several 2.7 buildbots are failing.
You can use your cpython ssh key to login to all snakebite buildbot http://mail.python.org/pipermail/python-dev/2012-September/121651.html |
New changeset a78ebf9aed06 by Serhiy Storchaka in branch '2.7': |
Thank you for point, Stefan. And thanks Trent for his project. |
New changeset ee93a89b4e0f by Serhiy Storchaka in branch '2.7': |
Sqlite module part extracted to bpo-17073. |
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': |
One of changes to Modules/_io/_iomodule.c is no longer actual since bpo-21679. Change to Modules/selectmodule.c is no longer actual since bpo-23485 (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': |
Good catch Eric. |
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:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: