summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorHanno Boeck <hanno@hboeck.de>2016-02-22 07:46:17 -0500
committerAllison Ryan Lortie <desrt@desrt.ca>2016-02-22 09:12:09 -0500
commit391c20fb04637aaba02785b3ec989344a21834e1 (patch)
tree55ed31da3998fc3a70ff88b8e8c0e45b56bb3896
parente7c8cb02ef6779e31a3907b325b47c76b6f4460d (diff)
downloadglib-391c20fb04637aaba02785b3ec989344a21834e1.tar.gz
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.
-rw-r--r--glib/gvariant-parser.c2
1 files changed, 1 insertions, 1 deletions
diff --git a/glib/gvariant-parser.c b/glib/gvariant-parser.c
index 3a8c63d3c..95674ff5f 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++)