luc14n0/glib
1
0
Fork 0
mirror of https://gitlab.gnome.org/GNOME/glib.git synced 2024-12-28 16:36:14 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge branch 'ossfuzz-9672-markup-overflow' into 'master'

Browse Source 
GMarkup buffer overflow fixes for error handling, round 2

See merge request GNOME/glib!422
This commit is contained in:
Philip Withnall 2018-10-29 22:03:05 +00:00
commit c2501a81f8
7 changed files with 93 additions and 75 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

36
glib/gmarkup.c
View File

@ -562,12 +562,14 @@ char_str (gunichar c,
* emitting it as hex escapes. */
static gchar*
utf8_str (const gchar *utf8,
gsize max_len,
gchar *buf)
{
gunichar c = g_utf8_get_char_validated (utf8, -1);
gunichar c = g_utf8_get_char_validated (utf8, max_len);
if (c == (gunichar) -1 || c == (gunichar) -2)
{
gchar *temp = g_strdup_printf ("\\x%02x", (guint)(guchar)*utf8);
guchar ch = (max_len > 0) ? (guchar) *utf8 : 0;
gchar *temp = g_strdup_printf ("\\x%02x", (guint) ch);
memset (buf, 0, 8);
memcpy (buf, temp, strlen (temp));
g_free (temp);
@ -1048,8 +1050,10 @@ emit_start_element (GMarkupParseContext *context,
tmp_error = NULL;
start_name = current_element (context);
if (context->parser->start_element &&
name_validate (context, start_name, error))
if (!name_validate (context, start_name, error))
return;
if (context->parser->start_element)
(* context->parser->start_element) (context,
start_name,
(const gchar **)attr_names,
@ -1222,7 +1226,8 @@ g_markup_parse_context_parse (GMarkupParseContext *context,
_("“%s” is not a valid character following "
"a “<” character; it may not begin an "
"element name"),
utf8_str (context->iter, buf));
utf8_str (context->iter,
context->current_text_end - context->iter, buf));
}
break;
@ -1263,7 +1268,8 @@ g_markup_parse_context_parse (GMarkupParseContext *context,
G_MARKUP_ERROR_PARSE,
_("Odd character “%s”, expected a “>” character "
"to end the empty-element tag “%s”"),
utf8_str (context->iter, buf),
utf8_str (context->iter,
context->current_text_end - context->iter, buf),
current_element (context));
}
break;
@ -1344,7 +1350,8 @@ g_markup_parse_context_parse (GMarkupParseContext *context,
G_MARKUP_ERROR_PARSE,
_("Odd character “%s”, expected a “=” after "
"attribute name “%s” of element “%s”"),
utf8_str (context->iter, buf),
utf8_str (context->iter,
context->current_text_end - context->iter, buf),
current_attribute (context),
current_element (context));
@ -1388,7 +1395,8 @@ g_markup_parse_context_parse (GMarkupParseContext *context,
"element “%s”, or optionally an attribute; "
"perhaps you used an invalid character in "
"an attribute name"),
utf8_str (context->iter, buf),
utf8_str (context->iter,
context->current_text_end - context->iter, buf),
current_element (context));
}
@ -1430,7 +1438,8 @@ g_markup_parse_context_parse (GMarkupParseContext *context,
_("Odd character “%s”, expected an open quote mark "
"after the equals sign when giving value for "
"attribute “%s” of element “%s”"),
utf8_str (context->iter, buf),
utf8_str (context->iter,
context->current_text_end - context->iter, buf),
current_attribute (context),
current_element (context));
}
@ -1563,8 +1572,10 @@ g_markup_parse_context_parse (GMarkupParseContext *context,
_("“%s” is not a valid character following "
"the characters “</”; “%s” may not begin an "
"element name"),
utf8_str (context->iter, buf),
utf8_str (context->iter, buf));
utf8_str (context->iter,
context->current_text_end - context->iter, buf),
utf8_str (context->iter,
context->current_text_end - context->iter, buf));
}
break;
@ -1599,7 +1610,8 @@ g_markup_parse_context_parse (GMarkupParseContext *context,
_("“%s” is not a valid character following "
"the close element name “%s”; the allowed "
"character is “>”"),
utf8_str (context->iter, buf),
utf8_str (context->iter,
context->current_text_end - context->iter, buf),
close_name->str);
}
else if (context->tag_stack == NULL)

2
glib/tests/Makefile.am
View File

@ -184,7 +184,7 @@ markup_tests = \
fail-36 fail-37 fail-38 fail-39 fail-40 \
fail-41 fail-42 fail-43 fail-44 fail-45 \
fail-46 fail-47 fail-48 fail-49 fail-50 \
fail-51 \
fail-51 fail-52 fail-53 \
valid-1 valid-2 valid-3 valid-4 valid-5 \
valid-6 valid-7 valid-8 valid-9 valid-10 \
valid-11 valid-12 valid-13 valid-14 valid-15 \

125
glib/tests/markup-parse.c
View File

@ -145,85 +145,86 @@ test_in_chunks (const gchar *contents,
return 0;
}
static int
test_file (const gchar *filename, GMarkupParseFlags flags)
/* Load the given @filename and parse it multiple times with different chunking
* and length handling. All results should be equal. %TRUE is returned if the
* file was parsed successfully on every attempt; %FALSE if it failed to parse
* on every attempt. The test aborts if some attempts succeed and some fail. */
static gboolean
test_file (const gchar *filename,
GMarkupParseFlags flags)
{
gchar *contents;
gsize length;
GError *error;
gchar *contents = NULL, *contents_unterminated = NULL;
gsize length_bytes;
GError *local_error = NULL;
GMarkupParseContext *context;
gint line, col;
guint n_failures = 0;
guint n_tests = 0;
const gsize chunk_sizes_bytes[] = { 1, 2, 5, 12, 1024 };
gsize i;
GString *first_string = NULL;
error = NULL;
if (!g_file_get_contents (filename,
&contents,
&length,
&error))
{
fprintf (stderr, "%s\n", error->message);
g_error_free (error);
return 1;
}
g_file_get_contents (filename, &contents, &length_bytes, &local_error);
g_assert_no_error (local_error);
/* Make a copy of the contents with no trailing nul. */
contents_unterminated = g_malloc (length_bytes);
if (contents_unterminated != NULL)
memcpy (contents_unterminated, contents, length_bytes);
/* Test with nul termination. */
context = g_markup_parse_context_new (&parser, flags, NULL, NULL);
g_assert (g_markup_parse_context_get_user_data (context) == NULL);
g_markup_parse_context_get_position (context, &line, &col);
g_assert (line == 1 && col == 1);
g_assert_cmpint (line, ==, 1);
g_assert_cmpint (col, ==, 1);
if (!g_markup_parse_context_parse (context, contents, length, NULL))
{
g_markup_parse_context_free (context);
g_free (contents);
return 1;
}
if (!g_markup_parse_context_end_parse (context, NULL))
{
g_markup_parse_context_free (context);
g_free (contents);
return 1;
}
if (!g_markup_parse_context_parse (context, contents, -1, NULL) ||
!g_markup_parse_context_end_parse (context, NULL))
n_failures++;
n_tests++;
g_markup_parse_context_free (context);
/* A byte at a time */
if (test_in_chunks (contents, length, 1, flags) != 0)
{
g_free (contents);
return 1;
}
/* FIXME: Swap out the error string so we only return one copy of it, not
* @n_tests copies. This should be fixed properly by eliminating the global
* state in this file. */
first_string = g_steal_pointer (&string);
string = g_string_new ("");
/* 2 bytes */
if (test_in_chunks (contents, length, 2, flags) != 0)
{
g_free (contents);
return 1;
}
/* With the length specified explicitly and a nul terminator present (since
* g_file_get_contents() always adds one). */
if (test_in_chunks (contents, length_bytes, length_bytes, flags) != 0)
n_failures++;
n_tests++;
/* 5 bytes */
if (test_in_chunks (contents, length, 5, flags) != 0)
{
g_free (contents);
return 1;
}
/* With the length specified explicitly and no nul terminator present. */
if (test_in_chunks (contents_unterminated, length_bytes, length_bytes, flags) != 0)
n_failures++;
n_tests++;
/* 12 bytes */
if (test_in_chunks (contents, length, 12, flags) != 0)
/* In various sized chunks. */
for (i = 0; i < G_N_ELEMENTS (chunk_sizes_bytes); i++)
{
g_free (contents);
return 1;
}
/* 1024 bytes */
if (test_in_chunks (contents, length, 1024, flags) != 0)
{
g_free (contents);
return 1;
if (test_in_chunks (contents, length_bytes, chunk_sizes_bytes[i], flags) != 0)
n_failures++;
n_tests++;
}
g_free (contents);
g_free (contents_unterminated);
return 0;
/* FIXME: Restore the error string. */
g_string_free (string, TRUE);
string = g_steal_pointer (&first_string);
/* We expect the file to either always be parsed successfully, or never be
* parsed successfully. There’s a bug in GMarkup if it sometimes parses
* successfully depending on how you chunk or terminate the input. */
if (n_failures > 0)
g_assert_cmpint (n_failures, ==, n_tests);
return (n_failures == 0);
}
static gchar *
@ -256,7 +257,7 @@ test_parse (gconstpointer d)
gchar *expected;
gboolean valid_input;
GError *error = NULL;
gint res;
gboolean res;
valid_input = strstr (filename, "valid") != NULL;
expected_file = get_expected_filename (filename, 0);
@ -265,7 +266,7 @@ test_parse (gconstpointer d)
string = g_string_sized_new (0);
res = test_file (filename, 0);
g_assert_cmpint (res, ==, valid_input ? 0 : 1);
g_assert_cmpint (res, ==, valid_input);
g_file_get_contents (expected_file, &expected, NULL, &error);
g_assert_no_error (error);
@ -283,7 +284,7 @@ test_parse (gconstpointer d)
string = g_string_sized_new (0);
res = test_file (filename, G_MARKUP_TREAT_CDATA_AS_TEXT);
g_assert_cmpint (res, ==, valid_input ? 0 : 1);
g_assert_cmpint (res, ==, valid_input);
g_file_get_contents (expected_file, &expected, NULL, &error);
g_assert_no_error (error);

1
glib/tests/markups/fail-52.expected Normal file
View File

@ -0,0 +1 @@
ERROR Error on line 1 char 5: Odd character “\xfc”, expected a “=” after attribute name “r” of element “”

1
glib/tests/markups/fail-52.gmarkup Normal file
View File

@ -0,0 +1 @@
< r <20>

2
glib/tests/markups/fail-53.expected Normal file
View File

@ -0,0 +1,2 @@
ELEMENT 'r'
ERROR Error on line 1 char 4: Odd character “\xfc”, expected a “>” character to end the empty-element tag “r”

1
glib/tests/markups/fail-53.gmarkup Normal file
View File

@ -0,0 +1 @@
<r/<2F>