crobinso pushed to qemu (f21). "CVE-2015-1779 vnc: insufficient resource limiting in VNC websockets decoder (bz #1205051, bz #1199572)"

notifications at fedoraproject.org notifications at fedoraproject.org
Thu Apr 2 18:12:13 UTC 2015


>From fc39a77d600968f5ad7dce2bf66e160caa90854a Mon Sep 17 00:00:00 2001
From: Cole Robinson <crobinso at redhat.com>
Date: Thu, 2 Apr 2015 14:11:41 -0400
Subject: CVE-2015-1779 vnc: insufficient resource limiting in VNC websockets
 decoder (bz #1205051, bz #1199572)


diff --git a/0015-CVE-2015-1779-incrementally-decode-websocket-frames.patch b/0015-CVE-2015-1779-incrementally-decode-websocket-frames.patch
new file mode 100644
index 0000000..05a3483
--- /dev/null
+++ b/0015-CVE-2015-1779-incrementally-decode-websocket-frames.patch
@@ -0,0 +1,238 @@
+From: "Daniel P. Berrange" <berrange at redhat.com>
+Date: Mon, 23 Mar 2015 22:58:21 +0000
+Subject: [PATCH] CVE-2015-1779: incrementally decode websocket frames
+
+The logic for decoding websocket frames wants to fully
+decode the frame header and payload, before allowing the
+VNC server to see any of the payload data. There is no
+size limit on websocket payloads, so this allows a
+malicious network client to consume 2^64 bytes in memory
+in QEMU. It can trigger this denial of service before
+the VNC server even performs any authentication.
+
+The fix is to decode the header, and then incrementally
+decode the payload data as it is needed. With this fix
+the websocket decoder will allow at most 4k of data to
+be buffered before decoding and processing payload.
+
+Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
+
+[ kraxel: fix frequent spurious disconnects, suggested by Peter Maydell ]
+
+  @@ -361,7 +361,7 @@ int vncws_decode_frame_payload(Buffer *input,
+  -        *payload_size = input->offset;
+  +        *payload_size = *payload_remain;
+
+[ kraxel: fix 32bit build ]
+
+  @@ -306,7 +306,7 @@ struct VncState
+  -    uint64_t ws_payload_remain;
+  +    size_t ws_payload_remain;
+
+Signed-off-by: Gerd Hoffmann <kraxel at redhat.com>
+(cherry picked from commit a2bebfd6e09d285aa793cae3fb0fc3a39a9fee6e)
+---
+ ui/vnc-ws.c | 105 ++++++++++++++++++++++++++++++++++++++++--------------------
+ ui/vnc-ws.h |   9 ++++--
+ ui/vnc.h    |   2 ++
+ 3 files changed, 80 insertions(+), 36 deletions(-)
+
+diff --git a/ui/vnc-ws.c b/ui/vnc-ws.c
+index e304baf..e5c1911 100644
+--- a/ui/vnc-ws.c
++++ b/ui/vnc-ws.c
+@@ -115,7 +115,7 @@ long vnc_client_read_ws(VncState *vs)
+ {
+     int ret, err;
+     uint8_t *payload;
+-    size_t payload_size, frame_size;
++    size_t payload_size, header_size;
+     VNC_DEBUG("Read websocket %p size %zd offset %zd\n", vs->ws_input.buffer,
+             vs->ws_input.capacity, vs->ws_input.offset);
+     buffer_reserve(&vs->ws_input, 4096);
+@@ -125,18 +125,39 @@ long vnc_client_read_ws(VncState *vs)
+     }
+     vs->ws_input.offset += ret;
+ 
+-    /* make sure that nothing is left in the ws_input buffer */
++    ret = 0;
++    /* consume as much of ws_input buffer as possible */
+     do {
+-        err = vncws_decode_frame(&vs->ws_input, &payload,
+-                              &payload_size, &frame_size);
+-        if (err <= 0) {
+-            return err;
++        if (vs->ws_payload_remain == 0) {
++            err = vncws_decode_frame_header(&vs->ws_input,
++                                            &header_size,
++                                            &vs->ws_payload_remain,
++                                            &vs->ws_payload_mask);
++            if (err <= 0) {
++                return err;
++            }
++
++            buffer_advance(&vs->ws_input, header_size);
+         }
++        if (vs->ws_payload_remain != 0) {
++            err = vncws_decode_frame_payload(&vs->ws_input,
++                                             &vs->ws_payload_remain,
++                                             &vs->ws_payload_mask,
++                                             &payload,
++                                             &payload_size);
++            if (err < 0) {
++                return err;
++            }
++            if (err == 0) {
++                return ret;
++            }
++            ret += err;
+ 
+-        buffer_reserve(&vs->input, payload_size);
+-        buffer_append(&vs->input, payload, payload_size);
++            buffer_reserve(&vs->input, payload_size);
++            buffer_append(&vs->input, payload, payload_size);
+ 
+-        buffer_advance(&vs->ws_input, frame_size);
++            buffer_advance(&vs->ws_input, payload_size);
++        }
+     } while (vs->ws_input.offset > 0);
+ 
+     return ret;
+@@ -274,15 +295,14 @@ void vncws_encode_frame(Buffer *output, const void *payload,
+     buffer_append(output, payload, payload_size);
+ }
+ 
+-int vncws_decode_frame(Buffer *input, uint8_t **payload,
+-                           size_t *payload_size, size_t *frame_size)
++int vncws_decode_frame_header(Buffer *input,
++                              size_t *header_size,
++                              size_t *payload_remain,
++                              WsMask *payload_mask)
+ {
+     unsigned char opcode = 0, fin = 0, has_mask = 0;
+-    size_t header_size = 0;
+-    uint32_t *payload32;
++    size_t payload_len;
+     WsHeader *header = (WsHeader *)input->buffer;
+-    WsMask mask;
+-    int i;
+ 
+     if (input->offset < WS_HEAD_MIN_LEN + 4) {
+         /* header not complete */
+@@ -292,7 +312,7 @@ int vncws_decode_frame(Buffer *input, uint8_t **payload,
+     fin = (header->b0 & 0x80) >> 7;
+     opcode = header->b0 & 0x0f;
+     has_mask = (header->b1 & 0x80) >> 7;
+-    *payload_size = header->b1 & 0x7f;
++    payload_len = header->b1 & 0x7f;
+ 
+     if (opcode == WS_OPCODE_CLOSE) {
+         /* disconnect */
+@@ -309,40 +329,57 @@ int vncws_decode_frame(Buffer *input, uint8_t **payload,
+         return -2;
+     }
+ 
+-    if (*payload_size < 126) {
+-        header_size = 6;
+-        mask = header->u.m;
+-    } else if (*payload_size == 126 && input->offset >= 8) {
+-        *payload_size = be16_to_cpu(header->u.s16.l16);
+-        header_size = 8;
+-        mask = header->u.s16.m16;
+-    } else if (*payload_size == 127 && input->offset >= 14) {
+-        *payload_size = be64_to_cpu(header->u.s64.l64);
+-        header_size = 14;
+-        mask = header->u.s64.m64;
++    if (payload_len < 126) {
++        *payload_remain = payload_len;
++        *header_size = 6;
++        *payload_mask = header->u.m;
++    } else if (payload_len == 126 && input->offset >= 8) {
++        *payload_remain = be16_to_cpu(header->u.s16.l16);
++        *header_size = 8;
++        *payload_mask = header->u.s16.m16;
++    } else if (payload_len == 127 && input->offset >= 14) {
++        *payload_remain = be64_to_cpu(header->u.s64.l64);
++        *header_size = 14;
++        *payload_mask = header->u.s64.m64;
+     } else {
+         /* header not complete */
+         return 0;
+     }
+ 
+-    *frame_size = header_size + *payload_size;
++    return 1;
++}
++
++int vncws_decode_frame_payload(Buffer *input,
++                               size_t *payload_remain, WsMask *payload_mask,
++                               uint8_t **payload, size_t *payload_size)
++{
++    size_t i;
++    uint32_t *payload32;
+ 
+-    if (input->offset < *frame_size) {
+-        /* frame not complete */
++    *payload = input->buffer;
++    /* If we aren't at the end of the payload, then drop
++     * off the last bytes, so we're always multiple of 4
++     * for purpose of unmasking, except at end of payload
++     */
++    if (input->offset < *payload_remain) {
++        *payload_size = input->offset - (input->offset % 4);
++    } else {
++        *payload_size = *payload_remain;
++    }
++    if (*payload_size == 0) {
+         return 0;
+     }
+-
+-    *payload = input->buffer + header_size;
++    *payload_remain -= *payload_size;
+ 
+     /* unmask frame */
+     /* process 1 frame (32 bit op) */
+     payload32 = (uint32_t *)(*payload);
+     for (i = 0; i < *payload_size / 4; i++) {
+-        payload32[i] ^= mask.u;
++        payload32[i] ^= payload_mask->u;
+     }
+     /* process the remaining bytes (if any) */
+     for (i *= 4; i < *payload_size; i++) {
+-        (*payload)[i] ^= mask.c[i % 4];
++        (*payload)[i] ^= payload_mask->c[i % 4];
+     }
+ 
+     return 1;
+diff --git a/ui/vnc-ws.h b/ui/vnc-ws.h
+index 95c1b0a..6e93fa0 100644
+--- a/ui/vnc-ws.h
++++ b/ui/vnc-ws.h
+@@ -83,7 +83,12 @@ long vnc_client_read_ws(VncState *vs);
+ void vncws_process_handshake(VncState *vs, uint8_t *line, size_t size);
+ void vncws_encode_frame(Buffer *output, const void *payload,
+             const size_t payload_size);
+-int vncws_decode_frame(Buffer *input, uint8_t **payload,
+-                               size_t *payload_size, size_t *frame_size);
++int vncws_decode_frame_header(Buffer *input,
++                              size_t *header_size,
++                              size_t *payload_remain,
++                              WsMask *payload_mask);
++int vncws_decode_frame_payload(Buffer *input,
++                               size_t *payload_remain, WsMask *payload_mask,
++                               uint8_t **payload, size_t *payload_size);
+ 
+ #endif /* __QEMU_UI_VNC_WS_H */
+diff --git a/ui/vnc.h b/ui/vnc.h
+index 334de9d..8630c6c 100644
+--- a/ui/vnc.h
++++ b/ui/vnc.h
+@@ -302,6 +302,8 @@ struct VncState
+ #ifdef CONFIG_VNC_WS
+     Buffer ws_input;
+     Buffer ws_output;
++    size_t ws_payload_remain;
++    WsMask ws_payload_mask;
+ #endif
+     /* current output mode information */
+     VncWritePixels *write_pixels;
diff --git a/0016-CVE-2015-1779-limit-size-of-HTTP-headers-from-websoc.patch b/0016-CVE-2015-1779-limit-size-of-HTTP-headers-from-websoc.patch
new file mode 100644
index 0000000..07a3cc3
--- /dev/null
+++ b/0016-CVE-2015-1779-limit-size-of-HTTP-headers-from-websoc.patch
@@ -0,0 +1,55 @@
+From: "Daniel P. Berrange" <berrange at redhat.com>
+Date: Mon, 23 Mar 2015 22:58:22 +0000
+Subject: [PATCH] CVE-2015-1779: limit size of HTTP headers from websockets
+ clients
+
+The VNC server websockets decoder will read and buffer data from
+websockets clients until it sees the end of the HTTP headers,
+as indicated by \r\n\r\n. In theory this allows a malicious to
+trick QEMU into consuming an arbitrary amount of RAM. In practice,
+because QEMU runs g_strstr_len() across the buffered header data,
+it will spend increasingly long burning CPU time searching for
+the substring match and less & less time reading data. So while
+this does cause arbitrary memory growth, the bigger problem is
+that QEMU will be burning 100% of available CPU time.
+
+A novnc websockets client typically sends headers of around
+512 bytes in length. As such it is reasonable to place a 4096
+byte limit on the amount of data buffered while searching for
+the end of HTTP headers.
+
+Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
+Signed-off-by: Gerd Hoffmann <kraxel at redhat.com>
+(cherry picked from commit 2cdb5e142fb93e875fa53c52864ef5eb8d5d8b41)
+---
+ ui/vnc-ws.c | 10 ++++++++--
+ 1 file changed, 8 insertions(+), 2 deletions(-)
+
+diff --git a/ui/vnc-ws.c b/ui/vnc-ws.c
+index e5c1911..1ea8554 100644
+--- a/ui/vnc-ws.c
++++ b/ui/vnc-ws.c
+@@ -89,8 +89,11 @@ void vncws_handshake_read(void *opaque)
+     VncState *vs = opaque;
+     uint8_t *handshake_end;
+     long ret;
+-    buffer_reserve(&vs->ws_input, 4096);
+-    ret = vnc_client_read_buf(vs, buffer_end(&vs->ws_input), 4096);
++    /* Typical HTTP headers from novnc are 512 bytes, so limiting
++     * total header size to 4096 is easily enough. */
++    size_t want = 4096 - vs->ws_input.offset;
++    buffer_reserve(&vs->ws_input, want);
++    ret = vnc_client_read_buf(vs, buffer_end(&vs->ws_input), want);
+ 
+     if (!ret) {
+         if (vs->csock == -1) {
+@@ -107,6 +110,9 @@ void vncws_handshake_read(void *opaque)
+         vncws_process_handshake(vs, vs->ws_input.buffer, vs->ws_input.offset);
+         buffer_advance(&vs->ws_input, handshake_end - vs->ws_input.buffer +
+                 strlen(WS_HANDSHAKE_END));
++    } else if (vs->ws_input.offset >= 4096) {
++        VNC_DEBUG("End of headers not found in first 4096 bytes\n");
++        vnc_client_error(vs);
+     }
+ }
+ 
diff --git a/qemu.spec b/qemu.spec
index 3d1de4c..c1f774c 100644
--- a/qemu.spec
+++ b/qemu.spec
@@ -152,7 +152,7 @@
 Summary: QEMU is a FAST! processor emulator
 Name: qemu
 Version: 2.1.3
-Release: 4%{?dist}
+Release: 5%{?dist}
 Epoch: 2
 License: GPLv2+ and LGPLv2+ and BSD
 Group: Development/Tools
@@ -215,6 +215,10 @@ Patch0012: 0012-cirrus-fix-blit-region-check.patch
 Patch0013: 0013-cirrus-don-t-overflow-CirrusVGAState-cirrus_bltbuf.patch
 # Fix .vdi disk corruption (bz #1199400)
 Patch0014: 0014-block-vdi-Add-locking-for-parallel-requests.patch
+# CVE-2015-1779 vnc: insufficient resource limiting in VNC websockets
+# decoder (bz #1205051, bz #1199572)
+Patch0015: 0015-CVE-2015-1779-incrementally-decode-websocket-frames.patch
+Patch0016: 0016-CVE-2015-1779-limit-size-of-HTTP-headers-from-websoc.patch
 
 BuildRequires: SDL2-devel
 BuildRequires: zlib-devel
@@ -764,6 +768,10 @@ CAC emulation development files.
 %patch0013 -p1
 # Fix .vdi disk corruption (bz #1199400)
 %patch0014 -p1
+# CVE-2015-1779 vnc: insufficient resource limiting in VNC websockets
+# decoder (bz #1205051, bz #1199572)
+%patch0015 -p1
+%patch0016 -p1
 
 
 %build
@@ -1544,6 +1552,10 @@ getent passwd qemu >/dev/null || \
 %endif
 
 %changelog
+* Thu Apr 02 2015 Cole Robinson <crobinso at redhat.com> - 2:2.1.3-5
+- CVE-2015-1779 vnc: insufficient resource limiting in VNC websockets decoder
+  (bz #1205051, bz #1199572)
+
 * Fri Mar 27 2015 Cole Robinson <crobinso at redhat.com> - 2:2.1.3-4
 - Qemu: PRDT overflow from guest to host (bz #1204919, bz #1205322)
 - CVE-2014-8106: cirrus: insufficient blit region checks (bz #1170612, bz
-- 
cgit v0.10.2


	http://pkgs.fedoraproject.org/cgit/qemu.git/commit/?h=f21&id=fc39a77d600968f5ad7dce2bf66e160caa90854a


More information about the scm-commits mailing list