Skip to content

Commit 6a19a59

Browse files
authored
pe: Reject cyclic resource trees (#499)
Parsing these currently results in a hang. This follows the approach taken by the LIEF PE parser. Others like `trailofbits/pe-parse` strictly limit the resource tree depth to 3 instead (which is the max seen in the wild) to side-step this problem.
1 parent 75479d4 commit 6a19a59

File tree

2 files changed

+21
-0
lines changed

2 files changed

+21
-0
lines changed

src/pe/resource.rs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -399,11 +399,24 @@ impl ResourceEntry {
399399
P: Fn(&Self) -> bool,
400400
{
401401
let mut current = *self;
402+
let mut visited = alloc::collections::BTreeSet::new();
403+
404+
// Track the offset to detect cycles
405+
visited.insert(current.offset_to_data_or_directory);
402406

403407
while let Some(next) = current.next_depth(bytes)? {
404408
if !predicate(&next) {
405409
return Ok(Some(next));
406410
}
411+
412+
// Reject PEs that have a malformed resource tree
413+
if !visited.insert(next.offset_to_data_or_directory) {
414+
return Err(error::Error::Malformed(format!(
415+
"Cycle detected in resource directory at offset {:#x}",
416+
next.offset_to_data_or_directory
417+
)));
418+
}
419+
407420
current = next;
408421
}
409422

@@ -1285,6 +1298,8 @@ mod tests {
12851298
const HAS_NO_RES: &[u8] = include_bytes!("../../tests/bins/pe/has_no_res.exe.bin");
12861299
const HAS_RES_FULL_VERSION_AND_MANIFEST: &[u8] =
12871300
include_bytes!("../../tests/bins/pe/has_res_full_version_and_manifest.exe.bin");
1301+
const MALFORMED_RESOURCE_TREE: &[u8] =
1302+
include_bytes!("../../tests/bins/pe/malformed_resource_tree.exe.bin");
12881303

12891304
/// Binary representation of following default LLD manifest (`/MANIFEST`) expect as UTF-8.
12901305
///
@@ -1916,4 +1931,10 @@ mod tests {
19161931
let result = to_utf16_string(bytes);
19171932
assert!(result.is_some());
19181933
}
1934+
1935+
#[test]
1936+
#[should_panic = "Cycle detected in resource directory at offset"]
1937+
fn malformed_resource_tree() {
1938+
let _ = crate::pe::PE::parse(MALFORMED_RESOURCE_TREE).unwrap();
1939+
}
19191940
}
259 Bytes
Binary file not shown.

0 commit comments

Comments
 (0)