Skip to content
This repository has been archived by the owner on Aug 15, 2021. It is now read-only.

Commit

Permalink
Prevent stack overflow from nested tags
Browse files Browse the repository at this point in the history
In the deserializer decrement the remaining depth
each time a tagged value is encountered to prevent
stack overflows caused by this recursion.
Small malicious input can cause this overflow,
therefore it is considered to be a security issue.

I'd like to thank Eric Rafaloff at Trail of Bits,
who discovered this issue during a code review,
for reporting it.

Add a test case.
  • Loading branch information
pyfisch committed Oct 3, 2019
1 parent 1a7f31f commit 1aec4f9
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 5 deletions.
10 changes: 5 additions & 5 deletions src/de.rs
Original file line number Diff line number Diff line change
Expand Up @@ -704,22 +704,22 @@ where
0xbf => self.parse_indefinite_map(visitor),

// Major type 6: optional semantic tagging of other major types
0xc0..=0xd7 => self.parse_value(visitor),
0xc0..=0xd7 => self.recursion_checked(|de| de.parse_value(visitor)),
0xd8 => {
self.parse_u8()?;
self.parse_value(visitor)
self.recursion_checked(|de| de.parse_value(visitor))
}
0xd9 => {
self.parse_u16()?;
self.parse_value(visitor)
self.recursion_checked(|de| de.parse_value(visitor))
}
0xda => {
self.parse_u32()?;
self.parse_value(visitor)
self.recursion_checked(|de| de.parse_value(visitor))
}
0xdb => {
self.parse_u64()?;
self.parse_value(visitor)
self.recursion_checked(|de| de.parse_value(visitor))
}
0xdc..=0xdf => Err(self.error(ErrorCode::UnassignedCode)),

Expand Down
11 changes: 11 additions & 0 deletions tests/de.rs
Original file line number Diff line number Diff line change
Expand Up @@ -725,4 +725,15 @@ mod std_tests {
let deserialized_ip = from_slice::<IpAddr>(&buf).unwrap();
assert_eq!(ip, deserialized_ip);
}

#[test]
fn attempt_stack_overflow() {
// Create a tag 17, followed by 999 more tag 17:
// 17(17(17(17(17(17(17(17(17(17(17(17(17(17(17(17(17(17(...
// This causes deep recursion in the decoder and may
// exhaust the stack and therfore result in a stack overflow.
let input = vec![0xd1; 1000];
let err = serde_cbor::from_slice::<serde_cbor::Value>(&input).expect_err("recursion limit");
assert!(err.is_syntax());
}
}

0 comments on commit 1aec4f9

Please sign in to comment.