From 17e9858cb0d666d2d93b2fd253394eb746d4703f Mon Sep 17 00:00:00 2001 From: Jan Starke Date: Thu, 13 Apr 2023 15:18:05 +0200 Subject: [PATCH 1/2] add watcher for assert --- src/binxml/deserializer.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/binxml/deserializer.rs b/src/binxml/deserializer.rs index 1d1f2ccc..6060908b 100644 --- a/src/binxml/deserializer.rs +++ b/src/binxml/deserializer.rs @@ -219,12 +219,15 @@ impl<'a> IterTokens<'a> { } let deserialized_token_result = self.visit_token(&mut cursor, t); - debug_assert!( - cursor.position() >= offset_from_chunk_start, - "Invalid state, cursor position at entering loop {}, now at {}", - offset_from_chunk_start, - cursor.position() - ); + // omit the assert if we ran into error previously + if deserialized_token_result.is_ok() { + debug_assert!( + cursor.position() >= offset_from_chunk_start, + "Invalid state, cursor position at entering loop {}, now at {}", + offset_from_chunk_start, + cursor.position() + ); + } Some(deserialized_token_result) } From c43a856c8664b838409f9e7cf3e352540b794ef1 Mon Sep 17 00:00:00 2001 From: Jan Starke Date: Thu, 13 Apr 2023 15:46:12 +0200 Subject: [PATCH 2/2] fix endless recursion --- src/binxml/tokens.rs | 71 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 63 insertions(+), 8 deletions(-) diff --git a/src/binxml/tokens.rs b/src/binxml/tokens.rs index 6e754175..d1aa0df8 100644 --- a/src/binxml/tokens.rs +++ b/src/binxml/tokens.rs @@ -247,11 +247,33 @@ pub fn read_substitution_descriptor( }) } +enum HeuristicsBehaviour { + ProhibitHeuristics { original_cursor_position: u64 }, + AllowHeuristics, +} + +#[inline] pub fn read_open_start_element( cursor: &mut Cursor<&[u8]>, chunk: Option<&EvtxChunk>, has_attributes: bool, is_substitution: bool, +) -> Result { + read_open_start_element_with_heuristics( + cursor, + chunk, + has_attributes, + is_substitution, + HeuristicsBehaviour::AllowHeuristics, + ) +} + +fn read_open_start_element_with_heuristics( + cursor: &mut Cursor<&[u8]>, + chunk: Option<&EvtxChunk>, + has_attributes: bool, + is_substitution: bool, + heuristics_behaviour: HeuristicsBehaviour, ) -> Result { trace!( "Offset `0x{:08x}` - OpenStartElement", @@ -284,14 +306,47 @@ pub fn read_open_start_element( "Detected a case where `dependency_identifier` should not have been read. \ Trying to read again without it." ); - cursor.seek(SeekFrom::Current(-6)).map_err(|e| { - WrappedIoError::io_error_with_message( - e, - "failed to skip when recovering from `dependency_identifier` hueristic", - cursor, - ) - })?; - return read_open_start_element(cursor, chunk, has_attributes, true); + + match heuristics_behaviour { + HeuristicsBehaviour::AllowHeuristics => { + let original_position = cursor.position(); + cursor.seek(SeekFrom::Current(-6)).map_err(|e| { + WrappedIoError::io_error_with_message( + e, + "failed to skip when recovering from `dependency_identifier` heuristic", + cursor, + ) + })?; + return read_open_start_element_with_heuristics( + cursor, + chunk, + has_attributes, + true, + HeuristicsBehaviour::ProhibitHeuristics { + original_cursor_position: original_position, + }, + ); + } + HeuristicsBehaviour::ProhibitHeuristics { + original_cursor_position, + } => { + // we are already in the recursive descent call, so the heuristic + // attempt has failed. We restore the cursor position ... + cursor.seek(SeekFrom::Start(original_cursor_position)).map_err(|e| { + WrappedIoError::io_error_with_message( + e, + "failed to restore original cursor position during `dependency_identifier` heuristic", + cursor, + ) + })?; + + // and inform about the failure. + return Err(DeserializationError::InvalidToken { + value: 0, + offset: cursor.position(), + }); + } + } } }