[librsvg/librsvg-2.48] (#588) - Fix panic when a big viewBox does creates an invalid transform



commit 0c86abd926582a9ed8da04743b9faac47d0f449f
Author: Federico Mena Quintero <federico gnome org>
Date:   Wed Apr 22 11:00:15 2020 -0500

    (#588) - Fix panic when a big viewBox does creates an invalid transform
    
    The example file, created through fuzzing, has a viewBox="0 0 6E20 540".
    
    This creates a non-invertible transform in
    AspectRatio.viewport_to_viewbox_transform(), and when the calling code
    sets that transform on a cr, it panics because the cr ends up in an
    error state.
    
    This commit makes viewport_to_viewbox_transform() return a Result, so
    that it returns Err when it would create an invalid transform.
    
    Fixes one of the cases in https://gitlab.gnome.org/GNOME/librsvg/-/issues/588

 rsvg_internals/src/aspect_ratio.rs                 | 87 +++++++++++++++++++---
 rsvg_internals/src/drawing_ctx.rs                  | 17 +++++
 .../588-big-viewbox-yields-invalid-transform.svg   | 23 ++++++
 3 files changed, 117 insertions(+), 10 deletions(-)
---
diff --git a/rsvg_internals/src/aspect_ratio.rs b/rsvg_internals/src/aspect_ratio.rs
index a8c9a82c..be06a8a4 100644
--- a/rsvg_internals/src/aspect_ratio.rs
+++ b/rsvg_internals/src/aspect_ratio.rs
@@ -150,13 +150,23 @@ impl AspectRatio {
         }
     }
 
-    // Computes the viewport to viewbox transformation, or returns None
-    // if the vbox has 0 width or height.
+    /// Computes the viewport to viewbox transformation.
+    ///
+    /// Given a viewport, returns a transformation that will create a coordinate
+    /// space inside it.  The `(vbox.x0, vbox.y0)` will be mapped to the viewport's
+    /// upper-left corner, and the `(vbox.x1, vbox.y1)` will be mapped to the viewport's
+    /// lower-right corner.
+    ///
+    /// If the vbox or viewport are empty, returns `Ok(None)`.  Per the SVG spec, either
+    /// of those mean that the corresponding element should not be rendered.
+    ///
+    /// If the vbox would create an invalid transform (say, a vbox with huge numbers that
+    /// leads to a near-zero scaling transform), returns an `Err(())`.
     pub fn viewport_to_viewbox_transform(
         &self,
         vbox: Option<ViewBox>,
         viewport: &Rect,
-    ) -> Option<Transform> {
+    ) -> Result<Option<Transform>, ()> {
         // width or height set to 0 disables rendering of the element
         // https://www.w3.org/TR/SVG/struct.html#SVGElementWidthAttribute
         // https://www.w3.org/TR/SVG/struct.html#UseElementWidthAttribute
@@ -164,25 +174,30 @@ impl AspectRatio {
         // https://www.w3.org/TR/SVG/painting.html#MarkerWidthAttribute
 
         if viewport.is_empty() {
-            return None;
+            return Ok(None);
         }
 
         // the preserveAspectRatio attribute is only used if viewBox is specified
         // https://www.w3.org/TR/SVG/coords.html#PreserveAspectRatioAttribute
-        if let Some(vbox) = vbox {
+        let transform = if let Some(vbox) = vbox {
             if vbox.0.is_empty() {
                 // Width or height of 0 for the viewBox disables rendering of the element
                 // https://www.w3.org/TR/SVG/coords.html#ViewBoxAttribute
-                None
+                return Ok(None);
             } else {
                 let r = self.compute(&vbox, viewport);
-                let t = Transform::new_translate(r.x0, r.y0)
+                Transform::new_translate(r.x0, r.y0)
                     .pre_scale(r.width() / vbox.0.width(), r.height() / vbox.0.height())
-                    .pre_translate(-vbox.0.x0, -vbox.0.y0);
-                Some(t)
+                    .pre_translate(-vbox.0.x0, -vbox.0.y0)
             }
         } else {
-            Some(Transform::new_translate(viewport.x0, viewport.y0))
+            Transform::new_translate(viewport.x0, viewport.y0)
+        };
+
+        if transform.is_invertible() {
+            Ok(Some(transform))
+        } else {
+            Err(())
         }
     }
 }
@@ -404,4 +419,56 @@ mod tests {
         let foo = foo.compute(&viewbox, &Rect::from_size(10.0, 1.0));
         assert_rect_equal(&foo, &Rect::new(0.0, -99.0, 10.0, 1.0));
     }
+
+    #[test]
+    fn empty_viewport() {
+        let a = AspectRatio::default();
+        let t = a.viewport_to_viewbox_transform(
+            Some(ViewBox::parse_str("10 10 40 40").unwrap()),
+            &Rect::from_size(0.0, 0.0),
+        );
+
+        assert_eq!(t, Ok(None));
+    }
+
+    #[test]
+    fn empty_viewbox() {
+        let a = AspectRatio::default();
+        let t = a.viewport_to_viewbox_transform(
+            Some(ViewBox::parse_str("10 10 0 0").unwrap()),
+            &Rect::from_size(10.0, 10.0),
+        );
+
+        assert_eq!(t, Ok(None));
+    }
+
+    #[test]
+    fn valid_viewport_and_viewbox() {
+        let a = AspectRatio::default();
+        let t = a.viewport_to_viewbox_transform(
+            Some(ViewBox::parse_str("10 10 40 40").unwrap()),
+            &Rect::new(1.0, 1.0, 2.0, 2.0),
+        );
+
+        assert_eq!(
+            t,
+            Ok(Some(
+                Transform::identity()
+                    .pre_translate(1.0, 1.0)
+                    .pre_scale(0.025, 0.025)
+                    .pre_translate(-10.0, -10.0)
+            ))
+        );
+    }
+
+    #[test]
+    fn invalid_viewbox() {
+        let a = AspectRatio::default();
+        let t = a.viewport_to_viewbox_transform(
+            Some(ViewBox::parse_str("0 0 6E20 540").unwrap()),
+            &Rect::new(1.0, 1.0, 2.0, 2.0),
+        );
+
+        assert_eq!(t, Err(()));
+    }
 }
diff --git a/rsvg_internals/src/drawing_ctx.rs b/rsvg_internals/src/drawing_ctx.rs
index afa33959..4f46d55b 100644
--- a/rsvg_internals/src/drawing_ctx.rs
+++ b/rsvg_internals/src/drawing_ctx.rs
@@ -295,6 +295,23 @@ impl DrawingCtx {
 
         preserve_aspect_ratio
             .viewport_to_viewbox_transform(vbox, &viewport)
+            .unwrap_or_else(|_e: ()| {
+                match vbox {
+                    None => unreachable!(
+                        "viewport_to_viewbox_transform only returns errors when vbox != None"
+                    ),
+                    Some(v) => {
+                        rsvg_log!(
+                            "ignoring viewBox ({}, {}, {}, {}) since it is not usable",
+                            v.0.x0,
+                            v.0.y0,
+                            v.0.width(),
+                            v.0.height()
+                        );
+                    }
+                }
+                None // so that the following and_then() won't run
+            })
             .and_then(|t| {
                 self.cr.transform(t.into());
 
diff --git a/tests/fixtures/render-crash/588-big-viewbox-yields-invalid-transform.svg 
b/tests/fixtures/render-crash/588-big-viewbox-yields-invalid-transform.svg
new file mode 100644
index 00000000..3e29cbe1
--- /dev/null
+++ b/tests/fixtures/render-crash/588-big-viewbox-yields-invalid-transform.svg
@@ -0,0 +1,23 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<svg version="1.1" xmlns="http://www.w3.org/2000/svg"; xmlns:xlink="http://www.w3.org/1999/xlink"; width="30" 
height="27.00" viewBox="0 0 6E20 540">
+  <defs>
+    <mask id="Mask_big_ex_small" maskUnits="userSpaceOnUse" x="0" y="0" width="6420" height="540">
+  <g>
+       <use xlink:href="#big" fill="white"/>
+       <use xlink:href="#small" fill="black"/>
+      </g>
+    </mask>
+    <g id="big_ex_small">
+      <use xlink:href="#big" mask="url(#Mask_big_ex_small)"/>
+    </g>
+    <mask id="Region0" maskUnits="userSpaceOnUse" x="0" y="0" width="6420" height="540" fill-rule="nonzero">
+      <use xlink:href="#big_ex_small" fill="white"/>
+    </mask>
+    <rect id="big" x="0" y="0" width="6420" height="540"/> <rect id="small" x="2760" y="20" width="900" 
height="480"/>
+  </defs>
+  <g mask="url(#Region0)">
+    <g transform="matrix(1.66667 0 0 1.66667 0 0)">
+      <rect x="0" y="0" width="6420" height="540" fill="black"/>
+    </g>
+  </g>
+</svg>


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