fix: bound DWARF .eh_frame_hdr parsing to the section#571
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c6e3fc8393
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
This comment has been minimized.
This comment has been minimized.
jbachorik
left a comment
There was a problem hiding this comment.
Sphinx review — issues in unchanged parseFde() body
Two findings that couldn't be anchored as inline comments because they fall in the unchanged part of parseFde():
dwarf.cpp:301 — unguarded cie_offset underflow (HIGH)
cie_offset is a u32 read directly from the untrusted, memory-mapped ELF image. When cie_offset > (fde_start - _section_start), the subtraction fde_start - cie_offset underflows past the start of the mapped allocation — undefined behaviour. A crafted small value also redirects CIE parsing to an attacker-controlled position inside the section, defeating the bounds model this PR establishes.
if (cie_offset > (size_t)(fde_start - _section_start)) {
return;
}
_ptr = fde_start - cie_offset;dwarf.cpp:308 — unclamped _ptr advance after untrusted LEB (MEDIUM)
_ptr += getLeb() advances by a decoded value from untrusted input without clamping to _section_end. A large value pushes _ptr past the section boundary, silently truncating the rest of the FDE. Same issue in parseInstructions() at the DW_CFA_def_cfa_expression and DW_CFA_expression branches (lines ~404, ~409).
After each unclamped advance: if (_ptr > _section_end) _ptr = _section_end;
Raised by Sphinx review.
a07abc8 to
40af28f
Compare
a412832 to
e504121
Compare
4b2d3b1
into
DataDog:main
What does this PR do?:
Fix bound checking on untrusted section size, avoid infinite loops, and fix small fuzz-harness only memory leak.
Motivation:
Make the dwarf parser more reliable.
Additional Notes:
How to test the change?:
For Datadog employees:
credentials of any kind, I've requested a review from
@DataDog/security-design-and-guidance.Unsure? Have a question? Request a review!