Initialize rem_* in DwarfParser.parseInstructions. #1039
Conversation
src/dwarf.cpp
Outdated
| u32 rem_cfa_reg = 0; | ||
| int rem_cfa_off = 0; | ||
| int rem_fp_off = 0; | ||
| int rem_pc_off = 0; |
There was a problem hiding this comment.
What problem does this fix?
rem values are valid only after they have been assigned to by DW_CFA_remember_state instruction. Otherwise, 0 is just as much garbage as any other random value.
There was a problem hiding this comment.
This is from a static analyzer finding. Hypothetically, there may be a corrupt dwarf file, causing uninitialized values to be used, which would add an invalid frame description.
There was a problem hiding this comment.
I understand. My point is it's not a fix anyway. I.e., when using 0 instead of uninitialized values, it will be just a similar invalid frame descriptor.
There was a problem hiding this comment.
Initializing with an empty frame descriptor would be slightly better, but does not change the situation fundamentally. I mean, bogus dwarf will result in an invalid or truncated stack trace anyway.
There was a problem hiding this comment.
I was able to make DwarfParser use uninitialized variables with this eh_frame_hdr, which could lead to undefined behavior:
00000000: 0103 0333 6100 0000 0100 0000 0100 0000 ...3a...........
00000010: 3000 0000 0000 0000 0000 0000 0000 0000 0...............
00000020: 0000 0000 0000 0000 0000 0000 0000 0000 ................
00000030: 1400 0000 0400 0000 0900 0000 0400 0000 ................
00000040: 000b 0000 00 .....
DwarfParser p("name", 0, eh_frame_hdr);
I haven't investigated if invalid or malicious values could lead to a crash.
Updated the code to use the empty frame descriptor.
2 participants
Description
rem_*variables may not be initialized in all code paths.How has this been tested?
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.