[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-stable] [Qemu-devel] [PATCH] json: Fix % handling when not int

From: Max Reitz
Subject: Re: [Qemu-stable] [Qemu-devel] [PATCH] json: Fix % handling when not interpolating
Date: Wed, 9 Jan 2019 14:06:22 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0

On 08.01.19 11:36, Markus Armbruster wrote:
> Copying block maintainers for help with assessing the bug's (non-)impact
> on security.
> Christophe Fergeau <address@hidden> writes:
>> On Mon, Jan 07, 2019 at 04:47:44PM +0100, Markus Armbruster wrote:
>>> 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?
>> This all came from 
>> https://lists.freedesktop.org/archives/spice-devel/2018-December/046644.html
>> Setting up a VM with libvirt with <graphics type='spice' autoport='yes' 
>> passwd='password%'/>
>> fails to start with:
>>   qemu-system-x86_64: qobject/json-parser.c:146: parse_string: Assertion 
>> `*ptr' failed.
>> If you use 'password%%' as the password instead, when trying to connect
>> to the VM, you type 'password%' as the password instead of 'password%%'
>> as configured in the domain XML.
> Thanks.
> As the commit message says, the bug bites when we parse a string
> containing '%s' with !ctxt->ap.  The parser then swallows a character.
> If it swallows the terminating '"', it fails the assertion.
> We parse with !ctxt->ap in the following cases:
> * Tests (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 :(
> * QMP input (monitor.c)
> * QGA input (qga/main.c)
> * qobject_from_json()
>   - JSON pseudo-filenames (block.c)
>     These are pseudo-filenames starting with "json:".
>   - JSON key pairs (block/rbd.c)
>     As far as I can tell, these can come only from pseudo-filenames
>     starting with "rbd:".
>   - JSON command line option arguments of -display and -blockdev
>     (qobject-input-visitor.c)
>     Reproducer: -blockdev '{"%"}'
> 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.
> I can't see how the bug could be exploited.  Neither failing an
> assertion nor skipping a character in a filename of your choice is
> interesting.  We don't support compiling with NDEBUG.
> Kevin, Max, do you agree?

I wouldn't call it "not interesting" if adding an image to your VM at
runtime can crash the whole thing.

(qemu-img create -f qcow2 -u -b 'json:{"%"}' foo.qcow2 64M)

Whether this is a security issue...  I don't know, but it is a DoS.


Attachment: signature.asc
Description: OpenPGP digital signature

reply via email to

[Prev in Thread] Current Thread [Next in Thread]