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 <cferg...@redhat.com> writes: > >> On Mon, Jan 07, 2019 at 04:47:44PM +0100, Markus Armbruster wrote: >>> Eric Blake <ebl...@redhat.com> 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.
Max
signature.asc
Description: OpenPGP digital signature
