Skip to content

Fix stack-buffer-overflow in parse_regex due to missing bounds checks - #1486

Merged
val-ms merged 2 commits into
Cisco-Talos:mainfrom
Shivam7-1:patch-1
Apr 24, 2025
Merged

Fix stack-buffer-overflow in parse_regex due to missing bounds checks#1486
val-ms merged 2 commits into
Cisco-Talos:mainfrom
Shivam7-1:patch-1

Conversation

@Shivam7-1

Copy link
Copy Markdown
Contributor

issue link: https://issues.oss-fuzz.com/issues/388922799
PR description
parse_regex iterates over the input pattern using the *last index but fails to enforce bounds checking before accessing p[*last]. When passed malformed or fuzzed input, this can lead to out-of-bounds reads, causing a crash or undefined behavior.

Fix:
Added strict (*last < pSize) bounds checking to:
-- The main loop condition in parse_regex
-- All cases where p[*last] is accessed (including switch statements and conditions)
Ensured safe handling of escape sequences, character classes, and alternates.
Added early exit if *last exceeds pSize to prevent further processing of invalid input.

@val-ms

val-ms commented Apr 14, 2025

Copy link
Copy Markdown
Contributor

Thank you for working on this. It's one we've been procrastinating because we don't consider this type overflow to be a security issue (hence the issue becoming public. I'll review your work shortly.

@val-ms val-ms left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There's an improvement to be made just from reading the code. I still need to think about it harder and play with it.

Comment thread libclamav/regex_suffix.c Outdated
Co-authored-by: Val S. <mx.val@icloud.com>

@val-ms val-ms left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I have been unable to reproduce the oss-fuzz issue using oss-fuzz.

E.g. I run python3 infra/helper.py reproduce clamav clamav_dbload_PDB_fuzzer ~/Downloads/clusterfuzz-testcase-minimized-clamav_dbload_PDB_fuzzer-5501437374693376 and it doesn't trigger.

I did run into an unrelated memory leak introduced in 1.5 development that is triggering with all oss-fuzz runs and for which I need to fix to get oss-fuzz testing back up and running. So I made a fix for that here ( #1489 ). But even on that branch, I couldn't reproduce this issue.

That said, I agree with your change.

@val-ms
val-ms merged commit 41aa292 into Cisco-Talos:main Apr 24, 2025
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.

2 participants