gh-148441: Avoid integer overflow in Expat's CharacterDataHandler#148904
gh-148441: Avoid integer overflow in Expat's CharacterDataHandler#148904ByteFlowing1337 wants to merge 11 commits intopython:mainfrom
Conversation
…vpCkR.rst Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
|
Done. |
|
@picnixz Thanks for the review. Please let me know if you have any further suggestions. |
|
The EMScripten tests fail. I suspect that we don't have enough memory on those machines. So you need to also ensure that you have enough memory (I don't remembner the resourdces to request, something likie big_memory) |
Try to fix unstable Windows CI test failure?
|
Now all CI tests pass! |
|
Thanks, I'll merge it in a few days to let others have a look if they want to. Until then, you don't need to update your branch (in general, don't update your branch unless main has changed in a non-compatible way (e.g., new CI failures)). |
| call_character_handler(self, data, len); | ||
| else { | ||
| if ((self->buffer_used + len) > self->buffer_size) { | ||
| if (len > (self->buffer_size - self->buffer_used)) { |
There was a problem hiding this comment.
This looks like a fix to an integer overflow rather than a buffer overflow. Am missing something?
There was a problem hiding this comment.
The cause is indeed an integer overflow, but I think the ASAN report says "heap-buffer overflow" in this case (so the result is a buffer overflow at the end). I'll just write "a crash" (people don't really care about the cause/exact result: if it crashes, it's bad; they can read the issue for more details).
There was a problem hiding this comment.
I've updated the title to reflect what the PR did but I'll stay vague in the NEWS.
picnixz
changed the title
picnixz
changed the title
3 participants
Fix the buffer overflow issue in #148441 , which will cause a core dump.