GNOME.org
 

[librsvg: 31/32] Use an ImplementationLimit enum to detail the LimitExceeded errors


commit 43d42701c4947c5cf88981029b1e19c517f3218e
Author: Federico Mena Quintero <federico gnome org>
Date:   Fri Dec 4 13:43:00 2020 -0600

    Use an ImplementationLimit enum to detail the LimitExceeded errors
    
    Now both {LoadingError, RenderingError}::LimitExceeded have an
    ImplementationLimit detail.  This is perhaps important information to
    convey precisely to the caller, instead of a squishy string.

 src/api.rs          |  2 +-
 src/document.rs     |  4 +--
 src/drawing_ctx.rs  |  8 +++---
 src/error.rs        | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++---
 src/paint_server.rs | 10 +++++---
 src/xml/mod.rs      |  9 +++----
 tests/src/errors.rs | 10 +++++---
 7 files changed, 90 insertions(+), 24 deletions(-)
---
diff --git a/src/api.rs b/src/api.rs
index a7a97acc..228a1d61 100644
--- a/src/api.rs
+++ b/src/api.rs
@@ -144,7 +144,7 @@
 #![warn(missing_docs)]
 
 pub use crate::{
-    error::{LoadingError, RenderingError},
+    error::{ImplementationLimit, LoadingError, RenderingError},
     length::{LengthUnit, RsvgLength as Length},
 };
 
diff --git a/src/document.rs b/src/document.rs
index 1e84a695..19d01bf5 100644
--- a/src/document.rs
+++ b/src/document.rs
@@ -260,9 +260,7 @@ fn image_loading_error_from_cairo(status: cairo::Status, aurl: &AllowedUrl) -> L
 
     match status {
         cairo::Status::NoMemory => LoadingError::OutOfMemory(format!("loading image: {}", url)),
-        cairo::Status::InvalidSize => {
-            LoadingError::LimitExceeded(format!("image too big: {}", url))
-        }
+        cairo::Status::InvalidSize => LoadingError::Other(format!("image too big: {}", url)),
         _ => LoadingError::Other(format!("cairo error: {}", status)),
     }
 }
diff --git a/src/drawing_ctx.rs b/src/drawing_ctx.rs
index acc11a21..8dd4b264 100644
--- a/src/drawing_ctx.rs
+++ b/src/drawing_ctx.rs
@@ -16,7 +16,7 @@ use crate::dasharray::Dasharray;
 use crate::document::AcquiredNodes;
 use crate::dpi::Dpi;
 use crate::element::Element;
-use crate::error::{AcquireError, RenderingError};
+use crate::error::{AcquireError, ImplementationLimit, RenderingError};
 use crate::filter::FilterValue;
 use crate::filters;
 use crate::float_eq_cairo::ApproxEqCairo;
@@ -1644,9 +1644,9 @@ impl DrawingCtx {
             }
 
             Err(AcquireError::MaxReferencesExceeded) => {
-                return Err(RenderingError::LimitExceeded(String::from(
-                    "maximum number of referenced objects",
-                )));
+                return Err(RenderingError::LimitExceeded(
+                    ImplementationLimit::TooManyReferencedElements,
+                ));
             }
 
             Err(AcquireError::InvalidLinkType(_)) => unreachable!(),
diff --git a/src/error.rs b/src/error.rs
index c65eca50..4fdcc7b9 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -7,6 +7,7 @@ use cssparser::{BasicParseError, BasicParseErrorKind, ParseErrorKind, ToCss};
 use markup5ever::QualName;
 
 use crate::io::IoError;
+use crate::limits;
 use crate::node::Node;
 use crate::url_resolver::Fragment;
 
@@ -127,7 +128,7 @@ pub enum RenderingError {
     Rendering(String),
 
     /// A particular implementation-defined limit was exceeded.
-    LimitExceeded(String),
+    LimitExceeded(ImplementationLimit),
 
     /// Tried to reference an SVG element that does not exist.
     IdNotFound,
@@ -154,7 +155,7 @@ impl fmt::Display for RenderingError {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match *self {
             RenderingError::Rendering(ref s) => write!(f, "rendering error: {}", s),
-            RenderingError::LimitExceeded(ref s) => write!(f, "limit exceeded: {}", s),
+            RenderingError::LimitExceeded(ref l) => write!(f, "{}", l),
             RenderingError::IdNotFound => write!(f, "element id not found"),
             RenderingError::InvalidId(ref s) => write!(f, "invalid id: {:?}", s),
             RenderingError::OutOfMemory(ref s) => write!(f, "out of memory: {}", s),
@@ -341,12 +342,56 @@ pub enum LoadingError {
     Io(String),
 
     /// A particular implementation-defined limit was exceeded.
-    LimitExceeded(String),
+    LimitExceeded(ImplementationLimit),
 
     /// Catch-all for loading errors.
     Other(String),
 }
 
+/// Errors for implementation-defined limits, to mitigate malicious SVG documents.
+///
+/// These get emitted as `LoadingError::LimitExceeded` or `RenderingError::LimitExceeded`.
+/// The limits are present to mitigate malicious SVG documents which may try to exhaust
+/// all available memory, or which would use large amounts of CPU time.
+#[non_exhaustive]
+#[derive(Debug, Copy, Clone)]
+pub enum ImplementationLimit {
+    /// Document exceeded the maximum number of times that elements
+    /// can be referenced through URL fragments.
+    ///
+    /// This is a mitigation for malicious documents that attempt to
+    /// consume exponential amounts of CPU time by creating millions
+    /// of references to SVG elements.  For example, the `<use>` and
+    /// `<pattern>` elements allow referencing other elements, which
+    /// can in turn reference other elements.  This can be used to
+    /// create documents which would require exponential amounts of
+    /// CPU time to be rendered.
+    ///
+    /// Librsvg deals with both cases by placing a limit on how many
+    /// references will be resolved during the SVG rendering process,
+    /// that is, how many `url(#foo)` will be resolved.
+    ///
+    /// These malicious documents are similar to the XML
+    /// [billion laughs attack], but done with SVG's referencing features.
+    ///
+    /// See issues
+    /// [#323](https://gitlab.gnome.org/GNOME/librsvg/issues/323) and
+    /// [#515](https://gitlab.gnome.org/GNOME/librsvg/issues/515) for
+    /// examples for the `<use>` and `<pattern>` elements,
+    /// respectively.
+    ///
+    /// [billion laughs attack]: https://bitbucket.org/tiran/defusedxml
+    TooManyReferencedElements,
+
+    /// Document exceeded the maximum number of elements that can be loaded.
+    ///
+    /// This is a mitigation for SVG files which create millions of
+    /// elements in an attempt to exhaust memory.  Librsvg does not't
+    /// allow loading more than a certain number of elements during
+    /// the initial loading process.
+    TooManyLoadedElements,
+}
+
 impl error::Error for LoadingError {}
 
 impl fmt::Display for LoadingError {
@@ -358,7 +403,7 @@ impl fmt::Display for LoadingError {
             LoadingError::BadCss => write!(f, "invalid CSS"),
             LoadingError::NoSvgRoot => write!(f, "XML does not have <svg> root"),
             LoadingError::Io(ref s) => write!(f, "I/O error: {}", s),
-            LoadingError::LimitExceeded(ref s) => write!(f, "limit exceeded: {}", s),
+            LoadingError::LimitExceeded(ref l) => write!(f, "{}", l),
             LoadingError::Other(ref s) => write!(f, "{}", s),
         }
     }
@@ -380,3 +425,21 @@ impl From<IoError> for LoadingError {
         }
     }
 }
+
+impl fmt::Display for ImplementationLimit {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match *self {
+            ImplementationLimit::TooManyReferencedElements => write!(
+                f,
+                "exceeded more than {} referenced elements",
+                limits::MAX_REFERENCED_ELEMENTS
+            ),
+
+            ImplementationLimit::TooManyLoadedElements => write!(
+                f,
+                "cannot load more than {} XML elements",
+                limits::MAX_LOADED_ELEMENTS
+            ),
+        }
+    }
+}
diff --git a/src/paint_server.rs b/src/paint_server.rs
index 73e9287d..8390bce2 100644
--- a/src/paint_server.rs
+++ b/src/paint_server.rs
@@ -6,7 +6,9 @@ use crate::bbox::BoundingBox;
 use crate::document::AcquiredNodes;
 use crate::drawing_ctx::DrawingCtx;
 use crate::element::Element;
-use crate::error::*;
+use crate::error::{
+    AcquireError, HrefError, ImplementationLimit, ParseError, RenderingError, ValueErrorKind,
+};
 use crate::gradient::{ResolvedGradient, UserSpaceGradient};
 use crate::node::NodeBorrow;
 use crate::parsers::Parse;
@@ -104,9 +106,9 @@ impl PaintServer {
                 .or_else(|err| match (err, alternate) {
                     (AcquireError::MaxReferencesExceeded, _) => {
                         rsvg_log!("exceeded maximum number of referenced objects");
-                        Err(RenderingError::LimitExceeded(String::from(
-                            "maximum number of referenced objects",
-                        )))
+                        Err(RenderingError::LimitExceeded(
+                            ImplementationLimit::TooManyReferencedElements,
+                        ))
                     }
 
                     // The following two cases catch AcquireError::CircularReference, which for
diff --git a/src/xml/mod.rs b/src/xml/mod.rs
index a7c125de..20f07409 100644
--- a/src/xml/mod.rs
+++ b/src/xml/mod.rs
@@ -21,7 +21,7 @@ use xml5ever::tokenizer::{TagKind, Token, TokenSink, XmlTokenizer, XmlTokenizerO
 
 use crate::attributes::Attributes;
 use crate::document::{Document, DocumentBuilder};
-use crate::error::LoadingError;
+use crate::error::{ImplementationLimit, LoadingError};
 use crate::io::{self, IoError};
 use crate::limits::MAX_LOADED_ELEMENTS;
 use crate::node::{Node, NodeBorrow};
@@ -165,10 +165,9 @@ impl XmlState {
 
     fn check_limits(&self) -> Result<(), ()> {
         if self.inner.borrow().num_loaded_elements > MAX_LOADED_ELEMENTS {
-            self.error(LoadingError::LimitExceeded(format!(
-                "cannot load more than {} XML elements",
-                MAX_LOADED_ELEMENTS
-            )));
+            self.error(LoadingError::LimitExceeded(
+                ImplementationLimit::TooManyLoadedElements,
+            ));
             Err(())
         } else {
             Ok(())
diff --git a/tests/src/errors.rs b/tests/src/errors.rs
index 6ef35603..b315fe2b 100644
--- a/tests/src/errors.rs
+++ b/tests/src/errors.rs
@@ -11,7 +11,7 @@
 #![cfg(test)]
 
 use cairo;
-use librsvg::{CairoRenderer, Loader, LoadingError, RenderingError};
+use librsvg::{CairoRenderer, ImplementationLimit, Loader, LoadingError, RenderingError};
 
 #[ignore]
 #[test]
@@ -20,7 +20,9 @@ fn too_many_elements() {
 
     assert!(matches!(
         Loader::new().read_path(name),
-        Err(LoadingError::LimitExceeded(_))
+        Err(LoadingError::LimitExceeded(
+            ImplementationLimit::TooManyLoadedElements
+        ))
     ));
 }
 
@@ -45,7 +47,9 @@ fn rendering_instancing_limit(name: &str) {
                 height: 500.0,
             },
         ),
-        Err(RenderingError::LimitExceeded(_))
+        Err(RenderingError::LimitExceeded(
+            ImplementationLimit::TooManyReferencedElements
+        ))
     ));
 }
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]