Skip to content

Commit 8492b0f

Browse files
authored
Merge commit from fork
* tport/ws: Fix heap OOB write in WebSocket payload unmasking. The XOR unmask loop iterated `wsh->datalen` (bytes in `wsh->buffer`: header + payload tail) but indexed into `wsh->body`, which holds only the per-frame payload bytes in `wsh->bbuffer`. The loop wrote up to 14 XOR'd bytes past the live payload region of `bbuffer`. Bound the loop by `wsh->rplen` (payload bytes copied into body), which is the correct cross-buffer count. * tport/ws: Fix heap OOB writes from undersized WebSocket buffers `ws_read_frame()` and the handshake parser undersized the heap buffers two ways, each writing past the allocation. 1. Trailing NUL. `ws_init()` and the `bbuffer` realloc sized buffers at exactly `buflen` / `bbuflen`, but a terminating '\0' is written at `buffer[datalen]` / `body[rplen]`. A buffer filled to capacity put that NUL one byte past the chunk. Fix: reserve a NUL slot, allocating `buflen + 1` / `bbuflen + 1` at init and on realloc. 2. Body undersized on the fragmentation path. The `bbuffer` grow check sized on the bytes still to read, not the full frame payload. Payload bytes arriving with the header in the initial read are copied in separately, so the body holds the whole `plen` at offset `blen`; the buffer was under-allocated by those bytes, letting a fragmented message write past the allocation. Fix: size on `blen + plen` (prior fragments plus this frame's full payload).
1 parent a404c31 commit 8492b0f

2 files changed

Lines changed: 15 additions & 6 deletions

File tree

libsofia-sip-ua/tport/ws.c

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -740,8 +740,9 @@ int ws_init(wsh_t *wsh, ws_socket_t sock, SSL_CTX *ssl_ctx, int close_sock, int
740740
wsh->buflen = 1024 * 64;
741741
wsh->bbuflen = wsh->buflen;
742742

743-
wsh->buffer = malloc(wsh->buflen);
744-
wsh->bbuffer = malloc(wsh->bbuflen);
743+
/* +1 NUL slot — see wsh_t in ws.h. */
744+
wsh->buffer = malloc(wsh->buflen + 1);
745+
wsh->bbuffer = malloc(wsh->bbuflen + 1);
745746
//printf("init %p %ld\n", (void *) wsh->bbuffer, wsh->bbuflen);
746747
//memset(wsh->buffer, 0, wsh->buflen);
747748
//memset(wsh->bbuffer, 0, wsh->bbuflen);
@@ -1043,18 +1044,21 @@ ssize_t ws_read_frame(wsh_t *wsh, ws_opcode_t *oc, uint8_t **data)
10431044

10441045
blen = wsh->body - wsh->bbuffer;
10451046

1046-
if (need + blen > (ssize_t)wsh->bbuflen) {
1047+
/* Body must hold blen accumulated bytes plus this frame's
1048+
* full payload. */
1049+
if (blen + wsh->plen > (ssize_t)wsh->bbuflen) {
10471050
void *tmp;
10481051

1049-
wsh->bbuflen = need + blen + wsh->rplen;
1052+
wsh->bbuflen = blen + wsh->plen;
10501053

10511054
if (wsh->payload_size_max && wsh->bbuflen > wsh->payload_size_max) {
10521055
/* size limit */
10531056
*oc = WSOC_CLOSE;
10541057
return ws_close(wsh, WS_NONE);
10551058
}
10561059

1057-
if ((tmp = realloc(wsh->bbuffer, wsh->bbuflen))) {
1060+
/* +1 NUL slot — see wsh_t in ws.h. */
1061+
if ((tmp = realloc(wsh->bbuffer, wsh->bbuflen + 1))) {
10581062
wsh->bbuffer = tmp;
10591063
} else {
10601064
abort();
@@ -1086,7 +1090,9 @@ ssize_t ws_read_frame(wsh_t *wsh, ws_opcode_t *oc, uint8_t **data)
10861090
if (mask && maskp) {
10871091
ssize_t i;
10881092

1089-
for (i = 0; i < wsh->datalen; i++) {
1093+
/* Unmask payload only. wsh->datalen tracks bytes in wsh->buffer
1094+
* (header + frame), but wsh->body holds just the rplen payload bytes.*/
1095+
for (i = 0; i < wsh->rplen; i++) {
10901096
wsh->body[i] ^= maskp[i % 4];
10911097
}
10921098
}

libsofia-sip-ua/tport/ws.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,9 @@ typedef enum {
9292

9393
typedef struct wsh_s {
9494
ws_socket_t sock;
95+
/* buffer/bbuffer are allocated as buflen+1 / bbuflen+1; the trailing
96+
* byte is reserved as a NUL terminator slot. buflen/bbuflen track
97+
* usable content length, not allocation size. */
9598
char *buffer;
9699
char *bbuffer;
97100
char *body;

0 commit comments

Comments
 (0)