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 3 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
86 changes: 84 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,82 @@ 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, pkt_flag_init, pkt_flag_hsk, pkt_flag_0rtt, pkt_flag_1rtt;
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);
allowed = XQC_FALSE;

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

if (frame_type > 0x1e) {
return XQC_OK;
}

switch (frame_type) {
/* IH01 */
case 0x00: /* PADDING */
case 0x01: /* PING */
case 0x1c: /* CONNECTION_CLOSE (transport) */
case 0x1d: /* CONNECTION_CLOSE (application) */
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 */
default:
if ((frame_type >= 0x04 && frame_type <= 0x05) /* RESET_STREAM, STOP_SENDING */
|| (frame_type >= 0x08 && frame_type <= 0x1a)) /* STREAM..PATH_CHALLENGE */
{
allowed = (pkt_flag_0rtt || pkt_flag_1rtt);
} else {
return XQC_OK;
}
break;
}

if (!allowed) {
xqc_log(conn->log, XQC_LOG_ERROR,
"|illegal frame in packet type|frame_type:%xL|pkt_type:%s|",
frame_type, xqc_pkt_type_2_str(pkt_type));
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 +288,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 +528,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
55 changes: 53 additions & 2 deletions src/transport/xqc_packet_out.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -994,13 +994,36 @@ xqc_write_stream_data_blocked_to_packet(xqc_connection_t *conn, xqc_stream_id_t
return -XQC_EWRITE_PKT;
}

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

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

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_streams_blocked_to_packet(xqc_connection_t *conn, uint64_t stream_limit, int bidirectional)
{
ssize_t ret;
xqc_packet_out_t *packet_out;
xqc_bool_t buff_pkt = XQC_FALSE;
xqc_pkt_type_t pkt_type = xqc_get_pkt_type_for_app_data_frame(conn, &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 @@ -1016,6 +1039,10 @@ xqc_write_streams_blocked_to_packet(xqc_connection_t *conn, uint64_t stream_limi

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

return XQC_OK;

error:
Expand Down Expand Up @@ -1057,6 +1084,20 @@ 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;

if (pkt_type == XQC_PTYPE_NUM
|| pkt_type == XQC_PTYPE_INIT
|| pkt_type == XQC_PTYPE_HSK)
{
pkt_type = xqc_get_pkt_type_for_app_data_frame(conn, &buff_pkt);
}

if (pkt_type == XQC_PTYPE_SHORT_HEADER
&& (conn->conn_flag & XQC_CONN_FLAG_CAN_SEND_1RTT) == 0)
{
buff_pkt = XQC_TRUE;
}

packet_out = xqc_write_new_packet(conn, pkt_type);
if (packet_out == NULL) {
Expand All @@ -1074,6 +1115,10 @@ xqc_write_max_stream_data_to_packet(xqc_connection_t *conn, xqc_stream_id_t stre

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

return XQC_OK;

error:
Expand All @@ -1086,13 +1131,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_get_pkt_type_for_app_data_frame(conn, &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 @@ -1108,6 +1155,10 @@ xqc_write_max_streams_to_packet(xqc_connection_t *conn, uint64_t max_stream, int

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

xqc_log(conn->log, XQC_LOG_DEBUG, "|new_max_stream:%ui|", max_stream);
return XQC_OK;

Expand Down
Loading