[glib/glib-2-62: 1/2] gvariant: Limit recursion in g_variant_parse()

[Simon McVittie <smcv src gnome org>]

commit 89c2ead766020020690c9c812bf5aea202ce3bd9
Author: Philip Withnall <withnall endlessm com>
Date:   Fri Oct 18 13:50:09 2019 +0100

    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 7c4e6e9fbe4), 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.
    
    (Backport to 2.62: Dropped the new `G_VARIANT_PARSE_ERROR_RECURSION`
    error code in favour of `G_VARIANT_PARSE_ERROR_FAILED`, to avoid adding
    API.)
    
    oss-fuzz#10286
    
    Signed-off-by: Philip Withnall <withnall endlessm com>

 glib/gvariant-parser.c | 55 ++++++++++++++++++++++++++++++++++----------------
 glib/tests/gvariant.c  | 24 ++++++++++++++++++++++
 2 files changed, 62 insertions(+), 17 deletions(-)
---
diff --git a/glib/gvariant-parser.c b/glib/gvariant-parser.c
index 5e96c1321..7d481f4b4 100644
--- a/glib/gvariant-parser.c
+++ b/glib/gvariant-parser.c
@@ -29,6 +29,7 @@
 #include "gstrfuncs.h"
 #include "gtestutils.h"
 #include "gvariant.h"
+#include "gvariant-internal.h"
 #include "gvarianttype.h"
 #include "gslice.h"
 #include "gthread.h"
@@ -640,6 +641,7 @@ ast_resolve (AST     *ast,
 
 
 static AST *parse (TokenStream  *stream,
+                   guint         max_depth,
                    va_list      *app,
                    GError      **error);
 
@@ -825,6 +827,7 @@ maybe_free (AST *ast)
 
 static AST *
 maybe_parse (TokenStream  *stream,
+             guint         max_depth,
              va_list      *app,
              GError      **error)
 {
@@ -838,7 +841,7 @@ maybe_parse (TokenStream  *stream,
 
   if (token_stream_consume (stream, "just"))
     {
-      child = parse (stream, app, error);
+      child = parse (stream, max_depth - 1, app, error);
       if (child == NULL)
         return NULL;
     }
@@ -955,6 +958,7 @@ array_free (AST *ast)
 
 static AST *
 array_parse (TokenStream  *stream,
+             guint         max_depth,
              va_list      *app,
              GError      **error)
 {
@@ -982,7 +986,7 @@ array_parse (TokenStream  *stream,
                                  error))
         goto error;
 
-      child = parse (stream, app, error);
+      child = parse (stream, max_depth - 1, app, error);
 
       if (!child)
         goto error;
@@ -1093,6 +1097,7 @@ tuple_free (AST *ast)
 
 static AST *
 tuple_parse (TokenStream  *stream,
+             guint         max_depth,
              va_list      *app,
              GError      **error)
 {
@@ -1121,7 +1126,7 @@ tuple_parse (TokenStream  *stream,
                                  error))
         goto error;
 
-      child = parse (stream, app, error);
+      child = parse (stream, max_depth - 1, app, error);
 
       if (!child)
         goto error;
@@ -1199,6 +1204,7 @@ variant_free (AST *ast)
 
 static AST *
 variant_parse (TokenStream  *stream,
+               guint         max_depth,
                va_list      *app,
                GError      **error)
 {
@@ -1211,7 +1217,7 @@ variant_parse (TokenStream  *stream,
   AST *value;
 
   token_stream_assert (stream, "<");
-  value = parse (stream, app, error);
+  value = parse (stream, max_depth - 1, app, error);
 
   if (!value)
     return NULL;
@@ -1385,6 +1391,7 @@ dictionary_free (AST *ast)
 
 static AST *
 dictionary_parse (TokenStream  *stream,
+                  guint         max_depth,
                   va_list      *app,
                   GError      **error)
 {
@@ -1412,7 +1419,7 @@ dictionary_parse (TokenStream  *stream,
       return (AST *) dict;
     }
 
-  if ((first = parse (stream, app, error)) == NULL)
+  if ((first = parse (stream, max_depth - 1, app, error)) == NULL)
     goto error;
 
   ast_array_append (&dict->keys, &n_keys, first);
@@ -1424,7 +1431,7 @@ dictionary_parse (TokenStream  *stream,
                              error))
     goto error;
 
-  if ((first = parse (stream, app, error)) == NULL)
+  if ((first = parse (stream, max_depth - 1, app, error)) == NULL)
     goto error;
 
   ast_array_append (&dict->values, &n_values, first);
@@ -1449,7 +1456,7 @@ dictionary_parse (TokenStream  *stream,
                                  " or '}' to follow dictionary entry", error))
         goto error;
 
-      child = parse (stream, app, error);
+      child = parse (stream, max_depth - 1, app, error);
 
       if (!child)
         goto error;
@@ -1460,7 +1467,7 @@ dictionary_parse (TokenStream  *stream,
                                  " to follow dictionary entry key", error))
         goto error;
 
-      child = parse (stream, app, error);
+      child = parse (stream, max_depth - 1, app, error);
 
       if (!child)
         goto error;
@@ -2192,6 +2199,7 @@ typedecl_free (AST *ast)
 
 static AST *
 typedecl_parse (TokenStream  *stream,
+                guint         max_depth,
                 va_list      *app,
                 GError      **error)
 {
@@ -2286,7 +2294,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);
       return NULL;
@@ -2302,26 +2310,35 @@ typedecl_parse (TokenStream  *stream,
 
 static AST *
 parse (TokenStream  *stream,
+       guint         max_depth,
        va_list      *app,
        GError      **error)
 {
   SourceRef source_ref;
   AST *result;
 
+  if (max_depth == 0)
+    {
+      token_stream_set_error (stream, error, FALSE,
+                              G_VARIANT_PARSE_ERROR_FAILED,
+                              "variant nested too deeply");
+      return NULL;
+    }
+
   token_stream_prepare (stream);
   token_stream_start_ref (stream, &source_ref);
 
   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, '('))
-    result = tuple_parse (stream, app, error);
+    result = tuple_parse (stream, max_depth, app, error);
 
   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, '{'))
-    result = dictionary_parse (stream, app, error);
+    result = dictionary_parse (stream, max_depth, app, error);
 
   else if (app && token_stream_peek (stream, '%'))
     result = positional_parse (stream, app, error);
@@ -2339,11 +2356,11 @@ parse (TokenStream  *stream,
 
   else if (token_stream_peek (stream, 'n') ||
            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, '@') ||
            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, '\'') ||
            token_stream_peek (stream, '"'))
@@ -2410,6 +2427,10 @@ parse (TokenStream  *stream,
  * Officially, the language understood by the parser is "any string
  * produced by g_variant_print()".
  *
+ * There may be implementation specific restrictions on deeply nested values,
+ * which would result in a %G_VARIANT_PARSE_ERROR_FAILED error. #GVariant is
+ * guaranteed to handle nesting up to at least 64 levels.
+ *
  * Returns: a non-floating reference to a #GVariant, or %NULL
  **/
 GVariant *
@@ -2430,7 +2451,7 @@ g_variant_parse (const GVariantType  *type,
   stream.stream = text;
   stream.end = limit;
 
-  if ((ast = parse (&stream, NULL, error)))
+  if ((ast = parse (&stream, G_VARIANT_MAX_RECURSION_DEPTH, NULL, error)))
     {
       if (type == NULL)
         result = ast_resolve (ast, error);
@@ -2515,7 +2536,7 @@ g_variant_new_parsed_va (const gchar *format,
   stream.stream = format;
   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);
       ast_free (ast);
diff --git a/glib/tests/gvariant.c b/glib/tests/gvariant.c
index 927ae4e12..0e5ec8efa 100644
--- a/glib/tests/gvariant.c
+++ b/glib/tests/gvariant.c
@@ -4180,6 +4180,29 @@ test_parser_integer_bounds (void)
 #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_FAILED);
+  g_assert_null (value);
+  g_error_free (local_error);
+  g_free (silly_dict);
+}
+
 static 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/parser", test_parses);
   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-positional", test_parse_positional);
   g_test_add_func ("/gvariant/parse/subprocess/bad-format-char", test_parse_bad_format_char);