Skip to content

Commit 70d664a

Browse files
Nashit-haduh95
authored andcommitted
inspector: fix out-of-bounds read in ws frame decoding
Signed-off-by: nashit hayyat <nashit@bugqore.com> PR-URL: #64618 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Kohei Ueno <kohei.ueno119@gmail.com>
1 parent 2539d1c commit 70d664a

2 files changed

Lines changed: 29 additions & 1 deletion

File tree

‎src/inspector_socket.cc‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,10 @@ static ws_decode_result decode_frame_hybi17(const std::vector<char>& buffer,
360360
}
361361
size_t payload_length = static_cast<size_t>(payload_length64);
362362

363-
if (buffer.size() - kMaskingKeyWidthInBytes < payload_length)
363+
// The masking key and the payload follow the header `it` already walked
364+
// past, so they have to fit in what is left rather than in the whole buffer.
365+
size_t remaining = static_cast<size_t>(buffer.end() - it);
366+
if (remaining < kMaskingKeyWidthInBytes + payload_length)
364367
return FRAME_INCOMPLETE;
365368

366369
std::vector<char>::const_iterator masking_key = it;

‎test/cctest/test_inspector_socket.cc‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -409,6 +409,31 @@ TEST_F(InspectorSocketTest, ReadsAndWritesInspectorMessage) {
409409
reinterpret_cast<uv_handle_t*>(&client_socket)));
410410
}
411411

412+
TEST_F(InspectorSocketTest, WaitsForFrameBodyToArrive) {
413+
do_write(const_cast<char*>(HANDSHAKE_REQ), sizeof(HANDSHAKE_REQ) - 1);
414+
expect_handshake();
415+
416+
// A header announcing a masked five byte payload, with the masking key and
417+
// the payload still on the wire. The frame has to be treated as incomplete
418+
// until the remaining bytes show up.
419+
const char FRAME_HEADER[] = {'\x81', '\x85'};
420+
do_write(FRAME_HEADER, sizeof(FRAME_HEADER));
421+
422+
// Round trip through the server so the header is guaranteed to have been
423+
// read on its own, instead of arriving together with the body below.
424+
const char SERVER_MESSAGE[] = "ping";
425+
const char SERVER_FRAME[] = {'\x81', '\x04', 'p', 'i', 'n', 'g'};
426+
delegate->Write(SERVER_MESSAGE, sizeof(SERVER_MESSAGE) - 1);
427+
expect_on_client(SERVER_FRAME, sizeof(SERVER_FRAME));
428+
429+
const char FRAME_BODY[] = {
430+
'\x01', '\x02', '\x03', '\x04', '\x69', '\x67', '\x6F', '\x68', '\x6E'};
431+
do_write(FRAME_BODY, sizeof(FRAME_BODY));
432+
433+
const char CLIENT_MESSAGE[] = "hello";
434+
delegate->ExpectData(CLIENT_MESSAGE, sizeof(CLIENT_MESSAGE) - 1);
435+
}
436+
412437
TEST_F(InspectorSocketTest, BufferEdgeCases) {
413438
do_write(const_cast<char*>(HANDSHAKE_REQ), sizeof(HANDSHAKE_REQ) - 1);
414439
expect_handshake();

0 commit comments

Comments
 (0)