[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] json: Fix % handling when not interpolating
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH] json: Fix % handling when not interpolating |
Date: |
Thu, 24 Jan 2019 10:35:52 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) |
Markus Armbruster <address@hidden> writes:
> Eric Blake <address@hidden> writes:
>
>> On 1/2/19 12:01 PM, Christophe Fergeau wrote:
>>> Adding Markus to cc: list, I forgot to do it when sending the patch.
>>
>> Also worth backporting via qemu-stable, now in cc.
>>
>>>
>>> Christophe
>>>
>>> On Wed, Jan 02, 2019 at 03:05:35PM +0100, Christophe Fergeau wrote:
>>>> commit 8bca4613 added support for %% in json strings when interpolating,
>>>> but in doing so, this broke handling of % when not interpolating as the
>>>> '%' is skipped in both cases.
>>>> This commit ensures we only try to handle %% when interpolating.
>
> Impact?
>
> If you're unable to assess, could you give us at least a reproducer?
>
>>>> Signed-off-by: Christophe Fergeau <address@hidden>
>>>> ---
>>>> qobject/json-parser.c | 10 ++++++----
>>>> tests/check-qjson.c | 5 +++++
>>>> 2 files changed, 11 insertions(+), 4 deletions(-)
>>>>
>>
>> Reviewed-by: Eric Blake <address@hidden>
>
> Patch looks good to me, but I'd like us to improve the commit message.
Let me try:
json: Fix % handling when not interpolating
Commit 8bca4613 added support for %% in json strings when interpolating,
but in doing so broke handling of % when not interpolating.
When parse_string() is fed a string token containing '%', it skips the
'%' regardless of ctxt->ap, i.e. even it's not interpolating. If the
'%' is the string's last character, it fails an assertion. Else, it
"merely" swallows the '%'.
Fix parse_string() to handle '%' specially only when interpolating.
To gauge the bug's impact, let's review non-interpolating users of this
parser, i.e. code passing NULL context to json_message_parser_init():
* tests/check-qjson.c, tests/test-qobject-input-visitor.c,
tests/test-visitor-serialization.c
Plenty of tests, but we still failed to cover the buggy case.
* monitor.c: QMP input
* qga/main.c: QGA input
* qobject_from_json():
- qobject-input-visitor.c: JSON command line option arguments of
-display and -blockdev
Reproducer: -blockdev '{"%"}'
- block.c: JSON pseudo-filenames starting with "json:"
Reproducer: https://bugzilla.redhat.com/show_bug.cgi?id=1668244#c3
- block/rbd.c: JSON key pairs
Pseudo-filenames starting with "rbd:".
Command line, QMP and QGA input are trusted.
Filenames are trusted when they come from command line, QMP or HMP.
They are untrusted when they come from from image file headers.
Example: QCOW2 backing file name. Note that this is *not* the security
boundary between host and guest. It's the boundary between host and an
image file from an untrusted source.
Neither failing an assertion nor skipping a character in a filename of
your choice looks exploitable. Note that we don't support compiling
with NDEBUG.
Fixes: 8bca4613e6cddd948895b8db3def05950463495b
Cc: address@hidden
Comments?
- Re: [Qemu-devel] [PATCH] json: Fix % handling when not interpolating, (continued)
- Re: [Qemu-devel] [PATCH] json: Fix % handling when not interpolating, Max Reitz, 2019/01/09
- Re: [Qemu-devel] [PATCH] json: Fix % handling when not interpolating, Markus Armbruster, 2019/01/09
- Re: [Qemu-devel] [PATCH] json: Fix % handling when not interpolating, Max Reitz, 2019/01/09
- Re: [Qemu-devel] [PATCH] json: Fix % handling when not interpolating, Markus Armbruster, 2019/01/09
- Re: [Qemu-devel] [PATCH] json: Fix % handling when not interpolating, Max Reitz, 2019/01/09
- Re: [Qemu-devel] [PATCH] json: Fix % handling when not interpolating, Daniel P . Berrangé, 2019/01/09
- Re: [Qemu-devel] [PATCH] json: Fix % handling when not interpolating, Max Reitz, 2019/01/09
- Re: [Qemu-devel] [PATCH] json: Fix % handling when not interpolating, Daniel P . Berrangé, 2019/01/09
- Re: [Qemu-devel] [PATCH] json: Fix % handling when not interpolating, Markus Armbruster, 2019/01/10
- Re: [Qemu-devel] [PATCH] json: Fix % handling when not interpolating, Richard W.M. Jones, 2019/01/22
- Re: [Qemu-devel] [PATCH] json: Fix % handling when not interpolating,
Markus Armbruster <=
- Re: [Qemu-devel] [PATCH] json: Fix % handling when not interpolating, Christophe Fergeau, 2019/01/24
- Re: [Qemu-devel] [PATCH] json: Fix % handling when not interpolating, Eric Blake, 2019/01/24
- Re: [Qemu-devel] [PATCH] json: Fix % handling when not interpolating, Markus Armbruster, 2019/01/24
- Re: [Qemu-devel] [PATCH] json: Fix % handling when not interpolating, Eric Blake, 2019/01/24
Re: [Qemu-devel] [PATCH] json: Fix % handling when not interpolating, Richard W.M. Jones, 2019/01/22
Re: [Qemu-devel] [PATCH] json: Fix % handling when not interpolating, Markus Armbruster, 2019/01/24