From 67c87cd508385158a8a0fb12a430dd19d2883974 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Wed, 20 May 2015 08:39:08 +0200
Subject: [PATCH 1/6] CVE-2015-1779: incrementally decode websocket frames
Message-id: <1432111149-11644-2-git-send-email-kraxel@redhat.com>
Patchwork-id: 65099
O-Subject: [RHEL-7.2 qemu-kvm PATCH 1/2] CVE-2015-1779: incrementally decode websocket frames
Bugzilla: 1206497
RH-Acked-by: Thomas Huth <thuth@redhat.com>
RH-Acked-by: Petr Matousek <pmatouse@redhat.com>
RH-Acked-by: Daniel P. Berrange <berrange@redhat.com>
From: "Daniel P. Berrange" <berrange@redhat.com>
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@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@redhat.com>
(cherry picked from commit a2bebfd6e09d285aa793cae3fb0fc3a39a9fee6e)
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
---
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 df89315..a7d457c 100644
--- a/ui/vnc-ws.c
+++ b/ui/vnc-ws.c
@@ -114,7 +114,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);
@@ -124,18 +124,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;
@@ -273,15 +294,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 */
@@ -291,7 +311,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 */
@@ -308,40 +328,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 6e99213..0efc5c6 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -290,6 +290,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;
--
1.8.3.1