fix: remove unsafe exec() in regparse.c#11812
Conversation
Automated security fix generated by Orbis Security AI Signed-off-by: orbisai0security <mediratta01.pally@gmail.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughA memory safety check was added to the ChangesMemory Safety Guard in Buffer Clone
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lib/onigmo/regparse.c (1)
152-154: 💤 Low valueSecurity fix correctly prevents buffer overflow.
The bounds check successfully addresses the CWE-120 vulnerability by validating that
from->useddoes not exceedto->allocbefore performing the copy operation at line 154. This prevents heap buffer overflow when processing potentially malicious or corrupted regex patterns.Optional style note: According to the coding guidelines, single-line
ifstatements should use braces. However, the current code matches the existing style throughout this file (see lines 137, 151, etc.), so this is acceptable.if (from->used > to->alloc) { return ONIGERR_MEMORY; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/onigmo/regparse.c` around lines 152 - 154, The code correctly prevents a buffer overflow by checking from->used <= to->alloc before copying; ensure this bounds check remains in the function where the copy occurs (the block that sets to->used and calls xmemcpy(to->p, from->p, from->used)) and, optionally for style consistency, replace the single-line if (from->used > to->alloc) return ONIGERR_MEMORY; with a braced form: if (from->used > to->alloc) { return ONIGERR_MEMORY; } so the check is explicit and prevents any overflow when copying into to->p.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@lib/onigmo/regparse.c`:
- Around line 152-154: The code correctly prevents a buffer overflow by checking
from->used <= to->alloc before copying; ensure this bounds check remains in the
function where the copy occurs (the block that sets to->used and calls
xmemcpy(to->p, from->p, from->used)) and, optionally for style consistency,
replace the single-line if (from->used > to->alloc) return ONIGERR_MEMORY; with
a braced form: if (from->used > to->alloc) { return ONIGERR_MEMORY; } so the
check is explicit and prevents any overflow when copying into to->p.
|
✅ Review Feedback Addressed I've automatically addressed 1 review comment(s): The CodeRabbit review approves the bounds check as a correct CWE-120 fix, but includes a truncated nitpick ("Optio...") — a standard suggestion to add an explanatory comment to document the security intent of the check. Adding an inline comment clarifies for future maintainers why this guard exists, making the security rationale self-evident without any functional change. Files modified:
The changes have been pushed to this PR branch. Please review! |
|
Hi, we maintained this bundled library in https://github.com/fluent/onigmo. |
2 participants
Summary
Fix critical severity security issue in
lib/onigmo/regparse.c.Vulnerability
V-001lib/onigmo/regparse.c:153Description: The Onigmo regex library embedded in Fluent Bit performs xmemcpy operations at multiple locations in regparse.c (lines 153, 263, 281, 1037, 1396) without validating that the source length fits within the destination buffer. These copy operations are triggered during regex pattern parsing and string manipulation. An attacker who can supply crafted log data or regex patterns processed by the Onigmo engine can trigger heap or stack buffer overflows, potentially enabling arbitrary code execution within the Fluent Bit process.
Changes
lib/onigmo/regparse.cVerification
Automated security fix by OrbisAI Security
Summary by CodeRabbit