Skip to content

Commit beab4c2

Browse files
rlanethTheldus
authored andcommitted
fix: harden frame length arithmetic and bound allocations
Prevent an attacker-controlled 64-bit frame-length from wrapping unsigned arithmetic and causing heap overwrites in read_single_frame() by adding explicit overflow checks and rejecting oversized messages.
1 parent 31eb58a commit beab4c2

2 files changed

Lines changed: 55 additions & 13 deletions

File tree

include/ws.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,10 @@ extern "C" {
160160
* @brief Protocol error
161161
*/
162162
#define WS_CLSE_PROTERR 1002
163+
/**
164+
* @brief Message too large
165+
*/
166+
#define WS_CLSE_BIGMSG 1009
163167
/**@}*/
164168
/**
165169
* @brief Inconsistent message (invalid utf-8)

src/ws.c

Lines changed: 51 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1220,6 +1220,23 @@ static int skip_frame(struct ws_frame_data *wfd, uint64_t frame_size)
12201220
return (0);
12211221
}
12221222

1223+
/**
1224+
* @brief Adds two uint64_t numbers detecting overflow.
1225+
*
1226+
* @param a Left operand.
1227+
* @param b Right operand.
1228+
* @param out Result pointer.
1229+
*
1230+
* @return Returns true if there is no overflow, false otherwise.
1231+
*/
1232+
static bool checked_add_u64(uint64_t a, uint64_t b, uint64_t *out)
1233+
{
1234+
if (a > UINT64_MAX - b)
1235+
return (false);
1236+
*out = a + b;
1237+
return (true);
1238+
}
1239+
12231240
/**
12241241
* Frame state data
12251242
*
@@ -1410,13 +1427,15 @@ static int handle_close_frame(struct ws_frame_data *wfd,
14101427
static int read_single_frame(struct ws_frame_data *wfd,
14111428
struct frame_state_data *fsd)
14121429
{
1413-
uint64_t *frame_size; /* Curr frame size. */
1414-
unsigned char *tmp; /* Tmp message. */
1415-
unsigned char *msg; /* Current message. */
1416-
uint64_t *msg_idx; /* Message index. */
1417-
uint8_t *masks; /* Current mask. */
1418-
int cur_byte; /* Curr byte read. */
1419-
uint64_t i; /* Loop index. */
1430+
uint64_t *frame_size; /* Curr frame size. */
1431+
uint64_t next_size = 0; /* Checked next sz. */
1432+
uint64_t alloc_size = 0; /* Checked alloc sz.*/
1433+
unsigned char *tmp; /* Tmp message. */
1434+
unsigned char *msg; /* Current message. */
1435+
uint64_t *msg_idx; /* Message index. */
1436+
uint8_t *masks; /* Current mask. */
1437+
int cur_byte; /* Curr byte read. */
1438+
uint64_t i; /* Loop index. */
14201439

14211440
/* Decide which mask and msg to use. */
14221441
if (is_control_frame(fsd->opcode)) {
@@ -1450,8 +1469,6 @@ static int read_single_frame(struct ws_frame_data *wfd,
14501469
(((uint64_t)next_byte(wfd))); /* frame[9]. */
14511470
}
14521471

1453-
*frame_size += fsd->frame_length;
1454-
14551472
/*
14561473
* Check frame size
14571474
*
@@ -1460,17 +1477,25 @@ static int read_single_frame(struct ws_frame_data *wfd,
14601477
* bytes. Also keep in mind that this is still true
14611478
* for continuation frames.
14621479
*/
1463-
if (*frame_size > MAX_FRAME_LENGTH)
1480+
if (!checked_add_u64(*frame_size, fsd->frame_length, &next_size) ||
1481+
next_size > MAX_FRAME_LENGTH)
14641482
{
14651483
DEBUG("Current frame from client %d, exceeds the maximum\n"
14661484
"amount of bytes allowed (%" PRId64 "/%d)!",
1467-
wfd->client->client_sock, *frame_size + fsd->frame_length,
1485+
wfd->client->client_sock, next_size,
14681486
MAX_FRAME_LENGTH);
14691487

1488+
/*
1489+
* We are rejecting a too-large message (or a wrapped size).
1490+
* RFC 6455 suggests code 1009 for this situation.
1491+
*/
1492+
do_close(wfd, WS_CLSE_BIGMSG);
14701493
wfd->error = 1;
14711494
return (-1);
14721495
}
14731496

1497+
*frame_size = next_size;
1498+
14741499
/* Read masks. */
14751500
masks[0] = next_byte(wfd);
14761501
masks[1] = next_byte(wfd);
@@ -1502,12 +1527,25 @@ static int read_single_frame(struct ws_frame_data *wfd,
15021527
{
15031528
if (!is_control_frame(fsd->opcode))
15041529
{
1505-
tmp = realloc(msg, *msg_idx + fsd->frame_length + fsd->is_fin);
1530+
if (!checked_add_u64(*msg_idx, fsd->frame_length, &alloc_size) ||
1531+
!checked_add_u64(alloc_size, fsd->is_fin, &alloc_size) ||
1532+
alloc_size > MAX_FRAME_LENGTH + 1)
1533+
{
1534+
DEBUG("Cannot allocate frame data: invalid message size "
1535+
"(idx=%" PRId64 ", len=%" PRId64 ", fin=%u)\n",
1536+
*msg_idx, fsd->frame_length, fsd->is_fin);
1537+
do_close(wfd, WS_CLSE_BIGMSG);
1538+
wfd->error = 1;
1539+
return (-1);
1540+
}
1541+
1542+
tmp = realloc(msg, alloc_size);
15061543
if (!tmp)
15071544
{
15081545
DEBUG("Cannot allocate memory, requested: % " PRId64 "\n",
1509-
(*msg_idx + fsd->frame_length + fsd->is_fin));
1546+
alloc_size);
15101547

1548+
do_close(wfd, WS_CLSE_BIGMSG);
15111549
wfd->error = 1;
15121550
return (-1);
15131551
}

0 commit comments

Comments
 (0)