gvariant: Limit recursion in g_variant_parse()

The token parsing done by g_variant_parse() uses recursive function
calls, so at some point it will hit the stack limit. As with previous
changes to `GVariantType` parsing (commit 7c4e6e9fbe), limit the level
of nesting of containers parsed by g_variant_parse() to something
reasonable. We guarantee 64 levels of nesting, which should be enough
for anyone, and is the same as what we guarantee for types.

oss-fuzz#10286

Signed-off-by: Philip Withnall <withnall@endlessm.com>
This commit is contained in:
Philip Withnall 2019-10-18 13:50:09 +01:00
parent 3dec72b946
commit 25c2266a33
3 changed files with 65 additions and 18 deletions

View File

@ -29,6 +29,7 @@
#include "gstrfuncs.h" #include "gstrfuncs.h"
#include "gtestutils.h" #include "gtestutils.h"
#include "gvariant.h" #include "gvariant.h"
#include "gvariant-internal.h"
#include "gvarianttype.h" #include "gvarianttype.h"
#include "gslice.h" #include "gslice.h"
#include "gthread.h" #include "gthread.h"
@ -66,6 +67,7 @@
* @G_VARIANT_PARSE_ERROR_UNKNOWN_KEYWORD: an unknown keyword was encountered * @G_VARIANT_PARSE_ERROR_UNKNOWN_KEYWORD: an unknown keyword was encountered
* @G_VARIANT_PARSE_ERROR_UNTERMINATED_STRING_CONSTANT: unterminated string constant * @G_VARIANT_PARSE_ERROR_UNTERMINATED_STRING_CONSTANT: unterminated string constant
* @G_VARIANT_PARSE_ERROR_VALUE_EXPECTED: no value given * @G_VARIANT_PARSE_ERROR_VALUE_EXPECTED: no value given
* @G_VARIANT_PARSE_ERROR_RECURSION: variant was too deeply nested; #GVariant is only guaranteed to handle nesting up to 64 levels (Since: 2.64)
* *
* Error codes returned by parsing text-format GVariants. * Error codes returned by parsing text-format GVariants.
**/ **/
@ -640,6 +642,7 @@ ast_resolve (AST *ast,
static AST *parse (TokenStream *stream, static AST *parse (TokenStream *stream,
guint max_depth,
va_list *app, va_list *app,
GError **error); GError **error);
@ -825,6 +828,7 @@ maybe_free (AST *ast)
static AST * static AST *
maybe_parse (TokenStream *stream, maybe_parse (TokenStream *stream,
guint max_depth,
va_list *app, va_list *app,
GError **error) GError **error)
{ {
@ -838,7 +842,7 @@ maybe_parse (TokenStream *stream,
if (token_stream_consume (stream, "just")) if (token_stream_consume (stream, "just"))
{ {
child = parse (stream, app, error); child = parse (stream, max_depth - 1, app, error);
if (child == NULL) if (child == NULL)
return NULL; return NULL;
} }
@ -955,6 +959,7 @@ array_free (AST *ast)
static AST * static AST *
array_parse (TokenStream *stream, array_parse (TokenStream *stream,
guint max_depth,
va_list *app, va_list *app,
GError **error) GError **error)
{ {
@ -982,7 +987,7 @@ array_parse (TokenStream *stream,
error)) error))
goto error; goto error;
child = parse (stream, app, error); child = parse (stream, max_depth - 1, app, error);
if (!child) if (!child)
goto error; goto error;
@ -1093,6 +1098,7 @@ tuple_free (AST *ast)
static AST * static AST *
tuple_parse (TokenStream *stream, tuple_parse (TokenStream *stream,
guint max_depth,
va_list *app, va_list *app,
GError **error) GError **error)
{ {
@ -1121,7 +1127,7 @@ tuple_parse (TokenStream *stream,
error)) error))
goto error; goto error;
child = parse (stream, app, error); child = parse (stream, max_depth - 1, app, error);
if (!child) if (!child)
goto error; goto error;
@ -1199,6 +1205,7 @@ variant_free (AST *ast)
static AST * static AST *
variant_parse (TokenStream *stream, variant_parse (TokenStream *stream,
guint max_depth,
va_list *app, va_list *app,
GError **error) GError **error)
{ {
@ -1211,7 +1218,7 @@ variant_parse (TokenStream *stream,
AST *value; AST *value;
token_stream_assert (stream, "<"); token_stream_assert (stream, "<");
value = parse (stream, app, error); value = parse (stream, max_depth - 1, app, error);
if (!value) if (!value)
return NULL; return NULL;
@ -1385,6 +1392,7 @@ dictionary_free (AST *ast)
static AST * static AST *
dictionary_parse (TokenStream *stream, dictionary_parse (TokenStream *stream,
guint max_depth,
va_list *app, va_list *app,
GError **error) GError **error)
{ {
@ -1412,7 +1420,7 @@ dictionary_parse (TokenStream *stream,
return (AST *) dict; return (AST *) dict;
} }
if ((first = parse (stream, app, error)) == NULL) if ((first = parse (stream, max_depth - 1, app, error)) == NULL)
goto error; goto error;
ast_array_append (&dict->keys, &n_keys, first); ast_array_append (&dict->keys, &n_keys, first);
@ -1424,7 +1432,7 @@ dictionary_parse (TokenStream *stream,
error)) error))
goto error; goto error;
if ((first = parse (stream, app, error)) == NULL) if ((first = parse (stream, max_depth - 1, app, error)) == NULL)
goto error; goto error;
ast_array_append (&dict->values, &n_values, first); ast_array_append (&dict->values, &n_values, first);
@ -1449,7 +1457,7 @@ dictionary_parse (TokenStream *stream,
" or '}' to follow dictionary entry", error)) " or '}' to follow dictionary entry", error))
goto error; goto error;
child = parse (stream, app, error); child = parse (stream, max_depth - 1, app, error);
if (!child) if (!child)
goto error; goto error;
@ -1460,7 +1468,7 @@ dictionary_parse (TokenStream *stream,
" to follow dictionary entry key", error)) " to follow dictionary entry key", error))
goto error; goto error;
child = parse (stream, app, error); child = parse (stream, max_depth - 1, app, error);
if (!child) if (!child)
goto error; goto error;
@ -2192,6 +2200,7 @@ typedecl_free (AST *ast)
static AST * static AST *
typedecl_parse (TokenStream *stream, typedecl_parse (TokenStream *stream,
guint max_depth,
va_list *app, va_list *app,
GError **error) GError **error)
{ {
@ -2286,7 +2295,7 @@ typedecl_parse (TokenStream *stream,
} }
} }
if ((child = parse (stream, app, error)) == NULL) if ((child = parse (stream, max_depth - 1, app, error)) == NULL)
{ {
g_variant_type_free (type); g_variant_type_free (type);
return NULL; return NULL;
@ -2302,26 +2311,35 @@ typedecl_parse (TokenStream *stream,
static AST * static AST *
parse (TokenStream *stream, parse (TokenStream *stream,
guint max_depth,
va_list *app, va_list *app,
GError **error) GError **error)
{ {
SourceRef source_ref; SourceRef source_ref;
AST *result; AST *result;
if (max_depth == 0)
{
token_stream_set_error (stream, error, FALSE,
G_VARIANT_PARSE_ERROR_RECURSION,
"variant nested too deeply");
return NULL;
}
token_stream_prepare (stream); token_stream_prepare (stream);
token_stream_start_ref (stream, &source_ref); token_stream_start_ref (stream, &source_ref);
if (token_stream_peek (stream, '[')) if (token_stream_peek (stream, '['))
result = array_parse (stream, app, error); result = array_parse (stream, max_depth, app, error);
else if (token_stream_peek (stream, '(')) else if (token_stream_peek (stream, '('))
result = tuple_parse (stream, app, error); result = tuple_parse (stream, max_depth, app, error);
else if (token_stream_peek (stream, '<')) else if (token_stream_peek (stream, '<'))
result = variant_parse (stream, app, error); result = variant_parse (stream, max_depth, app, error);
else if (token_stream_peek (stream, '{')) else if (token_stream_peek (stream, '{'))
result = dictionary_parse (stream, app, error); result = dictionary_parse (stream, max_depth, app, error);
else if (app && token_stream_peek (stream, '%')) else if (app && token_stream_peek (stream, '%'))
result = positional_parse (stream, app, error); result = positional_parse (stream, app, error);
@ -2339,11 +2357,11 @@ parse (TokenStream *stream,
else if (token_stream_peek (stream, 'n') || else if (token_stream_peek (stream, 'n') ||
token_stream_peek (stream, 'j')) token_stream_peek (stream, 'j'))
result = maybe_parse (stream, app, error); result = maybe_parse (stream, max_depth, app, error);
else if (token_stream_peek (stream, '@') || else if (token_stream_peek (stream, '@') ||
token_stream_is_keyword (stream)) token_stream_is_keyword (stream))
result = typedecl_parse (stream, app, error); result = typedecl_parse (stream, max_depth, app, error);
else if (token_stream_peek (stream, '\'') || else if (token_stream_peek (stream, '\'') ||
token_stream_peek (stream, '"')) token_stream_peek (stream, '"'))
@ -2410,6 +2428,10 @@ parse (TokenStream *stream,
* Officially, the language understood by the parser is "any string * Officially, the language understood by the parser is "any string
* produced by g_variant_print()". * produced by g_variant_print()".
* *
* There may be implementation specific restrictions on deeply nested values,
* which would result in a %G_VARIANT_PARSE_ERROR_RECURSION error. #GVariant is
* guaranteed to handle nesting up to at least 64 levels.
*
* Returns: a non-floating reference to a #GVariant, or %NULL * Returns: a non-floating reference to a #GVariant, or %NULL
**/ **/
GVariant * GVariant *
@ -2430,7 +2452,7 @@ g_variant_parse (const GVariantType *type,
stream.stream = text; stream.stream = text;
stream.end = limit; stream.end = limit;
if ((ast = parse (&stream, NULL, error))) if ((ast = parse (&stream, G_VARIANT_MAX_RECURSION_DEPTH, NULL, error)))
{ {
if (type == NULL) if (type == NULL)
result = ast_resolve (ast, error); result = ast_resolve (ast, error);
@ -2515,7 +2537,7 @@ g_variant_new_parsed_va (const gchar *format,
stream.stream = format; stream.stream = format;
stream.end = NULL; stream.end = NULL;
if ((ast = parse (&stream, app, &error))) if ((ast = parse (&stream, G_VARIANT_MAX_RECURSION_DEPTH, app, &error)))
{ {
result = ast_resolve (ast, &error); result = ast_resolve (ast, &error);
ast_free (ast); ast_free (ast);

View File

@ -327,7 +327,8 @@ typedef enum
G_VARIANT_PARSE_ERROR_UNEXPECTED_TOKEN, G_VARIANT_PARSE_ERROR_UNEXPECTED_TOKEN,
G_VARIANT_PARSE_ERROR_UNKNOWN_KEYWORD, G_VARIANT_PARSE_ERROR_UNKNOWN_KEYWORD,
G_VARIANT_PARSE_ERROR_UNTERMINATED_STRING_CONSTANT, G_VARIANT_PARSE_ERROR_UNTERMINATED_STRING_CONSTANT,
G_VARIANT_PARSE_ERROR_VALUE_EXPECTED G_VARIANT_PARSE_ERROR_VALUE_EXPECTED,
G_VARIANT_PARSE_ERROR_RECURSION
} GVariantParseError; } GVariantParseError;
#define G_VARIANT_PARSE_ERROR (g_variant_parse_error_quark ()) #define G_VARIANT_PARSE_ERROR (g_variant_parse_error_quark ())

View File

@ -4180,6 +4180,29 @@ test_parser_integer_bounds (void)
#undef test_bound #undef test_bound
} }
/* Test that #GVariants which recurse too deeply are rejected. */
static void
test_parser_recursion (void)
{
GVariant *value = NULL;
GError *local_error = NULL;
const guint recursion_depth = G_VARIANT_MAX_RECURSION_DEPTH + 1;
gchar *silly_dict = g_malloc0 (recursion_depth * 2 + 1);
gsize i;
for (i = 0; i < recursion_depth; i++)
{
silly_dict[i] = '{';
silly_dict[recursion_depth * 2 - i - 1] = '}';
}
value = g_variant_parse (NULL, silly_dict, NULL, NULL, &local_error);
g_assert_error (local_error, G_VARIANT_PARSE_ERROR, G_VARIANT_PARSE_ERROR_RECURSION);
g_assert_null (value);
g_error_free (local_error);
g_free (silly_dict);
}
static void static void
test_parse_bad_format_char (void) test_parse_bad_format_char (void)
{ {
@ -5154,6 +5177,7 @@ main (int argc, char **argv)
g_test_add_func ("/gvariant/byteswap", test_gv_byteswap); g_test_add_func ("/gvariant/byteswap", test_gv_byteswap);
g_test_add_func ("/gvariant/parser", test_parses); g_test_add_func ("/gvariant/parser", test_parses);
g_test_add_func ("/gvariant/parser/integer-bounds", test_parser_integer_bounds); g_test_add_func ("/gvariant/parser/integer-bounds", test_parser_integer_bounds);
g_test_add_func ("/gvariant/parser/recursion", test_parser_recursion);
g_test_add_func ("/gvariant/parse-failures", test_parse_failures); g_test_add_func ("/gvariant/parse-failures", test_parse_failures);
g_test_add_func ("/gvariant/parse-positional", test_parse_positional); g_test_add_func ("/gvariant/parse-positional", test_parse_positional);
g_test_add_func ("/gvariant/parse/subprocess/bad-format-char", test_parse_bad_format_char); g_test_add_func ("/gvariant/parse/subprocess/bad-format-char", test_parse_bad_format_char);