[librsvg/librsvg-2.50] (#691) Replace NumberList::Unbounded with NumberList::MaxLength



commit 7c0c44122e7df33236116a3a052dd997828128c6
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

 rsvg_internals/src/filters/component_transfer.rs |  3 ++-
 rsvg_internals/src/filters/convolve_matrix.rs    |  7 +++++--
 rsvg_internals/src/number_list.rs                | 12 ++++++++++++
 3 files changed, 19 insertions(+), 3 deletions(-)
---
diff --git a/rsvg_internals/src/filters/component_transfer.rs 
b/rsvg_internals/src/filters/component_transfer.rs
index aab6a0e5..235435ff 100644
--- a/rsvg_internals/src/filters/component_transfer.rs
+++ b/rsvg_internals/src/filters/component_transfer.rs
@@ -204,7 +204,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/rsvg_internals/src/filters/convolve_matrix.rs b/rsvg_internals/src/filters/convolve_matrix.rs
index a8f3f844..f64c10a4 100644
--- a/rsvg_internals/src/filters/convolve_matrix.rs
+++ b/rsvg_internals/src/filters/convolve_matrix.rs
@@ -155,10 +155,13 @@ 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/rsvg_internals/src/number_list.rs b/rsvg_internals/src/number_list.rs
index 3ae27ae2..00ed2c66 100644
--- a/rsvg_internals/src/number_list.rs
+++ b/rsvg_internals/src/number_list.rs
@@ -7,6 +7,7 @@ use crate::parsers::{optional_comma, Parse};
 
 #[derive(Eq, PartialEq)]
 pub enum NumberListLength {
+    MaxLength(usize),
     Exact(usize),
     Unbounded,
 }
@@ -20,6 +21,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(),
@@ -36,6 +42,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]