From 57fc0be857245af04dfd2819fdc040d71440a7a0 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Wed, 18 Sep 2019 17:06:20 +0100 Subject: [PATCH] gmarkup: Add a limit on the number of attributes in an element MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While the XML specification doesn’t prescribe a limit, no reasonable bit of XML is going to have more than 1000 attributes in a single XML element. Adding a limit reduces the changes of a runaway allocation loop caused by dodgy input. oss-fuzz#12960 Signed-off-by: Philip Withnall --- glib/gmarkup.c | 18 ++++++++++++++++-- glib/tests/markups/fail-54.expected | 1 + glib/tests/markups/fail-54.gmarkup | 1 + 3 files changed, 18 insertions(+), 2 deletions(-) create mode 100644 glib/tests/markups/fail-54.expected create mode 100644 glib/tests/markups/fail-54.gmarkup diff --git a/glib/gmarkup.c b/glib/gmarkup.c index daa856f46..ba4dfd2e4 100644 --- a/glib/gmarkup.c +++ b/glib/gmarkup.c @@ -970,9 +970,13 @@ current_attribute (GMarkupParseContext *context) return context->attr_names[context->cur_attr]->str; } -static void +static gboolean add_attribute (GMarkupParseContext *context, GString *str) { + /* Sanity check on the number of attributes. */ + if (context->cur_attr >= 1000) + return FALSE; + if (context->cur_attr + 2 >= context->alloc_attrs) { context->alloc_attrs += 5; /* silly magic number */ @@ -984,6 +988,8 @@ add_attribute (GMarkupParseContext *context, GString *str) context->attr_values[context->cur_attr] = NULL; context->attr_names[context->cur_attr+1] = NULL; context->attr_values[context->cur_attr+1] = NULL; + + return TRUE; } static void @@ -1332,7 +1338,15 @@ g_markup_parse_context_parse (GMarkupParseContext *context, if (!name_validate (context, context->partial_chunk->str, error)) break; - add_attribute (context, context->partial_chunk); + if (!add_attribute (context, context->partial_chunk)) + { + set_error (context, + error, + G_MARKUP_ERROR_PARSE, + _("Too many attributes in element “%s”"), + current_element (context)); + break; + } context->partial_chunk = NULL; context->start = NULL; diff --git a/glib/tests/markups/fail-54.expected b/glib/tests/markups/fail-54.expected new file mode 100644 index 000000000..0fc721a32 --- /dev/null +++ b/glib/tests/markups/fail-54.expected @@ -0,0 +1 @@ +ERROR Error on line 1 char 7908: Too many attributes in element “r” diff --git a/glib/tests/markups/fail-54.gmarkup b/glib/tests/markups/fail-54.gmarkup new file mode 100644 index 000000000..239108208 --- /dev/null +++ b/glib/tests/markups/fail-54.gmarkup @@ -0,0 +1 @@ +