Re: [Qemu-devel] [PATCH for-4.0? 0/3] usb-mtp: fix ObjectInfo request handling

[Daniel P . Berrangé]

On Mon, Apr 15, 2019 at 06:18:01PM +0100, Peter Maydell wrote:
> On Mon, 15 Apr 2019 at 18:10, Eric Blake <address@hidden> wrote:
> > On 4/15/19 10:45 AM, Daniel P. Berrangé wrote:
> > > The 2nd patch in this series is a security flaw fix since the
> > > code was not correctly validating guest provided data length.
> >
> > Given that this is a security flaw, I've added this series to
> > https://wiki.qemu.org/Planning/4.0 in case you're hoping to get it in -rc4.
> 
> What are the consequences of the flaw ? IIRC it's only one
> extra byte read?

No, it is arbitrary extra bytes read.  We have a USB packet N bytes in
length, where N is supposed to match

  sizeof(ObjectInfo) + (ObjectInfo.length * 2)

but we checked it against

  sizeof(ObjectInfo) + ObjectInfo.length

As a result a malicious value for ObjectInfo.length can make QEMU can
read  ObjectInfo.length too many bytes when converting the string from
UTF16 to UTF8.

IOW, the returned UTF-8 string will end with whatever data is next on
the heap. This in turn creates a filename on disk with this random
data. I don't think this is a serious problem though, because if you
have enabled write support you've already given the guest ermission
to create arbitrary named files, so I don't see what they gain by
trying to exploit this. We already outlaw "/" which is the main danger
point in guest provided filenames.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|