-
-
Notifications
You must be signed in to change notification settings - Fork 90
fix(debuginfo): Add some dwarf debuginfo recursion limits #1015
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 6 commits
13a89c8
1d4ad13
29a1ab0
2257ada
4b5e224
1704466
1a82117
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -520,7 +520,7 @@ | |
| entry: &Die<'d>, | ||
| language: Language, | ||
| bcsymbolmap: Option<&'d BcSymbolMap<'d>>, | ||
| prior_offset: Option<UnitOffset>, | ||
| depth: u8, | ||
| ) -> Result<Option<Name<'d>>, DwarfError> { | ||
| let mut fallback_name = None; | ||
| let mut reference_target = None; | ||
|
|
@@ -553,21 +553,16 @@ | |
|
|
||
| if let Some(attr) = reference_target { | ||
| return self.resolve_reference(*attr, |ref_unit, ref_entry| { | ||
| // Self-references may have a layer of indirection. Avoid infinite recursion | ||
| // in this scenario. | ||
| if let Some(prior) = prior_offset { | ||
| if self.offset() == ref_unit.offset() && prior == ref_entry.offset() { | ||
| return Ok(None); | ||
| } | ||
| // Avoid infinite recursion. | ||
| if depth == 0 { | ||
| return Err(DwarfError::new( | ||
| DwarfErrorKind::CorruptedData, | ||
| "Exceeded maximum resolve_function_name depth", | ||
| )); | ||
| } | ||
|
|
||
| if self.offset() != ref_unit.offset() || entry.offset() != ref_entry.offset() { | ||
| ref_unit.resolve_function_name( | ||
| ref_entry, | ||
| language, | ||
| bcsymbolmap, | ||
| Some(entry.offset()), | ||
| ) | ||
| ref_unit.resolve_function_name(ref_entry, language, bcsymbolmap, depth - 1) | ||
| } else { | ||
| Ok(None) | ||
| } | ||
|
|
@@ -588,7 +583,10 @@ | |
| prefer_dwarf_names: bool, | ||
| } | ||
|
|
||
| impl<'d, 'a> DwarfUnit<'d, 'a> { | ||
|
Check failure on line 586 in symbolic-debuginfo/src/dwarf.rs
|
||
| /// The maximum depth to recurse to in order to resolve a function name. | ||
| const MAX_RESOLVE_FUNCTION_DEPTH: u8 = 32; | ||
|
|
||
| /// Creates a DWARF unit from the gimli `Unit` type. | ||
| fn from_unit( | ||
| unit: &'a Unit<'d>, | ||
|
|
@@ -841,6 +839,7 @@ | |
| fn parse_functions( | ||
| &self, | ||
| depth: isize, | ||
| remaining_inline_depth: u32, | ||
| entries: &mut EntriesRaw<'d, '_>, | ||
| output: &mut FunctionsOutput<'_, 'd>, | ||
| ) -> Result<(), DwarfError> { | ||
|
|
@@ -850,9 +849,17 @@ | |
| if next_depth <= depth { | ||
| return Ok(()); | ||
| } | ||
|
|
||
| if let Some(abbrev) = entries.read_abbreviation()? { | ||
| if abbrev.tag() == constants::DW_TAG_subprogram { | ||
| self.parse_function(dw_die_offset, next_depth, entries, abbrev, output)?; | ||
| self.parse_function( | ||
| dw_die_offset, | ||
| next_depth, | ||
| remaining_inline_depth, | ||
| entries, | ||
| abbrev, | ||
| output, | ||
| )?; | ||
| } else { | ||
| entries.skip_attributes(abbrev.attributes())?; | ||
| } | ||
|
|
@@ -874,6 +881,7 @@ | |
| &self, | ||
| dw_die_offset: gimli::UnitOffset<usize>, | ||
| depth: isize, | ||
| remaining_inline_depth: u32, | ||
| entries: &mut EntriesRaw<'d, '_>, | ||
| abbrev: &gimli::Abbreviation, | ||
| output: &mut FunctionsOutput<'_, 'd>, | ||
|
|
@@ -909,7 +917,7 @@ | |
| // However, non-inlined functions may be present in this subtree, so we must still descend | ||
| // into it. | ||
| if ranges.is_empty() { | ||
| return self.parse_functions(depth, entries, output); | ||
| return self.parse_functions(depth, remaining_inline_depth, entries, output); | ||
| } | ||
|
|
||
| // Resolve functions in the symbol table first. Only if there is no entry, fall back | ||
|
|
@@ -936,7 +944,12 @@ | |
| let name = symbol_name | ||
| .or_else(|| { | ||
| self.inner | ||
| .resolve_function_name(&entry, language, self.bcsymbolmap, None) | ||
| .resolve_function_name( | ||
| &entry, | ||
| language, | ||
| self.bcsymbolmap, | ||
| DwarfUnit::MAX_RESOLVE_FUNCTION_DEPTH, | ||
| ) | ||
| .ok() | ||
| .flatten() | ||
| }) | ||
|
|
@@ -951,7 +964,13 @@ | |
| let size = range.end - range.begin; | ||
| ( | ||
| *range, | ||
| FunctionBuilder::new(name.clone(), self.compilation_dir(), address, size), | ||
| FunctionBuilder::new( | ||
| name.clone(), | ||
| self.compilation_dir(), | ||
| address, | ||
| size, | ||
| remaining_inline_depth, | ||
| ), | ||
| ) | ||
| }) | ||
| .collect(); | ||
|
|
@@ -960,6 +979,7 @@ | |
| self.parse_function_children( | ||
| depth, | ||
| 0, | ||
| remaining_inline_depth, | ||
| entries, | ||
| &mut builders, | ||
| output, | ||
|
|
@@ -1000,6 +1020,7 @@ | |
| &self, | ||
| depth: isize, | ||
| inline_depth: u32, | ||
| remaining_inline_depth: u32, | ||
| entries: &mut EntriesRaw<'d, '_>, | ||
| builders: &mut [(Range, FunctionBuilder<'d>)], | ||
| output: &mut FunctionsOutput<'_, 'd>, | ||
|
|
@@ -1019,13 +1040,21 @@ | |
| match abbrev.tag() { | ||
| constants::DW_TAG_subprogram => { | ||
| // Nested subprograms resolve their own language independently. | ||
| self.parse_function(dw_die_offset, next_depth, entries, abbrev, output)?; | ||
| self.parse_function( | ||
| dw_die_offset, | ||
| next_depth, | ||
| remaining_inline_depth, | ||
| entries, | ||
| abbrev, | ||
| output, | ||
| )?; | ||
| } | ||
| constants::DW_TAG_inlined_subroutine => { | ||
| self.parse_inlinee( | ||
| dw_die_offset, | ||
| next_depth, | ||
| inline_depth, | ||
| remaining_inline_depth, | ||
| entries, | ||
| abbrev, | ||
| builders, | ||
|
|
@@ -1060,12 +1089,20 @@ | |
| dw_die_offset: gimli::UnitOffset<usize>, | ||
| depth: isize, | ||
| inline_depth: u32, | ||
| remaining_inline_depth: u32, | ||
| entries: &mut EntriesRaw<'d, '_>, | ||
| abbrev: &gimli::Abbreviation, | ||
| builders: &mut [(Range, FunctionBuilder<'d>)], | ||
| output: &mut FunctionsOutput<'_, 'd>, | ||
| language: Language, | ||
| ) -> Result<(), DwarfError> { | ||
| if remaining_inline_depth == 0 { | ||
| return Err(DwarfError::new( | ||
| DwarfErrorKind::CorruptedData, | ||
| "Exceeded max parse inlinee depth", | ||
| )); | ||
| } | ||
|
|
||
| let (ranges, call_location) = self.parse_ranges(entries, abbrev, &mut output.range_buf)?; | ||
|
|
||
| ranges.retain(|range| range.end > range.begin); | ||
|
|
@@ -1078,7 +1115,7 @@ | |
| // However, non-inlined functions may be present in this subtree, so we must still descend | ||
| // into it. | ||
| if ranges.is_empty() { | ||
| return self.parse_functions(depth, entries, output); | ||
| return self.parse_functions(depth, remaining_inline_depth, entries, output); | ||
|
cursor[bot] marked this conversation as resolved.
|
||
| } | ||
| let ranges = ranges.clone(); | ||
|
|
||
|
|
@@ -1090,7 +1127,12 @@ | |
| // which carries the wrong language (e.g. a C++ LTO partial unit for C code). | ||
| let name = self | ||
| .inner | ||
| .resolve_function_name(&entry, language, self.bcsymbolmap, None) | ||
| .resolve_function_name( | ||
| &entry, | ||
| language, | ||
| self.bcsymbolmap, | ||
| DwarfUnit::MAX_RESOLVE_FUNCTION_DEPTH, | ||
| ) | ||
| .ok() | ||
| .flatten() | ||
| .unwrap_or_else(|| Name::new("", NameMangling::Unmangled, language)); | ||
|
|
@@ -1105,6 +1147,7 @@ | |
| self.parse_function_children( | ||
| depth, | ||
| inline_depth + 1, | ||
| remaining_inline_depth - 1, | ||
| entries, | ||
| builders, | ||
| output, | ||
|
|
@@ -1309,10 +1352,11 @@ | |
| fn functions( | ||
| &self, | ||
| seen_ranges: &mut BTreeSet<(u64, u64)>, | ||
| max_inline_depth: u32, | ||
| ) -> Result<Vec<Function<'d>>, DwarfError> { | ||
| let mut entries = self.inner.unit.entries_raw(None)?; | ||
| let mut output = FunctionsOutput::with_seen_ranges(seen_ranges); | ||
| self.parse_functions(-1, &mut entries, &mut output)?; | ||
| self.parse_functions(-1, max_inline_depth, &mut entries, &mut output)?; | ||
|
Check failure on line 1359 in symbolic-debuginfo/src/dwarf.rs
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. parse_function / parse_function_children lack depth limit for nested DW_TAG_subprogram While the PR caps inlined-subroutine recursion with Evidence
Also found at 5 additional locations
Identified by Warden · wrdn-dos-review · EVE-PMU |
||
| Ok(output.functions) | ||
| } | ||
| } | ||
|
|
@@ -1661,6 +1705,7 @@ | |
| pub struct DwarfDebugSession<'data> { | ||
| cell: SelfCell<Box<DwarfSections<'data>>, DwarfInfo<'data>>, | ||
| bcsymbolmap: Option<Arc<BcSymbolMap<'data>>>, | ||
| max_inline_depth: u32, | ||
| } | ||
|
|
||
| impl<'data> DwarfDebugSession<'data> { | ||
|
|
@@ -1670,6 +1715,7 @@ | |
| symbol_map: SymbolMap<'data>, | ||
| address_offset: i64, | ||
| kind: ObjectKind, | ||
| max_inline_depth: u32, | ||
| ) -> Result<Self, DwarfError> | ||
| where | ||
| D: Dwarf<'data>, | ||
|
|
@@ -1682,6 +1728,7 @@ | |
| Ok(DwarfDebugSession { | ||
| cell, | ||
| bcsymbolmap: None, | ||
| max_inline_depth, | ||
| }) | ||
| } | ||
|
|
||
|
|
@@ -1710,6 +1757,7 @@ | |
| functions: Vec::new().into_iter(), | ||
| seen_ranges: BTreeSet::new(), | ||
| finished: false, | ||
| max_inline_depth: self.max_inline_depth, | ||
|
Check failure on line 1760 in symbolic-debuginfo/src/dwarf.rs
|
||
| } | ||
| } | ||
|
|
||
|
|
@@ -1820,6 +1868,7 @@ | |
| functions: std::vec::IntoIter<Function<'s>>, | ||
| seen_ranges: BTreeSet<(u64, u64)>, | ||
| finished: bool, | ||
| max_inline_depth: u32, | ||
| } | ||
|
|
||
| impl<'s> Iterator for DwarfFunctionIterator<'s> { | ||
|
|
@@ -1841,7 +1890,7 @@ | |
| None => break, | ||
| }; | ||
|
|
||
| self.functions = match unit.functions(&mut self.seen_ranges) { | ||
| self.functions = match unit.functions(&mut self.seen_ranges, self.max_inline_depth) { | ||
|
Check failure on line 1893 in symbolic-debuginfo/src/dwarf.rs
|
||
| Ok(functions) => functions.into_iter(), | ||
| Err(error) => return Some(Err(error)), | ||
| }; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -72,6 +72,7 @@ | |
| data: &'data [u8], | ||
| is_malformed: bool, | ||
| max_decompressed_section_size: Option<usize>, | ||
| max_inline_depth: u32, | ||
| } | ||
|
|
||
| impl<'data> ElfObject<'data> { | ||
|
|
@@ -171,6 +172,7 @@ | |
| data, | ||
| is_malformed: true, | ||
| max_decompressed_section_size: opts.max_decompressed_section_size, | ||
| max_inline_depth: opts.max_inline_depth, | ||
|
Comment on lines
174
to
+175
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ELF decompression size guard defaults to unlimited, allowing OOM abort on tiny input
Evidence
Also found at 2 additional locations
Identified by Warden · wrdn-dos-review · NRR-XKG |
||
| }); | ||
| } | ||
| }; | ||
|
|
@@ -355,6 +357,7 @@ | |
| data, | ||
| is_malformed: false, | ||
| max_decompressed_section_size: opts.max_decompressed_section_size, | ||
| max_inline_depth: opts.max_inline_depth, | ||
|
Check failure on line 360 in symbolic-debuginfo/src/elf.rs
|
||
|
sentry-warden[bot] marked this conversation as resolved.
sentry-warden[bot] marked this conversation as resolved.
|
||
| }) | ||
| } | ||
|
|
||
|
|
@@ -562,7 +565,13 @@ | |
| /// [`has_debug_info`](struct.ElfObject.html#method.has_debug_info). | ||
| pub fn debug_session(&self) -> Result<DwarfDebugSession<'data>, DwarfError> { | ||
| let symbols = self.symbol_map(); | ||
| DwarfDebugSession::parse(self, symbols, self.load_address() as i64, self.kind()) | ||
| DwarfDebugSession::parse( | ||
|
Check failure on line 568 in symbolic-debuginfo/src/elf.rs
|
||
| self, | ||
| symbols, | ||
| self.load_address() as i64, | ||
| self.kind(), | ||
| self.max_inline_depth, | ||
| ) | ||
| } | ||
|
|
||
| /// Determines whether this object contains stack unwinding information. | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.