[libvirt] [PATCH] qemu: avoid denial of service reading from QEMU monitor (CVE-2018-xxxx)

[Daniel P. Berrange]

On Wed, Jan 17, 2018 at 05:13:06PM +0100, Michal Privoznik wrote:
> On 01/16/2018 06:01 PM, Daniel P. Berrange wrote:
> > We read from QEMU until seeing a \r\n pair to indicate a completed reply
> > or event. To avoid memory denial-of-service though, we must have a size
> > limit on amount of data we buffer. 10 MB is large enough that it ought
> > to cope with normal QEMU replies, and small enough that we're not
> > consuming unreasonable mem.
> > 
> > Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
> > ---
> >  src/qemu/qemu_monitor.c | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> > 
> > diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> > index 046caf001c..85c7d68a13 100644
> > --- a/src/qemu/qemu_monitor.c
> > +++ b/src/qemu/qemu_monitor.c
> > @@ -55,6 +55,15 @@ VIR_LOG_INIT("qemu.qemu_monitor");
> >  #define DEBUG_IO 0
> >  #define DEBUG_RAW_IO 0
> >  
> > +/* We read from QEMU until seeing a \r\n pair to indicate a
> > + * completed reply or event. To avoid memory denial-of-service
> > + * though, we must have a size limit on amount of data we
> > + * buffer. 10 MB is large enough that it ought to cope with
> > + * normal QEMU replies, and small enough that we're not
> > + * consuming unreasonable mem.
> > + */
> > +#define QEMU_MONITOR_MAX_RESPONSE (10 * 1024 * 1024)
> > +
> >  struct _qemuMonitor {
> >      virObjectLockable parent;
> >  
> > @@ -575,6 +584,12 @@ qemuMonitorIORead(qemuMonitorPtr mon)
> >      int ret = 0;
> >  
> >      if (avail < 1024) {
> > +        if (mon->bufferLength >= QEMU_MONITOR_MAX_RESPONSE) {
> > +            virReportSystemError(ERANGE,
> > +                                 _("No complete monitor response found in %d bytes"),
> > +                                 QEMU_MONITOR_MAX_RESPONSE);
> > +            return -1;
> > +        }
> >          if (VIR_REALLOC_N(mon->buffer,
> >                            mon->bufferLength + 1024) < 0)
> >              return -1;
> > 
> 
> ACK, although is this really a CVE? Doesn't look that harmful to me. I
> mean, owning qemu is not that easy, is it?

There are a never ending stream of CVEs in QEMU guest device models.
These are mitigated by SELinux preventing a compromised QEMU from attacking
resources on the host it isn't granted access to. So attention would naturally
focus on attacking things it already has access to like the libvirt monitor
connection. Memory denial of service in libvirt is not too serious, but
still a CVE bug. Worse would be if libvirtd has buffer overflow/crash
in parsing a JSON response...

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 :|