aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPeter Maydell <peter.maydell@linaro.org>2015-04-01 17:18:51 +0100
committerPeter Maydell <peter.maydell@linaro.org>2015-04-01 17:18:51 +0100
commitfde069f751a9aa7e597c9d297a9995eca418a403 (patch)
tree6c56b659ac57e398ef32f2a5f77dc9ba95a7d65f
parentb8a86c4ac4d04c106ba38fbd707041cba334a155 (diff)
parent2cdb5e142fb93e875fa53c52864ef5eb8d5d8b41 (diff)
Merge remote-tracking branch 'remotes/kraxel/tags/pull-cve-2015-1779-20150401-2' into staging
vnc: fix websocket security issues (cve-2015-1779). # gpg: Signature made Wed Apr 1 16:14:34 2015 BST using RSA key ID D3E87138 # gpg: Good signature from "Gerd Hoffmann (work) <kraxel@redhat.com>" # gpg: aka "Gerd Hoffmann <gerd@kraxel.org>" # gpg: aka "Gerd Hoffmann (private) <kraxel@gmail.com>" * remotes/kraxel/tags/pull-cve-2015-1779-20150401-2: CVE-2015-1779: limit size of HTTP headers from websockets clients CVE-2015-1779: incrementally decode websocket frames Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
-rw-r--r--ui/vnc-ws.c115
-rw-r--r--ui/vnc-ws.h9
-rw-r--r--ui/vnc.h2
3 files changed, 88 insertions, 38 deletions
diff --git a/ui/vnc-ws.c b/ui/vnc-ws.c
index 85dbb7e6ae..62eb97fe76 100644
--- a/ui/vnc-ws.c
+++ b/ui/vnc-ws.c
@@ -81,8 +81,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) {
@@ -99,6 +102,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);
}
}
@@ -107,7 +113,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);
@@ -117,18 +123,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;
@@ -265,15 +292,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 */
@@ -283,7 +309,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 */
@@ -300,40 +326,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 ef229b7c0c..14d4230eff 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 e19ac396f2..3f7c6a9bc6 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -306,6 +306,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;