gh-148441: Avoid integer overflow in Expat's CharacterDataHandler#148904
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 ping, 2 weeks have passed since your approval. Is it ready to be merged? :) |
|
Thanks @ByteFlowing1337 for the PR, and @picnixz for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13. |
|
Thanks @ByteFlowing1337 for the PR, and @picnixz for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14. |
|
Thanks @ByteFlowing1337 for the PR, and @picnixz for merging it 🌮🎉.. I'm working now to backport this PR to: 3.15. |
|
GH-149637 is a backport of this pull request to the 3.13 branch. |
I should say 2 weeks have passed since I forgot about it :') sorry! |
|
GH-149638 is a backport of this pull request to the 3.14 branch. |
|
GH-149639 is a backport of this pull request to the 3.15 branch. |
Fix the buffer overflow issue in #148441 , which will cause a core dump.