From cc0b8bdf124c47090d0b794c9b6e2e3852c860d9 Mon Sep 17 00:00:00 2001 From: Hanno Boeck 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