Detect reference cycles when parsing streams (with nom_parser)

src/document.rs

@@ -7,7 +7,7 @@ use crate::{Error, Result, Stream};
 use encoding_rs::UTF_16BE;
 use log::info;
 use std::cmp::max;
-use std::collections::{BTreeMap, HashMap};
+use std::collections::{BTreeMap, HashMap, HashSet};
 use std::io::Write;
 use std::str;
 
@@ -421,24 +421,29 @@ impl Document {
 
     /// Get resources used by a page.
     pub fn get_page_resources(&self, page_id: ObjectId) -> (Option<&Dictionary>, Vec<ObjectId>) {
-        fn collect_resources(page_node: &Dictionary, resource_ids: &mut Vec<ObjectId>, doc: &Document) {
-            if let Ok(resources_id) = page_node.get(b"Resources").and_then(Object::as_reference) {
-                resource_ids.push(resources_id);
+        fn collect_resources(
+            page_node: &Dictionary, resource_ids: &mut Vec<ObjectId>, doc: &Document,
+            already_seen: &mut HashSet<ObjectId>,
+        ) -> Result<()> {
+            if let Ok(resource_id) = page_node.get(b"Resources").and_then(Object::as_reference) {
+                resource_ids.push(resource_id);
             }
-            if let Ok(page_tree) = page_node
-                .get(b"Parent")
-                .and_then(Object::as_reference)
-                .and_then(|id| doc.get_dictionary(id))
-            {
-                collect_resources(page_tree, resource_ids, doc);
+            if let Ok(parent_id) = page_node.get(b"Parent").and_then(Object::as_reference) {
+                if already_seen.contains(&parent_id) {
+                    return Err(Error::ReferenceCycle);
+                }
+                already_seen.insert(parent_id);
+                let parent_dict = doc.get_dictionary(parent_id)?;
+                collect_resources(parent_dict, resource_ids, doc, already_seen)?;
             }
+            Ok(())
         }
 
         let mut resource_dict = None;
         let mut resource_ids = Vec::new();
         if let Ok(page) = self.get_dictionary(page_id) {
             resource_dict = page.get(b"Resources").and_then(Object::as_dict).ok();
-            collect_resources(page, &mut resource_ids, self);
+            collect_resources(page, &mut resource_ids, self, &mut HashSet::new()).unwrap();
         }
         (resource_dict, resource_ids)
     }

src/error.rs

@@ -31,6 +31,8 @@ pub enum Error {
     PageNumberNotFound(u32),
     /// Invalid object while parsing at offset.
     Parse { offset: usize },
+    /// Dereferencing object failed due to a reference cycle.
+    ReferenceCycle,
     /// Dereferencing object reached the limit.
     /// This might indicate a reference loop.
     ReferenceLimit,
@@ -67,7 +69,8 @@ impl fmt::Display for Error {
             Error::Offset(o) => write!(f, "Invalid file offset: {}", o),
             Error::PageNumberNotFound(p) => write!(f, "Page number {} could not be found", p),
             Error::Parse { offset, .. } => write!(f, "Invalid object at byte {}", offset),
-            Error::ReferenceLimit => write!(f, "Could not dereference an object; possible reference loop"),
+            Error::ReferenceCycle => write!(f, "Could not dereference an object; reference cycle detected"),
+            Error::ReferenceLimit => write!(f, "Could not dereference an object; possible reference cycle"),
             Error::StringDecode => write!(f, "Could not decode string"),
             Error::Syntax(msg) => write!(f, "Syntax error: {}", msg),
             Error::Trailer => write!(f, "Invalid file trailer"),

src/nom_parser.rs

@@ -3,6 +3,7 @@ use crate::content::*;
 use crate::error::XrefError;
 use crate::xref::*;
 use crate::Error;
+use std::collections::HashSet;
 use std::str::{self, FromStr};
 
 use nom::branch::alt;
@@ -270,12 +271,12 @@ fn dictionary(input: &[u8]) -> NomResult<Dictionary> {
     )(input)
 }
 
-fn stream<'a>(input: &'a [u8], reader: &Reader) -> NomResult<'a, Object> {
+fn stream<'a>(input: &'a [u8], reader: &Reader, already_seen: &mut HashSet<ObjectId>) -> NomResult<'a, Object> {
     let (i, dict) = terminated(dictionary, tuple((space, tag(b"stream"), eol)))(input)?;
 
     if let Ok(length) = dict.get(b"Length").and_then(|value| {
         if let Ok(id) = value.as_reference() {
-            reader.get_object(id).and_then(|value| value.as_i64())
+            reader.get_object(id, already_seen).and_then(|value| value.as_i64())
         } else {
             value.as_i64()
         }
@@ -326,22 +327,25 @@ pub fn direct_object(input: &[u8]) -> Option<Object> {
     strip_nom(_direct_object(input))
 }
 
-fn object<'a>(input: &'a [u8], reader: &Reader) -> NomResult<'a, Object> {
-    terminated(alt((|input| stream(input, reader), _direct_objects)), space)(input)
+fn object<'a>(input: &'a [u8], reader: &Reader, already_seen: &mut HashSet<ObjectId>) -> NomResult<'a, Object> {
+    terminated(
+        alt((|input| stream(input, reader, already_seen), _direct_objects)),
+        space,
+    )(input)
 }
 
 pub fn indirect_object(
-    input: &[u8], offset: usize, expected_id: Option<ObjectId>, reader: &Reader,
+    input: &[u8], offset: usize, expected_id: Option<ObjectId>, reader: &Reader, already_seen: &mut HashSet<ObjectId>,
 ) -> crate::Result<(ObjectId, Object)> {
-    let (id, mut object) = _indirect_object(&input[offset..], offset, expected_id, reader)?;
+    let (id, mut object) = _indirect_object(&input[offset..], offset, expected_id, reader, already_seen)?;
 
     offset_stream(&mut object, offset);
 
     Ok((id, object))
 }
 
 fn _indirect_object(
-    input: &[u8], offset: usize, expected_id: Option<ObjectId>, reader: &Reader,
+    input: &[u8], offset: usize, expected_id: Option<ObjectId>, reader: &Reader, already_seen: &mut HashSet<ObjectId>,
 ) -> crate::Result<(ObjectId, Object)> {
     let (i, (_, object_id)) =
         terminated(tuple((space, object_id)), pair(tag(b"obj"), space))(input).map_err(|_| Error::Parse { offset })?;
@@ -352,8 +356,11 @@ fn _indirect_object(
     }
 
     let object_offset = input.len() - i.len();
-    let (_, mut object) = terminated(|i| object(i, reader), tuple((space, opt(tag(b"endobj")), space)))(i)
-        .map_err(|_| Error::Parse { offset })?;
+    let (_, mut object) = terminated(
+        |i| object(i, reader, already_seen),
+        tuple((space, opt(tag(b"endobj")), space)),
+    )(i)
+    .map_err(|_| Error::Parse { offset })?;
 
     offset_stream(&mut object, object_offset);
 
@@ -418,7 +425,7 @@ pub fn xref_and_trailer(input: &[u8], reader: &Reader) -> crate::Result<(Xref, D
             Ok((xref, trailer))
         }),
         (|input| {
-            _indirect_object(input, 0, None, reader)
+            _indirect_object(input, 0, None, reader, &mut HashSet::new())
                 .map(|(_, obj)| {
                     let res = match obj {
                         Object::Stream(stream) => decode_xref_stream(stream),

src/reader.rs

@@ -2,7 +2,7 @@
 
 use log::{error, warn};
 use std::cmp;
-use std::collections::BTreeMap;
+use std::collections::{BTreeMap, HashSet};
 use std::convert::TryInto;
 #[cfg(not(feature = "async"))]
 use std::fs::File;
@@ -273,7 +273,7 @@ impl<'a> Reader<'a> {
         let entries_filter_map = |(_, entry): (&_, &_)| {
             if let XrefEntry::Normal { offset, .. } = *entry {
                 let (object_id, mut object) = self
-                    .read_object(offset as usize, None)
+                    .read_object(offset as usize, None, &mut HashSet::new())
                     .map_err(|e| error!("Object load error: {:?}", e))
                     .ok()?;
                 if let Some(filter_func) = filter_func {
@@ -363,12 +363,22 @@ impl<'a> Reader<'a> {
         let object = self.document.get_object(object_id)?;
         let stream = object.as_stream()?;
 
-        stream.dict.get(b"Length").and_then(|value| {
-            if let Ok(id) = value.as_reference() {
-                return self.document.get_object(id).and_then(Object::as_i64);
-            }
-            value.as_i64()
-        })
+        stream
+            .dict
+            .get(b"Length")
+            .and_then(|value| {
+                if let Ok(id) = value.as_reference() {
+                    return self.document.get_object(id).and_then(Object::as_i64);
+                }
+                value.as_i64()
+            })
+            .map_err(|err| {
+                error!(
+                    "Stream dictionary of '{} {} R' is missing the Length entry",
+                    object_id.0, object_id.1
+                );
+                err
+            })
     }
 
     /// Get object offset by object id.
@@ -386,19 +396,26 @@ impl<'a> Reader<'a> {
         }
     }
 
-    pub fn get_object(&self, id: ObjectId) -> Result<Object> {
+    pub fn get_object(&self, id: ObjectId, already_seen: &mut HashSet<ObjectId>) -> Result<Object> {
+        if already_seen.contains(&id) {
+            warn!("reference cycle detected resolving object {} {}", id.0, id.1);
+            return Err(Error::ReferenceCycle);
+        }
+        already_seen.insert(id);
         let offset = self.get_offset(id)?;
-        let (_, obj) = self.read_object(offset as usize, Some(id))?;
+        let (_, obj) = self.read_object(offset as usize, Some(id), already_seen)?;
 
         Ok(obj)
     }
 
-    fn read_object(&self, offset: usize, expected_id: Option<ObjectId>) -> Result<(ObjectId, Object)> {
+    fn read_object(
+        &self, offset: usize, expected_id: Option<ObjectId>, already_seen: &mut HashSet<ObjectId>,
+    ) -> Result<(ObjectId, Object)> {
         if offset > self.buffer.len() {
             return Err(Error::Offset(offset));
         }
 
-        parser::indirect_object(self.buffer, offset, expected_id, self)
+        parser::indirect_object(self.buffer, offset, expected_id, self, already_seen)
     }
 
     fn get_xref_start(buffer: &[u8]) -> Result<usize> {