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.

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

APSWBuffer_hash has undefined behavior due to signed integer overflows #274

Closed
dgrunwald opened this issue Oct 8, 2019 · 2 comments
Closed

Comments

@dgrunwald
Copy link

The hash calculation should use unsigned long / Py_uhash_t to ensure that overflows cause wrap-around as expected.

When compiling with gcc -fsanitize=undefined:

apsw/src/apswbuffer.c:121:18: runtime error: signed integer overflow: 10240061520092322 * 1000003 cannot be represented in type 'long int'
    #0 0x14554f995692 in APSWBuffer_hash apsw/src/apswbuffer.c:121
    #1 0x145559d8e7a0 in PyDict_Contains Objects/dictobject.c:3094
    #2 0x14554f9b6427 in statementcache_finalize apsw/src/statementcache.c:454
    #3 0x14554f9b7664 in resetcursor apsw/src/cursor.c:184
    #4 0x14554f9c70cc in APSWCursor_step apsw/src/cursor.c:825
    #5 0x14554f9c9b61 in APSWCursor_execute apsw/src/cursor.c:1046
    #6 0x145559d9dc6d in _PyCFunction_FastCallDict Objects/methodobject.c:234
    #7 0x145559e27033 in call_function Python/ceval.c:4788
    #8 0x145559e2a409 in _PyEval_EvalFrameDefault Python/ceval.c:3275
@rogerbinns
Copy link
Owner

The code was adapted from Python which is why it was signed. Next commit fixes this.

@dgrunwald
Copy link
Author

Py_hash_t is still signed; for hash calculations you need Py_uhash_t.

rogerbinns added a commit that referenced this issue Oct 20, 2019
Calculations are unsigned, return is signed.

Fixes #274 (again)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants