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

gvariant: Fix bounds checking in GVariant text format parser

Browse Source 
The token_stream_peek() functions were not doing any bounds checking, so
could potentially read 1 byte off the end of the input blob. This was
never noticed, since the input stream is almost always a nul-terminated
string. However, g_variant_parse() does allow non-nul-terminated strings
to be used with a @limit parameter, and the bugs become apparent under
valgrind if that parameter is used.

This includes modifications to the test cases to cover the
non-nul-terminated case.

Spotted by ossfuzz.

Signed-off-by: Philip Withnall <withnall@endlessm.com>
This commit is contained in:
Philip Withnall 2018-08-09 01:00:20 +01:00
parent 291bb0c584
commit 10ee7301e8
2 changed files with 44 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
glib/gvariant-parser.c
View File

@ -260,6 +260,9 @@ token_stream_prepare (TokenStream *stream)
stream->this = stream->stream;
stream->stream = end;
/* We must have at least one byte in a token. */
g_assert (stream->stream - stream->this >= 1);
return TRUE;
}
@ -276,7 +279,8 @@ token_stream_peek (TokenStream *stream,
if (!token_stream_prepare (stream))
return FALSE;
return stream->this[0] == first_char;
return stream->stream - stream->this >= 1 &&
stream->this[0] == first_char;
}
static gboolean
@ -287,7 +291,8 @@ token_stream_peek2 (TokenStream *stream,
if (!token_stream_prepare (stream))
return FALSE;
return stream->this[0] == first_char &&
return stream->stream - stream->this >= 2 &&
stream->this[0] == first_char &&
stream->this[1] == second_char;
}
@ -297,7 +302,8 @@ token_stream_is_keyword (TokenStream *stream)
if (!token_stream_prepare (stream))
return FALSE;
return g_ascii_isalpha (stream->this[0]) &&
return stream->stream - stream->this >= 2 &&
g_ascii_isalpha (stream->this[0]) &&
g_ascii_isalpha (stream->this[1]);
}
@ -307,10 +313,11 @@ token_stream_is_numeric (TokenStream *stream)
if (!token_stream_prepare (stream))
return FALSE;
return (g_ascii_isdigit (stream->this[0]) ||
stream->this[0] == '-' ||
stream->this[0] == '+' ||
stream->this[0] == '.');
return (stream->stream - stream->this >= 1 &&
(g_ascii_isdigit (stream->this[0]) ||
stream->this[0] == '-' ||
stream->this[0] == '+' ||
stream->this[0] == '.'));
}
static gboolean

39
glib/tests/gvariant.c
View File

@ -3889,27 +3889,48 @@ test_parse_failures (void)
"boolean 4", "8-9:", "can not parse as",
"int32 true", "6-10:", "can not parse as",
"[double 5, int32 5]", "1-9,11-18:", "common type",
"string 4", "7-8:", "can not parse as"
"string 4", "7-8:", "can not parse as",
"\x0a", "1:", "expected value",
"((", "2:", "expected value",
};
gint i;
for (i = 0; i < G_N_ELEMENTS (test); i += 3)
{
GError *error = NULL;
GError *error1 = NULL, *error2 = NULL;
GVariant *value;
value = g_variant_parse (NULL, test[i], NULL, NULL, &error);
g_assert (value == NULL);
/* Copy the test string and drop its nul terminator, then use the @limit
* parameter of g_variant_parse() to set the length. This allows valgrind
* to catch 1-byte heap buffer overflows. */
gsize test_len = MAX (strlen (test[i]), 1);
gchar *test_blob = g_malloc0 (test_len); /* no nul terminator */
if (!strstr (error->message, test[i+2]))
memcpy (test_blob, test[i], test_len);
value = g_variant_parse (NULL, test_blob, test_blob + test_len, NULL, &error1);
g_assert_null (value);
g_free (test_blob);
if (!strstr (error1->message, test[i+2]))
g_error ("test %d: Can't find '%s' in '%s'", i / 3,
test[i+2], error->message);
test[i+2], error1->message);
if (!g_str_has_prefix (error->message, test[i+1]))
if (!g_str_has_prefix (error1->message, test[i+1]))
g_error ("test %d: Expected location '%s' in '%s'", i / 3,
test[i+1], error->message);
test[i+1], error1->message);
g_error_free (error);
/* Test again with the nul terminator this time. The behaviour should be
* the same. */
value = g_variant_parse (NULL, test[i], NULL, NULL, &error2);
g_assert_null (value);
g_assert_cmpint (error1->domain, ==, error2->domain);
g_assert_cmpint (error1->code, ==, error2->code);
g_assert_cmpstr (error1->message, ==, error2->message);
g_clear_error (&error1);
g_clear_error (&error2);
}
}