[librsvg] rsvg_defs_lookup(): Do not allow looking up extern references

[Federico Mena Quintero <federico src gnome org>]

commit 3559b3e620446cd99b446e31156c156df846b860
Author: Federico Mena Quintero <federico gnome org>
Date:   Thu Nov 29 10:49:17 2018 -0600

    rsvg_defs_lookup(): Do not allow looking up extern references
    
    This function gets called directly from the public API, and a calling
    application should not be allowed to lookup an element with a name
    like "some-random-file#element_id", that is, the app should not be
    able to cause files to be read if they are not within the set of
    resources that the SVG actually references.
    
    The test is robust (only fragment IDs without a URL are allowed), but
    will inadvertently print a g_warning if someone runs rsvg-convert like
    
        rsvg-convert -i 'foo#bar' -o x.png x.svg
    
    We may be able to get rid of that g_warning once the public API is
    implemented in Rust, so it can have access to the URL parsing machinery.

 rsvg_internals/src/defs.rs | 33 +++++++++++++++++++++++++++++----
 tests/api.c                | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 61 insertions(+), 4 deletions(-)
---
diff --git a/rsvg_internals/src/defs.rs b/rsvg_internals/src/defs.rs
index c156bd01..049c64a8 100644
--- a/rsvg_internals/src/defs.rs
+++ b/rsvg_internals/src/defs.rs
@@ -7,6 +7,7 @@ use std::rc::Rc;
 use allowed_url::AllowedUrl;
 use handle::{self, RsvgHandle};
 use node::{Node, RsvgNode};
+use util::rsvg_g_warning;
 use util::utf8_cstr;
 
 pub enum RsvgDefs {}
@@ -190,10 +191,34 @@ pub extern "C" fn rsvg_defs_lookup(
     }
 
     match r.unwrap() {
-        Href::WithFragment(fragment) => match defs.lookup(handle, &fragment) {
-            Some(n) => n as *const RsvgNode,
-            None => ptr::null(),
-        },
+        Href::WithFragment(fragment) => {
+            if let Some(uri) = fragment.uri() {
+                // The public APIs to get geometries of individual elements, or to render
+                // them, should only allow referencing elements within the main handle's
+                // SVG file; that is, only plain "#foo" fragment IDs are allowed here.
+                // Otherwise, a calling program could request "another-file#foo" and cause
+                // another-file to be loaded, even if it is not part of the set of
+                // resources that the main SVG actually references.  In the future we may
+                // relax this requirement to allow lookups within that set, but not to
+                // other random files.
+
+                let msg = format!(
+                    "the public API is not allowed to look up external references: {}#{}",
+                    uri,
+                    fragment.fragment()
+                );
+
+                rsvg_log!("{}", msg);
+
+                rsvg_g_warning(&msg);
+                return ptr::null();
+            }
+
+            match defs.lookup(handle, &fragment) {
+                Some(n) => n as *const RsvgNode,
+                None => ptr::null(),
+            }
+        }
 
         _ => unreachable!(),
     }
diff --git a/tests/api.c b/tests/api.c
index 587f06d5..4d0553f6 100644
--- a/tests/api.c
+++ b/tests/api.c
@@ -648,6 +648,37 @@ empty_write_close (void)
     g_object_unref (handle);
 }
 
+static void
+cannot_request_external_elements (void)
+{
+    if (g_test_subprocess ()) {
+        /* We want to test that using one of the _sub() functions will fail
+         * if the element's id is within an external file.  First, ensure
+         * that the main file and the external file actually exist.
+         */
+
+        char *filename = get_test_filename ("example.svg");
+
+        RsvgHandle *handle;
+        GError *error = NULL;
+        RsvgPositionData pos;
+
+        handle = rsvg_handle_new_from_file (filename, &error);
+        g_free (filename);
+
+        g_assert (handle != NULL);
+        g_assert (error == NULL);
+
+        g_assert (rsvg_handle_get_position_sub (handle, &pos, "dpi.svg#one") == FALSE);
+
+        g_object_unref (handle);
+    }
+
+    g_test_trap_subprocess (NULL, 0, 0);
+    g_test_trap_assert_failed ();
+    g_test_trap_assert_stderr ("*WARNING*the public API is not allowed to look up external references*");
+}
+
 int
 main (int argc, char **argv)
 {
@@ -681,6 +712,7 @@ main (int argc, char **argv)
     g_test_add_func ("/api/render_cairo_sub", render_cairo_sub);
     g_test_add_func ("/api/no_write_before_close", no_write_before_close);
     g_test_add_func ("/api/empty_write_close", empty_write_close);
+    g_test_add_func ("/api/cannot_request_external_elements", cannot_request_external_elements);
 
     return g_test_run ();
 }