From d18c70573613aed420c4209c47f90483540cf18b Mon Sep 17 00:00:00 2001 From: Justus Winter Date: Tue, 22 Sep 2020 14:03:42 +0200 Subject: openpgp: Remove erroneous assertion. - We erroneously assumed that when BufferedReader::next() is called, a SEIP container must be opaque and hence there cannot be a buffered_reader::Reserve on the stack with Cookie::fake_eof set. But, we could simply call BufferedReader::next() after the SEIP packet is decrypted, or buffer a SEIP packet's body, then call BufferedReader::recurse(), which falls back to BufferedReader::next() because some data has been read. - Remove the erroneous assertion. - Fixes #455. --- openpgp/src/parse.rs | 54 +++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 49 insertions(+), 5 deletions(-) (limited to 'openpgp') diff --git a/openpgp/src/parse.rs b/openpgp/src/parse.rs index d2ac00c4..b6963d2c 100644 --- a/openpgp/src/parse.rs +++ b/openpgp/src/parse.rs @@ -4423,11 +4423,6 @@ impl <'a> PacketParser<'a> { Box::new(buffered_reader::EOF::with_cookie( Default::default()))), self.recursion_depth())?; - // At this point, next() has to point to a non-container - // packet or an opaque container (due to the maximum recursion - // level being reaching). In this case, there can't be a fake - // EOF. - assert!(! fake_eof); self.last_path.clear(); self.last_path.extend_from_slice(&self.path[..]); @@ -5861,6 +5856,55 @@ mod test { } + /// We erroneously assumed that when BufferedReader::next() is + /// called, a SEIP container be opaque and hence there cannot be a + /// buffered_reader::Reserve on the stack with Cookie::fake_eof + /// set. But, we could simply call BufferedReader::next() after + /// the SEIP packet is decrypted, or buffer a SEIP packet's body, + /// then call BufferedReader::recurse(), which falls back to + /// BufferedReader::next() because some data has been read. + #[test] + fn issue_455() -> Result<()> { + let sk: SessionKey = + crate::fmt::hex::decode("3E99593760EE241488462BAFAE4FA268\ + 260B14B82D310D196DCEC82FD4F67678")?.into(); + let algo = SymmetricAlgorithm::AES256; + + // Decrypt, then call BufferedReader::next(). + eprintln!("Decrypt, then next():\n"); + let mut ppr = PacketParser::from_bytes( + crate::tests::message("encrypted-to-testy.gpg"))?; + while let PacketParserResult::Some(mut pp) = ppr { + match &pp.packet { + Packet::SEIP(_) => { + pp.decrypt(algo, &sk)?; + }, + _ => (), + } + // Used to trigger the assertion failure on the SEIP + // packet: + ppr = pp.next()?.1; + } + + // Decrypt, buffer, then call BufferedReader::recurse(). + eprintln!("\nDecrypt, buffer, then recurse():\n"); + let mut ppr = PacketParser::from_bytes( + crate::tests::message("encrypted-to-testy.gpg"))?; + while let PacketParserResult::Some(mut pp) = ppr { + match &pp.packet { + Packet::SEIP(_) => { + pp.decrypt(algo, &sk)?; + pp.buffer_unread_content()?; + }, + _ => (), + } + // Used to trigger the assertion failure on the SEIP + // packet: + ppr = pp.recurse()?.1; + } + Ok(()) + } + /// Crash in the AED parser due to missing chunk size validation. #[test] fn issue_514() -> Result<()> { -- cgit v1.2.3