From cc0b8bdf124c47090d0b794c9b6e2e3852c860d9 Mon Sep 17 00:00:00 2001
From: Hanno Boeck <hanno@hboeck.de>
Date: Mon, 22 Feb 2016 07:46:17 -0500
Subject: [PATCH] GVariant text: fix scan of positional parameters

The scanning to find the end of a positional parameter designator in
GVariant text format (e.g. '%i') is currently broken in case the 'end'
pointer is not specified.

The scan is controlled by a somewhat complicated loop that needs to deal
properly with cases like (123, %(ii)) [where '%(ii)' is to be taken
together, but the final ')' not].

This loop missed the case where a format string passed to
g_variant_new_parsed() ended immediately after such a conversion, with a
nul character.  In this case the 'end' pointer is NULL, so the only way
we can find the end is by scanning for nul in the string.

In case of g_variant_new_parsed() [which is what this code was designed
to be used for], the bug is somewhat unlikely in practice: the only way
that a valid text-form GVariant could ever contain a positional
parameter replacement at the end of the string is if this positional
parameter were the only thing being returned.  In that case, the user
would likely have opted for a more direct approach.

Unfortunately, this code is also active in the tokenisation phase of
g_variant_parse(), before positional parameters are rejected as invalid
for that case.  Anyone who calls this function with a nul-terminated
string (and no end pointer) is vulnerable to a crash from malicious user
input.  This can be seen, at the very least with many commandline tools:

  $ dconf write /x '%i'
  Segmentation fault

We fix this problem by searching for the nul character in this case, in
addition to comparing the end pointer.

This problem is almost certainly limited to being able to cause crashes.
The loop in question only performs reads and, in the security-sensitive
case, the token will be quickly rejected after the loop is finished
(since it starts with '%' and the 'app' pointer is unset).  This is
further mitigated by the fact that there are no known cases of GVariant
text format being used as part of a protocol at a privilege barrier.
---
 glib/gvariant-parser.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/glib/gvariant-parser.c b/glib/gvariant-parser.c
index e7dab85..9f4bcc5 100644
--- a/glib/gvariant-parser.c
+++ b/glib/gvariant-parser.c
@@ -237,7 +237,7 @@ token_stream_prepare (TokenStream *stream)
        * Also: ] and > are never in format strings.
        */
       for (end = stream->stream + 1;
-           end != stream->end && *end != ',' &&
+           end != stream->end && *end != '\0' && *end != ',' &&
            *end != ':' && *end != '>' && *end != ']' && !g_ascii_isspace (*end);
            end++)
 
-- 
1.8.3.1