[Qemu-devel] [PATCH v1 1/7] io: monitor encoutput buffer size from websocket GSource

Daniel P. Berrange posted 7 patches 8 years, 4 months ago
There is a newer version of this series
[Qemu-devel] [PATCH v1 1/7] io: monitor encoutput buffer size from websocket GSource
Posted by Daniel P. Berrange 8 years, 4 months ago
The websocket GSource is monitoring the size of the rawoutput
buffer to determine if the channel can accepts more writes.
The rawoutput buffer, however, is merely a temporary staging
buffer before data is copied into the encoutput buffer. This
its size will always be zero when the GSource runs.

This flaw causes the encoutput buffer to grow without bound
if the other end of the underlying data channel doesn't
read data being sent. This can be seen with VNC if a client
is on a slow WAN link and the guest OS is sending many screen
updates. A malicious VNC client can act like it is on a slow
link by playing a video in the guest and then reading data
very slowly, causing QEMU host memory to expand arbitrarily.

This issue is assigned CVE-2017-????, publically reported in

  https://bugs.launchpad.net/qemu/+bug/1718964

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 io/channel-websock.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/io/channel-websock.c b/io/channel-websock.c
index d1d471f86e..04bcc059cd 100644
--- a/io/channel-websock.c
+++ b/io/channel-websock.c
@@ -28,7 +28,7 @@
 #include <time.h>
 
 
-/* Max amount to allow in rawinput/rawoutput buffers */
+/* Max amount to allow in rawinput/encoutput buffers */
 #define QIO_CHANNEL_WEBSOCK_MAX_BUFFER 8192
 
 #define QIO_CHANNEL_WEBSOCK_CLIENT_KEY_LEN 24
@@ -1208,7 +1208,7 @@ qio_channel_websock_source_check(GSource *source)
     if (wsource->wioc->rawinput.offset || wsource->wioc->io_eof) {
         cond |= G_IO_IN;
     }
-    if (wsource->wioc->rawoutput.offset < QIO_CHANNEL_WEBSOCK_MAX_BUFFER) {
+    if (wsource->wioc->encoutput.offset < QIO_CHANNEL_WEBSOCK_MAX_BUFFER) {
         cond |= G_IO_OUT;
     }
 
-- 
2.13.5


Re: [Qemu-devel] [PATCH v1 1/7] io: monitor encoutput buffer size from websocket GSource
Posted by Eric Blake 8 years, 4 months ago
On 10/10/2017 10:43 AM, Daniel P. Berrange wrote:
> The websocket GSource is monitoring the size of the rawoutput
> buffer to determine if the channel can accepts more writes.
> The rawoutput buffer, however, is merely a temporary staging
> buffer before data is copied into the encoutput buffer. This

s/This/Thus/

> its size will always be zero when the GSource runs.
> 
> This flaw causes the encoutput buffer to grow without bound
> if the other end of the underlying data channel doesn't
> read data being sent. This can be seen with VNC if a client
> is on a slow WAN link and the guest OS is sending many screen
> updates. A malicious VNC client can act like it is on a slow
> link by playing a video in the guest and then reading data
> very slowly, causing QEMU host memory to expand arbitrarily.
> 
> This issue is assigned CVE-2017-????, publically reported in

If we get the assignment in time, I'm sure you'll update this before the
PULL request.

> 
>   https://bugs.launchpad.net/qemu/+bug/1718964
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  io/channel-websock.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

> 
> diff --git a/io/channel-websock.c b/io/channel-websock.c
> index d1d471f86e..04bcc059cd 100644
> --- a/io/channel-websock.c
> +++ b/io/channel-websock.c
> @@ -28,7 +28,7 @@
>  #include <time.h>
>  
>  
> -/* Max amount to allow in rawinput/rawoutput buffers */
> +/* Max amount to allow in rawinput/encoutput buffers */
>  #define QIO_CHANNEL_WEBSOCK_MAX_BUFFER 8192
>  
>  #define QIO_CHANNEL_WEBSOCK_CLIENT_KEY_LEN 24
> @@ -1208,7 +1208,7 @@ qio_channel_websock_source_check(GSource *source)
>      if (wsource->wioc->rawinput.offset || wsource->wioc->io_eof) {
>          cond |= G_IO_IN;
>      }
> -    if (wsource->wioc->rawoutput.offset < QIO_CHANNEL_WEBSOCK_MAX_BUFFER) {
> +    if (wsource->wioc->encoutput.offset < QIO_CHANNEL_WEBSOCK_MAX_BUFFER) {
>          cond |= G_IO_OUT;
>      }
>  
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH v1 1/7] io: monitor encoutput buffer size from websocket GSource
Posted by Daniel P. Berrange 8 years, 4 months ago
On Tue, Oct 10, 2017 at 11:51:00AM -0500, Eric Blake wrote:
> On 10/10/2017 10:43 AM, Daniel P. Berrange wrote:
> > The websocket GSource is monitoring the size of the rawoutput
> > buffer to determine if the channel can accepts more writes.
> > The rawoutput buffer, however, is merely a temporary staging
> > buffer before data is copied into the encoutput buffer. This
> 
> s/This/Thus/
> 
> > its size will always be zero when the GSource runs.
> > 
> > This flaw causes the encoutput buffer to grow without bound
> > if the other end of the underlying data channel doesn't
> > read data being sent. This can be seen with VNC if a client
> > is on a slow WAN link and the guest OS is sending many screen
> > updates. A malicious VNC client can act like it is on a slow
> > link by playing a video in the guest and then reading data
> > very slowly, causing QEMU host memory to expand arbitrarily.
> > 
> > This issue is assigned CVE-2017-????, publically reported in
> 
> If we get the assignment in time, I'm sure you'll update this before the
> PULL request.

Yes, exactly the plan...



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