Commit 572f95f7 authored by Federico Mena Quintero's avatar Federico Mena Quintero
Browse files

(#515) - Add a limit for the number of loaded elements

This fixes the last part of #515, an enormous SVG file with millions
of elements, which causes out-of-memory.

To avoid unbounded memory consumption, we'll set a hard limit on the
number of loaded elements.  The largest legitimate file I have is a
map rendering with about 26K elements; here we set a limit of 200,000
elements for good measure.

Fixes #515
parent 9a32e5e3
......@@ -28,7 +28,15 @@ fn main() {
assert!(width > 0 && height > 0);
let handle = librsvg::Loader::new().read_path(input).unwrap();
let handle = match librsvg::Loader::new().read_path(input) {
Ok(handle) => handle,
Err(e) => {
eprintln!("loading error: {}", e);
process::exit(1);
}
};
let renderer = librsvg::CairoRenderer::new(&handle);
let surface = cairo::ImageSurface::create(cairo::Format::ARgb32, width, height).unwrap();
......
/// This is a mitigation for the security-related bug
/// https://gitlab.gnome.org/GNOME/librsvg/issues/323 - imagine
/// the XML [billion laughs attack], but done by creating deeply
/// nested groups of `<use>` elements. The first one references
/// the second one ten times, the second one references the third
/// one ten times, and so on. In the file given, this causes
/// 10^17 objects to be rendered. While this does not exhaust
/// memory, it would take a really long time.
/// This is a mitigation for the security-related bugs:
/// https://gitlab.gnome.org/GNOME/librsvg/issues/323
/// https://gitlab.gnome.org/GNOME/librsvg/issues/515
///
/// Imagine the XML [billion laughs attack], but done in SVG's terms:
///
/// - #323 above creates deeply nested groups of `<use>` elements.
/// The first one references the second one ten times, the second one
/// references the third one ten times, and so on. In the file given,
/// this causes 10^17 objects to be rendered. While this does not
/// exhaust memory, it would take a really long time.
///
/// - #515 has deeply nested references of `<pattern>` elements. Each
/// object inside each pattern has an attribute
/// fill="url(#next_pattern)", so the number of final rendered objects
/// grows exponentially.
///
/// We deal 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.
///
/// [billion laughs attack]: https://bitbucket.org/tiran/defusedxml
pub const MAX_REFERENCED_ELEMENTS: usize = 500_000;
/// This is a mitigation for SVG files which create millions of elements
/// in an attempt to exhaust memory. We don't allow loading more than
/// this number of elements during the initial streaming load process.
pub const MAX_LOADED_ELEMENTS: usize = 200_000;
......@@ -14,6 +14,7 @@ use crate::css::CssRules;
use crate::error::LoadingError;
use crate::handle::LoadOptions;
use crate::io::{self, get_input_stream_for_loading};
use crate::limits::MAX_LOADED_ELEMENTS;
use crate::node::{NodeData, NodeType, RsvgNode};
use crate::property_bag::PropertyBag;
use crate::style::NodeStyle;
......@@ -68,6 +69,7 @@ extern "C" {
/// what creates normal graphical elements.
struct XmlStateInner {
weak: Option<Weak<XmlState>>,
num_loaded_elements: usize,
tree_root: Option<RsvgNode>,
ids: Option<HashMap<String, RsvgNode>>,
css_rules: Option<CssRules>,
......@@ -107,6 +109,7 @@ impl XmlState {
XmlState {
inner: RefCell::new(XmlStateInner {
weak: None,
num_loaded_elements: 0,
tree_root: None,
ids: Some(HashMap::new()),
css_rules: Some(CssRules::default()),
......@@ -153,13 +156,29 @@ impl XmlState {
}
}
fn check_limits(&self) -> Result<(), ()> {
if self.inner.borrow().num_loaded_elements > MAX_LOADED_ELEMENTS {
self.error(ParseFromStreamError::XmlParseError(format!(
"cannot load more than {} XML elements",
MAX_LOADED_ELEMENTS
)));
Err(())
} else {
Ok(())
}
}
pub fn start_element(&self, name: &str, pbag: &PropertyBag) -> Result<(), ()> {
self.check_limits()?;
let context = self.inner.borrow().context();
if let Context::FatalError(_) = context {
return Err(());
}
self.inner.borrow_mut().num_loaded_elements += 1;
// FIXME: we should deal with namespaces at some point
let name = skip_namespace(name);
......@@ -328,15 +347,12 @@ impl XmlState {
}
fn element_creation_characters(&self, text: &str) {
let inner = self.inner.borrow();
let mut current_node = self.inner.borrow().current_node.as_ref().unwrap().clone();
if text.len() != 0 {
// When the last child is a Chars node we can coalesce
// the text and avoid screwing up the Pango layouts
let chars_node = if let Some(child) = inner
.current_node
.as_ref()
.unwrap()
let chars_node = if let Some(child) = current_node
.last_child()
.filter(|c| c.borrow().get_type() == NodeType::Chars)
{
......@@ -350,8 +366,8 @@ impl XmlState {
Box::new(NodeChars::new()),
));
let mut node = inner.current_node.as_ref().unwrap().clone();
node.append(child.clone());
self.inner.borrow_mut().num_loaded_elements += 1;
current_node.append(child.clone());
child
};
......
......@@ -24,9 +24,10 @@ get_test_filename (const char *basename) {
}
static void
test_non_svg_element (void)
test_loading_error (gconstpointer data)
{
char *filename = get_test_filename ("335-non-svg-element.svg");
const char *basename = data;
char *filename = get_test_filename (basename);
RsvgHandle *handle;
GError *error = NULL;
......@@ -67,7 +68,10 @@ main (int argc, char **argv)
{
g_test_init (&argc, &argv, NULL);
g_test_add_func ("/errors/non_svg_element", test_non_svg_element);
g_test_add_data_func_full ("/errors/non_svg_element",
"335-non-svg-element.svg",
test_loading_error,
NULL);
g_test_add_data_func_full ("/errors/instancing_limit/323-nested-use.svg",
"323-nested-use.svg",
......@@ -79,6 +83,11 @@ main (int argc, char **argv)
test_instancing_limit,
NULL);
g_test_add_data_func_full ("/errors/515-too-many-elements.svgz",
"515-too-many-elements.svgz",
test_loading_error,
NULL);
return g_test_run ();
}
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment