Skip to content

Conversation

@nielsdos
Copy link
Member

This is not exploitable right now because libxml guarantees right now a maximum string length of 1M bytes. But if that limit were to ever change this could overflow in the future leading to exploits. Again, not exploitable right now, but just making it more future-proof.

This is _not_ exploitable right now because libxml guarantees right now
a maximum string length of 1M bytes. But if that limit were to ever
change this could overflow in the future leading to exploits.
Again, not exploitable right now, but just making it more future-proof.
@devnexen
Copy link
Member

devnexen commented Nov 28, 2023

question: would not you need an explicit cast for xmlStrlen too (just to avoid possible build warnings) ? e.g.

_build_comment(comment, xmlStrlen(comment), &d_comment, &d_comment_len);

Copy link
Member

@Girgias Girgias left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nielsdos
Copy link
Member Author

question: would not you need an explicit cast for xmlStrlen too (just to avoid possible build warnings) ? e.g.

C should implicitly cast, but under -Wconversion could indeed warn. We don't enable this for PHP but I agree that we can do the cast so at least the intent is clear; and if we ever enable such warning there won't be a problem here.

@nielsdos nielsdos merged commit f1bc43b into php:master Nov 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants