gh-148252: Avoid overflow in _remote_debugging binary reader bounds checks#148972
gh-148252: Avoid overflow in _remote_debugging binary reader bounds checks#148972maurycy wants to merge 4 commits intopython:mainfrom
_remote_debugging binary reader bounds checks#148972Conversation
|
|
||
| /* File structure sizes */ | ||
| #define FILE_FOOTER_SIZE 32 | ||
| #define SAMPLE_RECORD_HEADER_SIZE (sizeof(uint64_t) + sizeof(uint32_t) + 1) |
There was a problem hiding this comment.
There's already SAMPLE_HEADER_FIXED_SIZE:
but it's hard-coded 13. Maybe it should be changed to this addition?
grep for 13 yields more results:
- https://github.com/python/cpython/blob/665b7dfcfa2/Modules/_remote_debugging/binary_io_writer.c#L656
- https://github.com/python/cpython/blob/665b7dfcfa2/Modules/_remote_debugging/binary_io_writer.c#L657
- https://github.com/python/cpython/blob/665b7dfcfa2/Modules/_remote_debugging/binary_io_reader.c#L979
It looks that there's opportunity to move it to binary_io.h. Makes sense, @pablogsal ?
There was a problem hiding this comment.
I think it makes sense indeed 👍 Can you do the change in this PR?
There was a problem hiding this comment.
Voilà: f14741f
Truth to be told, I think there's a massive room for improvement with consts here:
https://github.com/maurycy/cpython/blob/remote-debugging-int-bounds/Modules/_remote_debugging/binary_io_reader.c#L26
https://github.com/maurycy/cpython/blob/remote-debugging-int-bounds/Modules/_remote_debugging/binary_io_reader.c#L26
or magic offsets:
cpython/Modules/_remote_debugging/binary_io_writer.c
Lines 1146 to 1154 in b2f126c
(perhaps just FILE_FOOTER_SIZE here etc.)
https://github.com/maurycy/cpython/blob/f14741f422d0983f9a3c6a7bdd9c47b931e3fb1e/Modules/_remote_debugging/binary_io_reader.c#L985-L1003
https://github.com/maurycy/cpython/blob/f14741f422d0983f9a3c6a7bdd9c47b931e3fb1e/Modules/_remote_debugging/binary_io_writer.c#L651-L662
https://github.com/maurycy/cpython/blob/f14741f422d0983f9a3c6a7bdd9c47b931e3fb1e/Modules/_remote_debugging/binary_io_writer.c#L590-L595
(perhaps just SAMPLE_OFF warranted here as well)
etc., etc.
If interest in cleaning up these – I can create a PR.
_remote_debugging binary reader bound checks_remote_debugging binary reader bounds checks
I don't think a test makes sense; it'd fail only on 32-bit machines.
More details: