Repository navigation
Conversation
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. |
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces bounds checks in FIRCLSDwarfParseCIERecord and FIRCLSDwarfParseFDERecord to ensure that the parsed CIE and FDE headers do not extend past their declared lengths, along with a corresponding unit test. Feedback on the changes highlights a potential integer overflow vulnerability where a very large augmentationLength could cause ptr to wrap around and bypass the new ptr > endAddress check, and suggests an improved check to prevent this.
|
Pushed db6cd0c to address the review: the FDE augmentation length is now bounded against the remaining record bytes before it's added to the cursor, so a huge length can't overflow the pointer and wrap past the end check. Regression test covers the wrapping case. |
|
@isl-Ramzi Would you fix the formatting? |
|
Sure, fixed in bf5e402. It was the comment alignment in the new FIRCLSDwarfTests.m test, which was the only file the style check flagged. |
FIRCLSDwarfParseCIERecord and FIRCLSDwarfParseFDERecord set the DWARF instruction block size with instructions.length = (uint32_t)(endAddress - ptr), but nothing checks that ptr is still at or before endAddress after the header fields are read. A record in a loaded image's __eh_frame whose declared length is shorter than the header it actually carries (or an FDE with a large augmentation length) leaves ptr past endAddress, so that subtraction goes negative and the cast wraps to a length near 4 GB. FIRCLSDwarfInstructionsEnumerate then walks the instruction cursor across that bogus range and reads well past the mapped section during crash-time unwinding.
The fix rejects the record when ptr has moved beyond endAddress, at both the CIE and FDE sites, before the length is computed. Valid records are unaffected since their headers always fit within the declared length. Added a regression test in FIRCLSDwarfTests.m that builds a CIE plus an FDE with a too-short length and checks the parse now fails cleanly instead of returning a wrapped ~4 GB instruction length.