Skip to content

Commit

Permalink
fix vulnerablilities:NULL pointer dereference and Heap OOB read and S…
Browse files Browse the repository at this point in the history
…tack OOB read when copying a string for logs, and limit crypto stream frame data buffer and unack packets send buffer
  • Loading branch information
dreamwind1985 committed Dec 4, 2024
1 parent 6ba26c9 commit 39539fe
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 9 deletions.
7 changes: 5 additions & 2 deletions src/http3/xqc_h3_stream.c
Original file line number Diff line number Diff line change
Expand Up @@ -1687,8 +1687,10 @@ xqc_h3_stream_process_data(xqc_stream_t *stream, xqc_h3_stream_t *h3s, xqc_bool_

} else if (h3s->type == XQC_H3_STREAM_TYPE_BYTESTEAM) {
//TODO: mark the fin flag of the bytestream and record time
xqc_h3_ext_bytestream_fin_rcvd(h3s->h3_ext_bs);
xqc_h3_ext_bytestream_set_fin_rcvd_flag(h3s->h3_ext_bs);
if (h3s->h3_ext_bs) {
xqc_h3_ext_bytestream_fin_rcvd(h3s->h3_ext_bs);
xqc_h3_ext_bytestream_set_fin_rcvd_flag(h3s->h3_ext_bs);
}
}
}

Expand Down Expand Up @@ -1882,6 +1884,7 @@ xqc_h3_stream_read_notify(xqc_stream_t *stream, void *user_data)

/* TODO: BYTESTRAM: notify DATA to application ASAP */
if (h3s->type == XQC_H3_STREAM_TYPE_BYTESTEAM
&& h3s->h3_ext_bs
&& xqc_h3_ext_bytestream_should_notify_read(h3s->h3_ext_bs))
{
ret = xqc_h3_ext_bytestream_notify_read(h3s->h3_ext_bs);
Expand Down
24 changes: 20 additions & 4 deletions src/transport/xqc_frame.c
Original file line number Diff line number Diff line change
Expand Up @@ -619,13 +619,29 @@ xqc_process_stream_frame(xqc_connection_t *conn, xqc_packet_in_t *packet_in)
return ret;
}

xqc_int_t
xqc_check_crypto_frame_data_buffer_exceed(xqc_stream_t *stream, xqc_stream_frame_t *stream_frame, uint64_t threshold)
{
if (stream_frame->data_offset + stream_frame->data_length > stream->stream_data_in.next_read_offset + threshold) {
return -XQC_ELIMIT;
}
return XQC_OK;
}

xqc_int_t
xqc_insert_crypto_frame(xqc_connection_t *conn, xqc_stream_t *stream, xqc_stream_frame_t *stream_frame)
{
unsigned char inserted = 0;
xqc_list_head_t *pos;
xqc_stream_frame_t *frame;
xqc_int_t ret;
ret = xqc_check_crypto_frame_data_buffer_exceed(stream, stream_frame, XQC_CONN_MAX_CRYPTO_DATA_TOTAL_LEN);
if (ret != XQC_OK) {
XQC_CONN_ERR(conn, TRA_CRYPTO_BUFFER_EXCEEDED);
xqc_log(conn->log, XQC_LOG_ERROR,
"|crypto frame data buffer exceed|");
return ret;
}
xqc_list_for_each_reverse(pos, &stream->stream_data_in.frames_tailq) {
frame = xqc_list_entry(pos, xqc_stream_frame_t, sf_list);

Expand Down Expand Up @@ -697,7 +713,7 @@ xqc_process_crypto_frame(xqc_connection_t *conn, xqc_packet_in_t *packet_in)
if (ret != XQC_OK) {
xqc_log(conn->log, XQC_LOG_ERROR, "|xqc_insert_crypto_frame error|");
xqc_destroy_stream_frame(stream_frame);
return -1;
return ret;
}

ret = xqc_read_crypto_stream(stream);
Expand Down Expand Up @@ -1508,8 +1524,8 @@ xqc_process_path_challenge_frame(xqc_connection_t *conn, xqc_packet_in_t *packet
}

xqc_log(conn->log, XQC_LOG_DEBUG,
"|path:%ui|state:%d|RECV path_challenge_data:%s|cid:%s|",
path->path_id, path->path_state,
"|path:%ui|state:%d|RECV path_challenge_data:%*s|cid:%s|",
path->path_id, path->path_state, XQC_PATH_CHALLENGE_DATA_LEN,
path_challenge_data, xqc_dcid_str(conn->engine, &packet_in->pi_pkt.pkt_dcid));

ret = xqc_write_path_response_frame_to_packet(conn, path, path_challenge_data);
Expand Down Expand Up @@ -2110,4 +2126,4 @@ xqc_process_repair_frame(xqc_connection_t *conn, xqc_packet_in_t *packet_in)
return XQC_OK;
}

#endif
#endif
11 changes: 10 additions & 1 deletion src/transport/xqc_send_queue.c
Original file line number Diff line number Diff line change
Expand Up @@ -277,10 +277,19 @@ xqc_send_queue_remove_probe(xqc_list_head_t *pos)
void
xqc_send_queue_insert_unacked(xqc_packet_out_t *packet_out, xqc_list_head_t *head, xqc_send_queue_t *send_queue)
{
xqc_connection_t *conn = send_queue->sndq_conn;
xqc_list_add_tail(&packet_out->po_list, head);
if (!(packet_out->po_flag & XQC_POF_IN_UNACK_LIST)) {
send_queue->sndq_packets_in_unacked_list++;
packet_out->po_flag |= XQC_POF_IN_UNACK_LIST;
if (send_queue->sndq_packets_in_unacked_list > XQC_SNDQ_MAX_UNACK_PACKETS_LIMIT) {
if (conn) {
XQC_CONN_ERR(conn, XQC_ELIMIT);
xqc_log(conn->log, XQC_LOG_ERROR,
"|sndq unack packets exceed|sndq_packets_in_unacked_list:%ui|",
send_queue->sndq_packets_in_unacked_list);
}
}
}
}

Expand Down Expand Up @@ -710,4 +719,4 @@ xqc_send_queue_drop_stream_frame_packets(xqc_connection_t *conn, xqc_stream_id_t
if (count > 0) {
xqc_log(conn->log, XQC_LOG_DEBUG, "|stream_id:%ui|to_drop: %d|count:%d|", stream_id, to_drop, count);
}
}
}
1 change: 1 addition & 0 deletions src/transport/xqc_send_queue.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#define XQC_SNDQ_PACKETS_USED_MAX 18000
#define XQC_SNDQ_RELEASE_ENOUGH_SPACE_TH 10 /* 1 / 10*/
#define XQC_SNDQ_MAX_UNACK_PACKETS_LIMIT (100 * 1000) /* limit unack packets to avoid ddos attack */

typedef struct xqc_send_queue_s {

Expand Down
4 changes: 2 additions & 2 deletions tests/test_server.c
Original file line number Diff line number Diff line change
Expand Up @@ -1367,8 +1367,8 @@ xqc_server_request_read_notify(xqc_h3_request_t *h3_request, xqc_request_notify_

for (int i = 0; i < headers->count; i++) {
printf("%s = %s\n", (char *)headers->headers[i].name.iov_base, (char *)headers->headers[i].value.iov_base);

if (memcmp((char *)headers->headers[i].name.iov_base, "priority", 8) == 0) {
if (headers->headers[i].name.iov_len == 8
&& memcmp((char *)headers->headers[i].name.iov_base, "priority", 8) == 0) {
xqc_h3_priority_t h3_prio;
xqc_int_t ret = xqc_parse_http_priority(&h3_prio,
headers->headers[i].value.iov_base,
Expand Down

0 comments on commit 39539fe

Please sign in to comment.