[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PULL 56/58] json: Improve safety of qobject_from_jsonf_nof
From: |
Markus Armbruster |
Subject: |
[Qemu-devel] [PULL 56/58] json: Improve safety of qobject_from_jsonf_nofail() & friends |
Date: |
Fri, 24 Aug 2018 21:32:04 +0200 |
The JSON parser optionally supports interpolation. This is used to
build QObjects by parsing string templates. The templates are C
literals, so parse errors (such as invalid interpolation
specifications) are actually programming errors. Consequently, the
functions providing parsing with interpolation
(qobject_from_jsonf_nofail(), qobject_from_vjsonf_nofail(),
qdict_from_jsonf_nofail(), qdict_from_vjsonf_nofail()) pass
&error_abort to the parser.
However, there's another, more dangerous kind of programming error:
since we use va_arg() to get the value to interpolate, behavior is
undefined when the variable argument isn't consistent with the
interpolation specification.
The same problem exists with printf()-like functions, and the solution
is to have the compiler check consistency. This is what
GCC_FMT_ATTR() is about.
To enable this type checking for interpolation as well, we carefully
chose our interpolation specifications to match printf conversion
specifications, and decorate functions parsing templates with
GCC_FMT_ATTR().
Note that this only protects against undefined behavior due to type
errors. It can't protect against use of invalid interpolation
specifications that happen to be valid printf conversion
specifications.
However, there's still a gaping hole in the type checking: GCC
recognizes '%' as start of printf conversion specification anywhere in
the template, but the parser recognizes it only outside JSON strings.
For instance, if someone were to pass a "{ '%s': %d }" template, GCC
would require a char * and an int argument, but the parser would
va_arg() only an int argument, resulting in undefined behavior.
Avoid undefined behavior by catching the programming error at run
time: have the parser recognize and reject '%' in JSON strings.
Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
---
qobject/json-parser.c | 12 ++++++++++--
tests/check-qjson.c | 17 +++++++----------
2 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/qobject/json-parser.c b/qobject/json-parser.c
index 273f448a52..63e9229f1c 100644
--- a/qobject/json-parser.c
+++ b/qobject/json-parser.c
@@ -144,7 +144,8 @@ static QString *parse_string(JSONParserContext *ctxt,
JSONToken *token)
while (*ptr != quote) {
assert(*ptr);
- if (*ptr == '\\') {
+ switch (*ptr) {
+ case '\\':
beg = ptr++;
switch (*ptr++) {
case '"':
@@ -205,7 +206,14 @@ static QString *parse_string(JSONParserContext *ctxt,
JSONToken *token)
parse_error(ctxt, token, "invalid escape sequence in string");
goto out;
}
- } else {
+ break;
+ case '%':
+ if (ctxt->ap) {
+ parse_error(ctxt, token, "can't interpolate into string");
+ goto out;
+ }
+ /* fall through */
+ default:
cp = mod_utf8_codepoint(ptr, 6, &end);
if (cp < 0) {
parse_error(ctxt, token, "invalid UTF-8 sequence in string");
diff --git a/tests/check-qjson.c b/tests/check-qjson.c
index 936258ddd4..a1854573de 100644
--- a/tests/check-qjson.c
+++ b/tests/check-qjson.c
@@ -1037,16 +1037,13 @@ static void interpolation_unknown(void)
static void interpolation_string(void)
{
- QLitObject decoded = QLIT_QLIST(((QLitObject[]){
- QLIT_QSTR("%s"),
- QLIT_QSTR("eins"),
- {}}));
- QObject *qobj;
-
- /* Dangerous misfeature: % is silently ignored in strings */
- qobj = qobject_from_jsonf_nofail("['%s', %s]", "eins", "zwei");
- g_assert(qlit_equal_qobject(&decoded, qobj));
- qobject_unref(qobj);
+ if (g_test_subprocess()) {
+ qobject_from_jsonf_nofail("['%s', %s]", "eins", "zwei");
+ }
+ g_test_trap_subprocess(NULL, 0, 0);
+ g_test_trap_assert_failed();
+ g_test_trap_assert_stderr("*Unexpected error*"
+ "can't interpolate into string*");
}
static void simple_dict(void)
--
2.17.1
- [Qemu-devel] [PULL 32/58] json-parser: simplify and avoid JSONParserContext allocation, (continued)
- [Qemu-devel] [PULL 32/58] json-parser: simplify and avoid JSONParserContext allocation, Markus Armbruster, 2018/08/24
- [Qemu-devel] [PULL 58/58] json: Update references to RFC 7159 to RFC 8259, Markus Armbruster, 2018/08/24
- [Qemu-devel] [PULL 42/58] json: Improve names of lexer states related to numbers, Markus Armbruster, 2018/08/24
- [Qemu-devel] [PULL 57/58] json: Support %% in JSON strings when interpolating, Markus Armbruster, 2018/08/24
- [Qemu-devel] [PULL 45/58] json: Fix streamer not to ignore trailing unterminated structures, Markus Armbruster, 2018/08/24
- [Qemu-devel] [PULL 55/58] json: Keep interpolation state in JSONParserContext, Markus Armbruster, 2018/08/24
- [Qemu-devel] [PULL 44/58] json: Fix latent parser aborts at end of input, Markus Armbruster, 2018/08/24
- [Qemu-devel] [PULL 51/58] json: Make JSONToken opaque outside json-parser.c, Markus Armbruster, 2018/08/24
- [Qemu-devel] [PULL 41/58] json: Replace %I64d, %I64u by %PRId64, %PRIu64, Markus Armbruster, 2018/08/24
- [Qemu-devel] [PULL 38/58] json: Treat unwanted interpolation as lexical error, Markus Armbruster, 2018/08/24
- [Qemu-devel] [PULL 56/58] json: Improve safety of qobject_from_jsonf_nofail() & friends,
Markus Armbruster <=
- [Qemu-devel] [PULL 53/58] json: Clean up headers, Markus Armbruster, 2018/08/24
- [Qemu-devel] [PULL 46/58] json: Assert json_parser_parse() consumes all tokens on success, Markus Armbruster, 2018/08/24
- [Qemu-devel] [PULL 47/58] qjson: Have qobject_from_json() & friends reject empty and blank, Markus Armbruster, 2018/08/24
- [Qemu-devel] [PULL 50/58] json: Unbox tokens queue in JSONMessageParser, Markus Armbruster, 2018/08/24
- [Qemu-devel] [PULL 48/58] json: Enforce token count and size limits more tightly, Markus Armbruster, 2018/08/24
- [Qemu-devel] [PULL 49/58] json: Streamline json_message_process_token(), Markus Armbruster, 2018/08/24
- [Qemu-devel] [PULL 54/58] tests/drive_del-test: Fix harmless JSON interpolation bug, Markus Armbruster, 2018/08/24
- [Qemu-devel] [PULL 43/58] qjson: Fix qobject_from_json() & friends for multiple values, Markus Armbruster, 2018/08/24
- [Qemu-devel] [PULL 52/58] qobject: Drop superfluous includes of qemu-common.h, Markus Armbruster, 2018/08/24
- Re: [Qemu-devel] [PULL 00/58] QObject patches for 2018-08-24, Peter Maydell, 2018/08/25