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 (#468)

Author: dreamwind1985

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);

src/transport/xqc_frame.c

@@ -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);
 
@@ -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
+#endif

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);
     }
-}
+}

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 {
 

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,