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

Guard against out-of-bounds memory access when parsing LIMIT_HEAP et al #463

Merged
merged 1 commit into from
Sep 6, 2024

Conversation

alexdowad
Copy link
Contributor

@alexdowad alexdowad commented Sep 6, 2024

While fuzzing PCRE2, I found a case in which pcre2_compile may incorrectly read past the end of the pattern string which it is given. Here are the details:

Patterns passed to pcre2_compile are not guaranteed to be null-terminated. Also, it can happen that there is an invalid pattern like this:

(*LIMIT_HEAP=123

If the next byte of memory after the end of the pattern happens to be a digit, it will be parsed as part of the limit value. Or, if the next byte is a right parenthesis character, it will be taken as the end of the (*LIMIT_HEAP=nnn) construct.

This will result in skipatstart being larger than patlen, which will result in underflow and an erroneous call to malloc requesting a huge number of bytes.

Patterns passed to pcre2_compile are not guaranteed to be
null-terminated. Also, it can happen that there is an invalid
pattern like this:

    (*LIMIT_HEAP=123

If the next byte of memory after the end of the pattern happens
to be a digit, it will be parsed as part of the limit value. Or,
if the next byte is a right parenthesis character, it will be taken
as the end of the (*LIMIT_HEAP=nnn) construct.

This will result in `skipatstart` being larger than `patlen`, which
will result in underflow and an erroneous call to malloc requesting
a huge number of bytes.
@@ -111,10 +111,10 @@ Minimum depth limit = 10
3: ee

/(*LIMIT_MATCH=12bc)abc/
Failed: error 160 at offset 17: (*VERB) not recognized or malformed
Failed: error 160 at offset 16: (*VERB) not recognized or malformed
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is more logical now: the first bad character in the LIMIT_MATCH construct is at offset 16, not offset 17.

Copy link
Collaborator

@zherczeg zherczeg left a comment

Choose a reason for hiding this comment

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

LGTM

@PhilipHazel PhilipHazel merged commit ef218fb into PCRE2Project:master Sep 6, 2024
11 checks passed
@alexdowad alexdowad deleted the membug branch September 6, 2024 22:13
carenas added a commit to carenas/pcre2 that referenced this pull request Sep 21, 2024
As reported recently by ef218fb (Guard against out-of-bounds memory
access when parsing LIMIT_HEAP et al (PCRE2Project#463), 2024-09-07), a malformed
pattern could result in reading 1 byte past its end.

Fix a similar issue that affects all VERBs and add test cases to
ensure the original bug and all its siblings are no longer an issue.

While at it fix the wording of the related documentation.
carenas added a commit to carenas/pcre2 that referenced this pull request Sep 22, 2024
As reported recently by ef218fb (Guard against out-of-bounds memory
access when parsing LIMIT_HEAP et al (PCRE2Project#463), 2024-09-07), a malformed
pattern could result in reading 1 byte past its end.

Fix a similar issue that affects all VERBs and add test cases to
ensure the original bug and all its siblings are no longer an issue.

While at it fix the wording of the related documentation.
PhilipHazel pushed a commit that referenced this pull request Sep 22, 2024
As reported recently by ef218fb (Guard against out-of-bounds memory
access when parsing LIMIT_HEAP et al (#463), 2024-09-07), a malformed
pattern could result in reading 1 byte past its end.

Fix a similar issue that affects all VERBs and add test cases to
ensure the original bug and all its siblings are no longer an issue.

While at it fix the wording of the related documentation.
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

Successfully merging this pull request may close these issues.

3 participants