Skip to content

Commit 7e1eb3c

Browse files
authored
[mod_xml_rpc] Fix OOB write and read-loop hang in WebSocket parser (#3114)
`ws_read_frame()` had two defects in the framing path: - After header parsing, the remaining payload count `need = plen - (datalen - header)` could go negative when the initial read buffered more bytes than the frame's declared length, even with an in-range `plen`. A negative `need` passed the signed size guard and reached `ws_raw_read()` as a `size_t` near its maximum, driving a `memcpy` past `wsh->buffer`. Reject `need < 0` with a protocol-error close before the read loop. - The loop filling the frame header called `ws_raw_read()` without checking its result, so a connection that stopped delivering header bytes left the loop with no terminating condition, spinning or hanging the handler thread. Close on a non-advancing read, matching the payload read loop.
1 parent a047b7a commit 7e1eb3c

1 file changed

Lines changed: 15 additions & 6 deletions

File tree

  • src/mod/xml_int/mod_xml_rpc

src/mod/xml_int/mod_xml_rpc/ws.c

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -430,14 +430,17 @@ issize_t ws_read_frame(wsh_t *wsh, ws_opcode_t *oc, uint8_t **data)
430430
}
431431

432432
if ((wsh->datalen = ws_raw_read(wsh, wsh->buffer, 14)) < need) {
433-
while (!wsh->down && (wsh->datalen += ws_raw_read(wsh, wsh->buffer + wsh->datalen, 14 - wsh->datalen)) < need) ;
433+
while (!wsh->down && wsh->datalen < need) {
434+
issize_t r = ws_raw_read(wsh, wsh->buffer + wsh->datalen, 14 - wsh->datalen);
434435

435-
#if 0
436-
if (0 && (wsh->datalen += ws_raw_read(wsh, wsh->buffer + wsh->datalen, 14 - wsh->datalen)) < need) {
437-
/* too small - protocol err */
438-
return ws_close(wsh, WS_PROTO_ERR);
436+
if (r < 1) {
437+
/* invalid read - protocol err .. */
438+
*oc = WSOC_CLOSE;
439+
return ws_close(wsh, WS_PROTO_ERR);
440+
}
441+
442+
wsh->datalen += r;
439443
}
440-
#endif
441444
}
442445

443446
*oc = *wsh->buffer & 0xf;
@@ -517,6 +520,12 @@ issize_t ws_read_frame(wsh_t *wsh, ws_opcode_t *oc, uint8_t **data)
517520

518521
need = (wsh->plen - (wsh->datalen - need));
519522

523+
if (need < 0) {
524+
/* more buffered than the frame declares - protocol err */
525+
*oc = WSOC_CLOSE;
526+
return ws_close(wsh, WS_PROTO_ERR);
527+
}
528+
520529
/* Reserve 1 byte for the trailing NUL below. */
521530
if ((need + wsh->datalen) >= (issize_t)wsh->buflen) {
522531
/* too big - Ain't nobody got time fo' dat */

0 commit comments

Comments
 (0)