From 39539fe58f1b6b466cb00e6285197828a178bbe0 Mon Sep 17 00:00:00 2001 From: dreamwind1985 Date: Wed, 4 Dec 2024 18:39:13 +0800 Subject: [PATCH] fix vulnerablilities:NULL pointer dereference and Heap OOB read and Stack OOB read when copying a string for logs, and limit crypto stream frame data buffer and unack packets send buffer --- src/http3/xqc_h3_stream.c | 7 +++++-- src/transport/xqc_frame.c | 24 ++++++++++++++++++++---- src/transport/xqc_send_queue.c | 11 ++++++++++- src/transport/xqc_send_queue.h | 1 + tests/test_server.c | 4 ++-- 5 files changed, 38 insertions(+), 9 deletions(-) diff --git a/src/http3/xqc_h3_stream.c b/src/http3/xqc_h3_stream.c index 2e876ac1..19a8322c 100644 --- a/src/http3/xqc_h3_stream.c +++ b/src/http3/xqc_h3_stream.c @@ -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); + } } } @@ -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); diff --git a/src/transport/xqc_frame.c b/src/transport/xqc_frame.c index 5ef9ebd0..aaf0dd28 100644 --- a/src/transport/xqc_frame.c +++ b/src/transport/xqc_frame.c @@ -619,6 +619,14 @@ 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) @@ -626,6 +634,14 @@ xqc_insert_crypto_frame(xqc_connection_t *conn, xqc_stream_t *stream, xqc_stream 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); @@ -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); @@ -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); @@ -2110,4 +2126,4 @@ xqc_process_repair_frame(xqc_connection_t *conn, xqc_packet_in_t *packet_in) return XQC_OK; } -#endif \ No newline at end of file +#endif diff --git a/src/transport/xqc_send_queue.c b/src/transport/xqc_send_queue.c index 64b7b4a4..b7bb850b 100644 --- a/src/transport/xqc_send_queue.c +++ b/src/transport/xqc_send_queue.c @@ -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); + } + } } } @@ -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); } -} \ No newline at end of file +} diff --git a/src/transport/xqc_send_queue.h b/src/transport/xqc_send_queue.h index 4cca22eb..0267dff0 100644 --- a/src/transport/xqc_send_queue.h +++ b/src/transport/xqc_send_queue.h @@ -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 { diff --git a/tests/test_server.c b/tests/test_server.c index 7f7fd027..745b0146 100644 --- a/tests/test_server.c +++ b/tests/test_server.c @@ -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,