diff options
author | Neal H. Walfield <neal@pep.foundation> | 2017-12-12 14:04:49 +0100 |
---|---|---|
committer | Neal H. Walfield <neal@pep.foundation> | 2017-12-12 14:04:49 +0100 |
commit | 795e9600746f97e34ee48621229ea475247aa145 (patch) | |
tree | baeab4d55d9cccd9b9e13f0b199bfab999ae134b /src | |
parent | 042555d261c8257bffe6bf29332aa3d9729e6950 (diff) |
Hard limit OpenPGP packet recursion to 255 levels.
- Reading from the pipeline is a recursive operation (when reading
from a reader, the reader calls the underlying reader, etc.). It
is probably possible to refactor this to be iterative, but the
cost is too high when in practice more than 16 levels of recursion
is almost certainly an error, and for 16 levels, there is more
than enough headroom (I blow the stack for >512 levels of
recursion on 64-bit Linux).
Diffstat (limited to 'src')
-rw-r--r-- | src/openpgp/parse/parse.rs | 23 |
1 files changed, 12 insertions, 11 deletions
diff --git a/src/openpgp/parse/parse.rs b/src/openpgp/parse/parse.rs index 2da11831..ca131017 100644 --- a/src/openpgp/parse/parse.rs +++ b/src/openpgp/parse/parse.rs @@ -14,7 +14,7 @@ use super::*; /// [ encryption container: [ signature: [ compressioned data: [ literal data ]]]] /// /// So, this should be more than enough. -const MAX_RECURSION_DEPTH : usize = 16; +const MAX_RECURSION_DEPTH : u8 = 16; // Packet headers. @@ -525,10 +525,14 @@ pub struct PacketParser<'a> { // This packets recursion depth. A top-level packet has a // recursion depth of 0. - recursion_depth: usize, + recursion_depth: u8, // The maximum allowed recursion depth. - max_recursion_depth: usize, + // + // There is absolutely no reason that this should be more than + // 255. Moreover, if it is too large, then a read from the + // pipeline will blow the stack. + max_recursion_depth: u8, // The packet that is being parsed. packet: Packet, @@ -568,7 +572,7 @@ impl <'a> PacketParser<'a> { /// To manage the amount of recursion manually, just pass /// std::usize::MAX. pub fn new<R: BufferedReader + 'a>(bio: R, - max_recursion_depth: Option<usize>) + max_recursion_depth: Option<u8>) -> Result<Option<PacketParser<'a>>, std::io::Error> { // Parse the first packet. let pp = PacketParser::parse(bio)?; @@ -773,7 +777,7 @@ impl Container { impl Message { pub fn deserialize<R: BufferedReader> - (bio: R, max_recursion_depth: Option<usize>) + (bio: R, max_recursion_depth: Option<u8>) -> Result<Message, std::io::Error> { // Create a top-level container. let mut top_level = Container::new(); @@ -974,14 +978,11 @@ fn compression_quine_test_2 () { let mut f = File::open(&path).expect(&path.to_string_lossy()); let bio = BufferedReaderGeneric::new(&mut f, None); - // XXX: If I set this higher, I get a stack overflow stemming from - // PacketParser::finish and its call to steal_eof. This needs to - // be investigated. - let max_recursion_depth = 512; + let max_recursion_depth = 255; let mut ppo : Option<PacketParser> = PacketParser::new(bio, Some(max_recursion_depth)).unwrap(); - let mut count = 0; + let mut count : usize = 0; loop { if let Some(pp2) = ppo { count += 1; @@ -992,5 +993,5 @@ fn compression_quine_test_2 () { break; } } - assert_eq!(count, 1 + max_recursion_depth); + assert_eq!(count, 1 + max_recursion_depth as usize); } |