[librsvg: 2/5] (#691) Replace NumberList::Unbounded with NumberList::MaxLength




commit e273ab9a28b82fd9b567c6a6b3c3e7409a84e89e
Author: madds-h <madeline hollandart io>
Date:   Sun Apr 18 22:25:10 2021 -0500

    (#691) Replace NumberList::Unbounded with NumberList::MaxLength
    
    Limits size of convolve_matrix matrix and component_transfer
    tableValues lists to mitigate malicious SVGs
    Implements MaxLength, similar to Exact but only returns
    if the limit is met
    
    https://gitlab.gnome.org/GNOME/librsvg/-/issues/691

 src/filters/component_transfer.rs |  3 ++-
 src/filters/convolve_matrix.rs    |  8 +++++---
 src/parsers.rs                    | 12 ++++++++++++
 3 files changed, 19 insertions(+), 4 deletions(-)
---
diff --git a/src/filters/component_transfer.rs b/src/filters/component_transfer.rs
index 4b8c152e..a10e619b 100644
--- a/src/filters/component_transfer.rs
+++ b/src/filters/component_transfer.rs
@@ -217,7 +217,8 @@ macro_rules! func_x {
                         expanded_name!("", "type") => self.function_type = attr.parse(value)?,
                         expanded_name!("", "tableValues") => {
                             let NumberList(v) =
-                                NumberList::parse_str(value, NumberListLength::Unbounded)
+                                // #691: Limit list to 256 to mitigate malicious SVGs
+                                NumberList::parse_str(value, NumberListLength::MaxLength(256))
                                     .attribute(attr)?;
                             self.table_values = v;
                         }
diff --git a/src/filters/convolve_matrix.rs b/src/filters/convolve_matrix.rs
index 1e4318a1..c75d9b5a 100644
--- a/src/filters/convolve_matrix.rs
+++ b/src/filters/convolve_matrix.rs
@@ -105,10 +105,12 @@ impl SetAttributes for FeConvolveMatrix {
 
                 // #352: Parse as an unbounded list rather than exact length to prevent aborts due
                 //       to huge allocation attempts by underlying Vec::with_capacity().
-                let NumberList(v) = NumberList::parse_str(value, NumberListLength::Unbounded)
+                // #691: Limit list to 400 (20x20) to mitigate malicious SVGs
+                let NumberList(v) = NumberList::parse_str(value, NumberListLength::MaxLength(400))
                     .attribute(attr.clone())?;
-
-                if v.len() != number_of_elements {
+                // #691: Update check as v.len can be different than number of elements because
+                //       of the above limit (and will = 400 if that happens)
+                if v.len() != number_of_elements && v.len() != 400 {
                     return Err(ValueErrorKind::value_error(&format!(
                         "incorrect number of elements: expected {}",
                         number_of_elements
diff --git a/src/parsers.rs b/src/parsers.rs
index 59c8dc6e..44a70933 100644
--- a/src/parsers.rs
+++ b/src/parsers.rs
@@ -133,6 +133,7 @@ impl Parse for u32 {
 
 #[derive(Eq, PartialEq)]
 pub enum NumberListLength {
+    MaxLength(usize),
     Exact(usize),
     Unbounded,
 }
@@ -147,6 +148,11 @@ impl NumberList {
         length: NumberListLength,
     ) -> Result<Self, ParseError<'i>> {
         let mut v = match length {
+            NumberListLength::MaxLength(l) if l > 0 => Vec::<f64>::with_capacity(l),
+            NumberListLength::MaxLength(_) => {
+                //cargo fmt suggests this formatting for some reason
+                unreachable!("NumberListLength::MaxLength cannot be 0")
+            }
             NumberListLength::Exact(l) if l > 0 => Vec::<f64>::with_capacity(l),
             NumberListLength::Exact(_) => unreachable!("NumberListLength::Exact cannot be 0"),
             NumberListLength::Unbounded => Vec::<f64>::new(),
@@ -163,6 +169,12 @@ impl NumberList {
 
             v.push(f64::parse(parser)?);
 
+            if let NumberListLength::MaxLength(l) = length {
+                if i + 1 == l {
+                    break;
+                }
+            }
+
             if let NumberListLength::Exact(l) = length {
                 if i + 1 == l {
                     break;


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]