Page MenuHomePhabricator

[libunwind] Fix UB in EHHeaderParser::findFDE
ClosedPublic

Authored by jgorbe on Apr 7 2020, 2:06 PM.

Details

Summary

When the EHHeaderInfo object filled by decodeEHHdr has fde_count == 0, findFDE does the following:

  • sets low = 0 and len = hdrInfo.fde_count as a preparation to start a binary search
  • because len is 0, the binary search loop is skipped
  • the code still tries to find a table entry at hdrInfo.table + low * tableEntrySize and decode it.

This is wrong when fde_count is 0, and trying to decode a table entry that isn't there will lead to reading garbage offsets and can cause segfaults.

Diff Detail

Event Timeline

jgorbe created this revision.Apr 7 2020, 2:06 PM
saugustine accepted this revision.Apr 7 2020, 2:16 PM

Nice catch. Probably should wait for an unwind owner to accept though.

This revision is now accepted and ready to land.Apr 7 2020, 2:16 PM
mstorsjo accepted this revision.Apr 7 2020, 2:36 PM

LGTM

scw accepted this revision.Apr 7 2020, 2:56 PM
This revision was automatically updated to reflect the committed changes.
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptApr 7 2020, 3:17 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
MaskRay added a subscriber: MaskRay.Apr 7 2020, 3:17 PM

It seems that the problem can be reproduced with a very simple program

Hex dump of section '.eh_frame':
  0x00016410 00000000                            ....

Linux Standard Base 5.0

The .eh_frame section shall contain 1 or more Call Frame Information (CFI) records.

So the linker always adds a frame. findFDE may work on such an empty FDE list and trigger an out-of-bound access.

MaskRay added inline comments.Apr 7 2020, 3:21 PM
libunwind/src/EHHeaderParser.hpp
112

LLVM's code style uses

if (hdrInfo.fde_count == 0)
  return false;

though there is probably not meaningful to change it now..