Skip to content

[+] Fix: Validate frame types in packets to prevent protocol violations #525

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Sy0307 wants to merge 9 commits into
base: main
Choose a base branch
from
Open

[+] Fix: Validate frame types in packets to prevent protocol violations #525

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
8102e1a
[+] Fix: Validate frame types in packets to prevent protocol violations
Sy0307 Dec 15, 2025
0a5a962
[+] Fix: Refactor frame type validation logic in packets for clarity …
Sy0307 Dec 17, 2025
8dea4f9
[+] Fix: Add packet type determination to follow QUIC RFC table 3 spe…
Sy0307 Dec 18, 2025
e8469ec
[~] Fix: Enhance frame type validation and add tests for application …
Sy0307 May 7, 2026
d8a9978
[~] Fix: Refine frame validation for RFC compliance
Sy0307 May 29, 2026
116f2d2
[~] Fix: Resolve conflicts with main
Sy0307 May 29, 2026
590f872
[~] Fix: Use dynamic crypto error range
Sy0307 May 29, 2026
70abcd0
[~] Fix: Restore merge-lost stream frame test helper and includes
Sy0307 Jun 1, 2026
480f9a1
[~] Fix: Set correct pkt_type in CID tests, avoid CRYPTO_ERROR collis…
Sy0307 Jun 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
109 changes: 107 additions & 2 deletions src/transport/xqc_frame.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,105 @@ xqc_insert_stream_frame(xqc_connection_t *conn, xqc_stream_t *stream, xqc_stream
}


static xqc_int_t
xqc_validate_frame_type_in_pkt(xqc_connection_t *conn, xqc_packet_in_t *packet_in, uint64_t frame_type)
{
/*
* RFC 9000 Section 12.4 Table 3:
* frame types that are permitted in each packet type.
*/
xqc_bool_t allowed = XQC_FALSE;
xqc_bool_t pkt_flag_init = XQC_FALSE, pkt_flag_hsk = XQC_FALSE;
xqc_bool_t pkt_flag_0rtt = XQC_FALSE, pkt_flag_1rtt = XQC_FALSE;
if ((packet_in->pi_flag & XQC_PIF_FEC_RECOVERED) != 0) {
return XQC_OK;
}
xqc_pkt_type_t pkt_type = packet_in->pi_pkt.pkt_type;
if (pkt_type >= XQC_PTYPE_NUM) {
return XQC_OK;
}
pkt_flag_init = (pkt_type == XQC_PTYPE_INIT);
pkt_flag_hsk = (pkt_type == XQC_PTYPE_HSK);
pkt_flag_0rtt = (pkt_type == XQC_PTYPE_0RTT);
pkt_flag_1rtt = (pkt_type == XQC_PTYPE_SHORT_HEADER);

if (!pkt_flag_init && !pkt_flag_hsk && !pkt_flag_0rtt && !pkt_flag_1rtt) {
return XQC_OK;
}

if (frame_type > 0x1e && frame_type != 0x30 && frame_type != 0x31) {
return XQC_OK;
}

switch (frame_type) {
case 0x30:
case 0x31:
allowed = (pkt_flag_0rtt || pkt_flag_1rtt);
break;

/* IH01 */
case 0x00: /* PADDING */
case 0x01: /* PING */
case 0x1c: /* CONNECTION_CLOSE (transport) */
allowed = XQC_TRUE;

@Yanmei-Liu Yanmei-Liu May 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not true. Actually in RFC 9000 which says:

Sending a CONNECTION_CLOSE of type 0x1d in an Initial or Handshake packet could expose application state or be used to alter application state. A CONNECTION_CLOSE of type 0x1d MUST be replaced by a CONNECTION_CLOSE of type 0x1c when sending the frame in Initial or Handshake packets. Otherwise, information about the application state might be revealed. Endpoints MUST clear the value of the Reason Phrase field and SHOULD use the APPLICATION_ERROR code when converting to a CONNECTION_CLOSE of type 0x1c.

And also in Section 12.5 "Frames and Number Spaces" which says:

CONNECTION_CLOSE frames signaling errors at the QUIC layer (type
0x1c) MAY appear in any packet number space. CONNECTION_CLOSE
frames signaling application errors (type 0x1d) MUST only appear
in the application data packet number space.

break;

/* IH_1 */
case 0x02: /* ACK */
case 0x03: /* ACK (ECN) */
case 0x06: /* CRYPTO */
allowed = (pkt_flag_init || pkt_flag_hsk || pkt_flag_1rtt);
break;

/* ___1 */
case 0x07: /* NEW_TOKEN */
case 0x1b: /* PATH_RESPONSE */
case 0x1e: /* HANDSHAKE_DONE */
allowed = pkt_flag_1rtt;
break;

/* __01 */
case 0x04: /* RESET_STREAM */
case 0x05: /* STOP_SENDING */
case 0x08: /* STREAM */
case 0x09:
case 0x0a:
case 0x0b:
case 0x0c:
case 0x0d:
case 0x0e:
case 0x0f:
case 0x10: /* MAX_DATA */
case 0x11: /* MAX_STREAM_DATA */
case 0x12: /* MAX_STREAMS */
case 0x13:
case 0x14: /* DATA_BLOCKED */
case 0x15: /* STREAM_DATA_BLOCKED */
case 0x16: /* STREAMS_BLOCKED */
case 0x17:
case 0x18: /* NEW_CONNECTION_ID */
case 0x19: /* RETIRE_CONNECTION_ID */
case 0x1a: /* PATH_CHALLENGE */
case 0x1d: /* CONNECTION_CLOSE (application) */
allowed = (pkt_flag_0rtt || pkt_flag_1rtt);
break;

default:
return XQC_OK;
}

if (!allowed) {
xqc_log(conn->log, XQC_LOG_ERROR,
"|illegal frame in packet type|frame_type:%xL|pkt_type:%s|pkt_num:%ui|",
frame_type, xqc_pkt_type_2_str(pkt_type), packet_in->pi_pkt.pkt_num);
XQC_CONN_ERR(conn, TRA_PROTOCOL_VIOLATION);
return -XQC_EPROTO;
}

return XQC_OK;
}


xqc_int_t
xqc_process_frames(xqc_connection_t *conn, xqc_packet_in_t *packet_in)
{
Expand Down Expand Up @@ -212,6 +311,11 @@ xqc_process_frames(xqc_connection_t *conn, xqc_packet_in_t *packet_in)

xqc_log(conn->log, XQC_LOG_DEBUG, "|frame_type:%xL|", frame_type);

ret = xqc_validate_frame_type_in_pkt(conn, packet_in, frame_type);
if (ret != XQC_OK) {
return ret;
}

switch (frame_type) {

case 0x00:
Expand Down Expand Up @@ -447,8 +551,9 @@ xqc_process_stream_frame(xqc_connection_t *conn, xqc_packet_in_t *packet_in)
xqc_stream_t *stream = NULL;
xqc_stream_frame_t *stream_frame;

if (packet_in->pi_pkt.pkt_type == XQC_PTYPE_INIT
|| packet_in->pi_pkt.pkt_type == XQC_PTYPE_HSK)
if ((packet_in->pi_flag & XQC_PIF_FEC_RECOVERED) == 0
&& (packet_in->pi_pkt.pkt_type == XQC_PTYPE_INIT
|| packet_in->pi_pkt.pkt_type == XQC_PTYPE_HSK))
{
xqc_log(conn->log, XQC_LOG_ERROR,
"|illegal STREAM frame in %s packet, close with PROTOCOL_VIOLATION|",
Expand Down
108 changes: 96 additions & 12 deletions src/transport/xqc_packet_out.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -789,13 +789,23 @@ xqc_write_conn_close_to_packet(xqc_connection_t *conn, uint64_t err_code)
pkt_type = XQC_PTYPE_SHORT_HEADER;
}

xqc_bool_t is_app = err_code >= H3_NO_ERROR;
if (pkt_type == XQC_PTYPE_INIT || pkt_type == XQC_PTYPE_HSK) {
if (err_code >= TRA_CRYPTO_ERROR_BASE && err_code <= TRA_CRYPTO_ERROR_BASE + 0xff) {
is_app = XQC_FALSE;
} else if (is_app) {
is_app = XQC_FALSE;
err_code = TRA_APPLICATION_ERROR;
}
}

packet_out = xqc_write_new_packet(conn, pkt_type);
if (packet_out == NULL) {
xqc_log(conn->log, XQC_LOG_ERROR, "|xqc_write_new_packet error|");
return -XQC_EWRITE_PKT;
}

ret = xqc_gen_conn_close_frame(packet_out, err_code, err_code >= H3_NO_ERROR ? 1:0, 0);
ret = xqc_gen_conn_close_frame(packet_out, err_code, is_app, 0);
if (ret < 0) {
xqc_log(conn->log, XQC_LOG_ERROR, "|xqc_gen_conn_close_frame error|");
goto error;
Expand Down Expand Up @@ -935,13 +945,51 @@ xqc_write_stop_sending_to_packet(xqc_connection_t *conn, xqc_stream_t *stream,
return -XQC_EWRITE_PKT;
}


static xqc_pkt_type_t
xqc_select_pkt_type_for_app_data_frame(xqc_connection_t *conn,
xqc_pkt_type_t requested_pkt_type, xqc_bool_t *buff_pkt)
{
*buff_pkt = XQC_FALSE;

if (requested_pkt_type == XQC_PTYPE_0RTT) {
return XQC_PTYPE_0RTT;
}

if (conn->conn_flag & XQC_CONN_FLAG_CAN_SEND_1RTT) {
if (requested_pkt_type == XQC_PTYPE_SHORT_HEADER) {
return requested_pkt_type;
}

return XQC_PTYPE_SHORT_HEADER;
}

if (requested_pkt_type == XQC_PTYPE_SHORT_HEADER) {
*buff_pkt = XQC_TRUE;
return requested_pkt_type;
}

if (conn->conn_type == XQC_CONN_TYPE_CLIENT
&& conn->conn_state == XQC_CONN_STATE_CLIENT_INITIAL_SENT
&& xqc_conn_is_ready_to_send_early_data(conn))
{
conn->conn_flag |= XQC_CONN_FLAG_HAS_0RTT;
return XQC_PTYPE_0RTT;
}

*buff_pkt = XQC_TRUE;
return XQC_PTYPE_SHORT_HEADER;
}

int
xqc_write_data_blocked_to_packet(xqc_connection_t *conn, uint64_t data_limit)
{
ssize_t ret;
xqc_packet_out_t *packet_out;
xqc_bool_t buff_pkt = XQC_FALSE;
xqc_pkt_type_t pkt_type = xqc_select_pkt_type_for_app_data_frame(conn, XQC_PTYPE_NUM, &buff_pkt);

packet_out = xqc_write_new_packet(conn, XQC_PTYPE_SHORT_HEADER);
packet_out = xqc_write_new_packet(conn, pkt_type);
if (packet_out == NULL) {
xqc_log(conn->log, XQC_LOG_ERROR, "|xqc_write_new_packet error|");
return -XQC_EWRITE_PKT;
Expand All @@ -956,7 +1004,11 @@ xqc_write_data_blocked_to_packet(xqc_connection_t *conn, uint64_t data_limit)
packet_out->po_used_size += ret;

/* we need to send this packet asap */
xqc_send_queue_move_to_high_pri(&packet_out->po_list, conn->conn_send_queue);
if (buff_pkt) {
xqc_conn_buff_1rtt_packet(conn, packet_out);
} else {
xqc_send_queue_move_to_high_pri(&packet_out->po_list, conn->conn_send_queue);
}

return XQC_OK;

Expand All @@ -970,7 +1022,10 @@ xqc_write_stream_data_blocked_to_packet(xqc_connection_t *conn, xqc_stream_id_t
{
ssize_t ret;
xqc_packet_out_t *packet_out;
packet_out = xqc_write_new_packet(conn, XQC_PTYPE_SHORT_HEADER);
xqc_bool_t buff_pkt = XQC_FALSE;
xqc_pkt_type_t pkt_type = xqc_select_pkt_type_for_app_data_frame(conn, XQC_PTYPE_NUM, &buff_pkt);

packet_out = xqc_write_new_packet(conn, pkt_type);
if (packet_out == NULL) {
xqc_log(conn->log, XQC_LOG_ERROR, "|xqc_write_new_packet error|");
return -XQC_EWRITE_PKT;
Expand All @@ -985,7 +1040,11 @@ xqc_write_stream_data_blocked_to_packet(xqc_connection_t *conn, xqc_stream_id_t
packet_out->po_used_size += ret;

/* we need to send this packet asap */
xqc_send_queue_move_to_high_pri(&packet_out->po_list, conn->conn_send_queue);
if (buff_pkt) {
xqc_conn_buff_1rtt_packet(conn, packet_out);
} else {
xqc_send_queue_move_to_high_pri(&packet_out->po_list, conn->conn_send_queue);
}

return XQC_OK;

Expand All @@ -999,8 +1058,10 @@ xqc_write_streams_blocked_to_packet(xqc_connection_t *conn, uint64_t stream_limi
{
ssize_t ret;
xqc_packet_out_t *packet_out;
xqc_bool_t buff_pkt = XQC_FALSE;
xqc_pkt_type_t pkt_type = xqc_select_pkt_type_for_app_data_frame(conn, XQC_PTYPE_NUM, &buff_pkt);

packet_out = xqc_write_new_packet(conn, XQC_PTYPE_NUM);
packet_out = xqc_write_new_packet(conn, pkt_type);
if (packet_out == NULL) {
xqc_log(conn->log, XQC_LOG_ERROR, "|xqc_write_new_packet error|");
return -XQC_EWRITE_PKT;
Expand All @@ -1014,7 +1075,11 @@ xqc_write_streams_blocked_to_packet(xqc_connection_t *conn, uint64_t stream_limi

packet_out->po_used_size += ret;

xqc_send_queue_move_to_high_pri(&packet_out->po_list, conn->conn_send_queue);
if (buff_pkt) {
xqc_conn_buff_1rtt_packet(conn, packet_out);
} else {
xqc_send_queue_move_to_high_pri(&packet_out->po_list, conn->conn_send_queue);
}

return XQC_OK;

Expand All @@ -1028,8 +1093,10 @@ xqc_write_max_data_to_packet(xqc_connection_t *conn, uint64_t max_data)
{
ssize_t ret;
xqc_packet_out_t *packet_out;
xqc_bool_t buff_pkt = XQC_FALSE;
xqc_pkt_type_t pkt_type = xqc_select_pkt_type_for_app_data_frame(conn, XQC_PTYPE_NUM, &buff_pkt);

packet_out = xqc_write_new_packet(conn, XQC_PTYPE_SHORT_HEADER);
packet_out = xqc_write_new_packet(conn, pkt_type);
if (packet_out == NULL) {
xqc_log(conn->log, XQC_LOG_ERROR, "|xqc_write_new_packet error|");
return -XQC_EWRITE_PKT;
Expand All @@ -1043,7 +1110,11 @@ xqc_write_max_data_to_packet(xqc_connection_t *conn, uint64_t max_data)

packet_out->po_used_size += ret;

xqc_send_queue_move_to_high_pri(&packet_out->po_list, conn->conn_send_queue);
if (buff_pkt) {
xqc_conn_buff_1rtt_packet(conn, packet_out);
} else {
xqc_send_queue_move_to_high_pri(&packet_out->po_list, conn->conn_send_queue);
}

return XQC_OK;

Expand All @@ -1057,6 +1128,9 @@ xqc_write_max_stream_data_to_packet(xqc_connection_t *conn, xqc_stream_id_t stre
{
ssize_t ret = XQC_OK;
xqc_packet_out_t *packet_out;
xqc_bool_t buff_pkt = XQC_FALSE;

pkt_type = xqc_select_pkt_type_for_app_data_frame(conn, pkt_type, &buff_pkt);

packet_out = xqc_write_new_packet(conn, pkt_type);
if (packet_out == NULL) {
Expand All @@ -1072,7 +1146,11 @@ xqc_write_max_stream_data_to_packet(xqc_connection_t *conn, xqc_stream_id_t stre

packet_out->po_used_size += ret;

xqc_send_queue_move_to_high_pri(&packet_out->po_list, conn->conn_send_queue);
if (buff_pkt) {
xqc_conn_buff_1rtt_packet(conn, packet_out);
} else {
xqc_send_queue_move_to_high_pri(&packet_out->po_list, conn->conn_send_queue);
}

return XQC_OK;

Expand All @@ -1086,13 +1164,15 @@ xqc_write_max_streams_to_packet(xqc_connection_t *conn, uint64_t max_stream, int
{
ssize_t ret = XQC_ERROR;
xqc_packet_out_t *packet_out;
xqc_bool_t buff_pkt = XQC_FALSE;
xqc_pkt_type_t pkt_type = xqc_select_pkt_type_for_app_data_frame(conn, XQC_PTYPE_NUM, &buff_pkt);

if (max_stream > XQC_MAX_STREAMS) {
xqc_log(conn->log, XQC_LOG_ERROR, "|xqc_write_max_streams_to_packet error|set max_stream:%ui", max_stream);
return -XQC_EPARAM;
}

packet_out = xqc_write_new_packet(conn, XQC_PTYPE_NUM);
packet_out = xqc_write_new_packet(conn, pkt_type);
if (packet_out == NULL) {
xqc_log(conn->log, XQC_LOG_ERROR, "|xqc_write_new_packet error|");
return -XQC_EWRITE_PKT;
Expand All @@ -1106,7 +1186,11 @@ xqc_write_max_streams_to_packet(xqc_connection_t *conn, uint64_t max_stream, int

packet_out->po_used_size += ret;

xqc_send_queue_move_to_high_pri(&packet_out->po_list, conn->conn_send_queue);
if (buff_pkt) {
xqc_conn_buff_1rtt_packet(conn, packet_out);
} else {
xqc_send_queue_move_to_high_pri(&packet_out->po_list, conn->conn_send_queue);
}

xqc_log(conn->log, XQC_LOG_DEBUG, "|new_max_stream:%ui|", max_stream);
return XQC_OK;
Expand Down
2 changes: 2 additions & 0 deletions tests/unittest/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,8 @@ main()
|| !CU_add_test(pSuite, "xqc_test_engine_packet_process", xqc_test_engine_packet_process)
|| !CU_add_test(pSuite, "xqc_test_stream_frame", xqc_test_stream_frame)
|| !CU_add_test(pSuite, "xqc_test_process_frame", xqc_test_process_frame)
|| !CU_add_test(pSuite, "xqc_test_handshake_app_conn_close_is_converted", xqc_test_handshake_app_conn_close_is_converted)
|| !CU_add_test(pSuite, "xqc_test_1rtt_only_flow_control_frames_are_buffered", xqc_test_1rtt_only_flow_control_frames_are_buffered)
|| !CU_add_test(pSuite, "xqc_test_parse_padding_frame", xqc_test_parse_padding_frame)
|| !CU_add_test(pSuite, "xqc_test_large_ack_frame", xqc_test_large_ack_frame)
/* issue #632: ACK_ECN frame parsing (RFC 9000 19.3) */
Expand Down
2 changes: 2 additions & 0 deletions tests/unittest/xqc_cid_test.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ xqc_test_recv_retire_cid()
0x00, /* Sequence Number */};
xqc_packet_in_t packet_in;
memset(&packet_in, 0, sizeof(xqc_packet_in_t));
packet_in.pi_pkt.pkt_type = XQC_PTYPE_SHORT_HEADER;
packet_in.pos = XQC_RETIRE_CID_FRAME;
packet_in.last = packet_in.pos + sizeof(XQC_RETIRE_CID_FRAME);

Expand Down Expand Up @@ -182,6 +183,7 @@ xqc_test_retire_cid_with_odcid_in_set()
0x00, /* Sequence Number */};
xqc_packet_in_t packet_in;
memset(&packet_in, 0, sizeof(xqc_packet_in_t));
packet_in.pi_pkt.pkt_type = XQC_PTYPE_SHORT_HEADER;
packet_in.pos = XQC_RETIRE_CID_FRAME;
packet_in.last = packet_in.pos + sizeof(XQC_RETIRE_CID_FRAME);

Expand Down
Loading
Loading