Merge branch '1481-variant-parse-overflow' into 'master'

gvariant: Fix bounds checking in GVariant text format parser

Closes #1481

See merge request GNOME/glib!236
This commit is contained in:
Emmanuele Bassi 2018-08-09 10:25:48 +00:00
commit 0d271223d8
2 changed files with 44 additions and 16 deletions

View File

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

View File

@ -3889,27 +3889,48 @@ test_parse_failures (void)
"boolean 4", "8-9:", "can not parse as", "boolean 4", "8-9:", "can not parse as",
"int32 true", "6-10:", "can not parse as", "int32 true", "6-10:", "can not parse as",
"[double 5, int32 5]", "1-9,11-18:", "common type", "[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; gint i;
for (i = 0; i < G_N_ELEMENTS (test); i += 3) for (i = 0; i < G_N_ELEMENTS (test); i += 3)
{ {
GError *error = NULL; GError *error1 = NULL, *error2 = NULL;
GVariant *value; GVariant *value;
value = g_variant_parse (NULL, test[i], NULL, NULL, &error); /* Copy the test string and drop its nul terminator, then use the @limit
g_assert (value == NULL); * 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, 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, 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);
} }
} }