Skip to content

quic: zero-initialize scid and odcid in port_default_packet_handler - #30611

Closed
programsurf wants to merge 1 commit into
openssl:masterfrom
programsurf:fix/quic-uninit-scid
Closed

quic: zero-initialize scid and odcid in port_default_packet_handler#30611
programsurf wants to merge 1 commit into
openssl:masterfrom
programsurf:fix/quic-uninit-scid

Conversation

@programsurf

@programsurf programsurf commented Mar 27, 2026

Copy link
Copy Markdown

QUIC_CONN_ID scid and odcid are declared without initialization.
When validate_addr == 0 and the client's Initial has no token,
port_validate_token() is never called, so scid is never written.
The uninitialized scid is passed to port_bind_channel() ->
ossl_quic_channel_on_new_conn(), where garbage data is copied
to ch->cur_remote_dcid.

Zero-initialize both variables to prevent use of uninitialized
memory.

CWE-457

Reported-by: Sunwoo Lee [email protected]

CLA: trivial

Checklist
  • documentation is added or updated
  • tests are added or updated

@programsurf
programsurf force-pushed the fix/quic-uninit-scid branch from 10ae8b5 to cb09832 Compare March 27, 2026 23:33
@openssl-machine openssl-machine added the approval: review pending This pull request needs review by a committer label Mar 27, 2026
Sashan
Sashan requested changes Mar 27, 2026
View reviewed changes

@Sashan Sashan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you for spotting the issue. IMO the change can be more radical here. the scid is not needed at all at least it seems to me.

thanks for your work.

Comment thread ssl/quic/quic_port.c Resolved
@programsurf
programsurf force-pushed the fix/quic-uninit-scid branch from cb09832 to 97f2ac1 Compare March 27, 2026 23:58
Sunwoo Lee @programsurf
quic: remove unused scid from port_default_packet_handler
2dba5ff 
  Remove the scid variable entirely from port_default_packet_handler()
  and all functions that accept it as a parameter. The scid was never
  used meaningfully — cur_remote_dcid is set later during the handshake.

  Remove scid parameter from:
    - port_bind_channel()
    - port_validate_token()
    - ossl_quic_channel_on_new_conn()
    - ossl_quic_bind_channel()
    - ch_on_new_conn_common()

  Remove the cur_remote_dcid = *peer_scid assignment in
  ch_on_new_conn_common() as it wrote dead data.

  CWE-457

  Reported-by: Sunwoo Lee <[email protected]>

  CLA: trivial
@programsurf
programsurf force-pushed the fix/quic-uninit-scid branch from 97f2ac1 to 2dba5ff Compare March 28, 2026 00:00
@loci-dev loci-dev mentioned this pull request Mar 28, 2026
Open
2 tasks
@Sashan Sashan added branch: master Applies to master branch triaged: cleanup The issue/pr deals with cleanup of comments/docs not altering code significantly labels Mar 28, 2026
Sashan
Sashan approved these changes Mar 28, 2026
View reviewed changes

@Sashan Sashan left a comment •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

changes look good. thank you very much.

to committers: technically it's a cleanup change, but I think it's worth to be back ported to release branches too. if you agree just add release branch tags. thanks.

EDIT: change/cleanup is relevant to QUIC-server only, the QUIC server has been shipped with 3.5, thus the relevant branches are: 3.5, 3.6 and 4.0 the

@openssl-machine openssl-machine added approval: done This pull request has the required number of approvals approval: ready to merge The 24 hour grace period has passed, ready to merge and removed approval: review pending This pull request needs review by a committer approval: done This pull request has the required number of approvals labels Apr 3, 2026
@openssl-machine

openssl-machine commented Apr 4, 2026

Copy link
Copy Markdown
Collaborator

This pull request is ready to merge

@t8m t8m added triaged: bug The issue/pr is/fixes a bug tests: exempted The PR is exempt from requirements for testing branch: 3.5 Applies to openssl-3.5 branch: 3.6 Applies to openssl-3.6 (EOL) branch: 4.0 Applies to openssl-4.0 cla: trivial One of the commits is marked as 'CLA: trivial'. Review for non-trivial change in such commit. labels Apr 8, 2026
@t8m

t8m commented Apr 8, 2026

Copy link
Copy Markdown
Member

OK with CLA: trivial as it is just removing unused code.

@t8m

t8m commented Apr 8, 2026

Copy link
Copy Markdown
Member

Merged to the master, 4.0, 3.6 and 3.5 branches. Thank you for your contribution.

@t8m t8m closed this Apr 8, 2026
openssl-machine pushed a commit that referenced this pull request Apr 8, 2026
Sunwoo Lee @t8m
quic: remove unused scid from port_default_packet_handler
9648d58 
  Remove the scid variable entirely from port_default_packet_handler()
  and all functions that accept it as a parameter. The scid was never
  used meaningfully — cur_remote_dcid is set later during the handshake.

  Remove scid parameter from:
    - port_bind_channel()
    - port_validate_token()
    - ossl_quic_channel_on_new_conn()
    - ossl_quic_bind_channel()
    - ch_on_new_conn_common()

  Remove the cur_remote_dcid = *peer_scid assignment in
  ch_on_new_conn_common() as it wrote dead data.

  CWE-457

  Reported-by: Sunwoo Lee <[email protected]>

  CLA: trivial

Reviewed-by: Saša Nedvědický <[email protected]>
Reviewed-by: Frederik Wedel-Heinen <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
MergeDate: Wed Apr  8 10:21:55 2026
(Merged from #30611)

(cherry picked from commit 275dab5)
openssl-machine pushed a commit that referenced this pull request Apr 8, 2026
Sunwoo Lee @t8m
quic: remove unused scid from port_default_packet_handler
6967bbc 
  Remove the scid variable entirely from port_default_packet_handler()
  and all functions that accept it as a parameter. The scid was never
  used meaningfully — cur_remote_dcid is set later during the handshake.

  Remove scid parameter from:
    - port_bind_channel()
    - port_validate_token()
    - ossl_quic_channel_on_new_conn()
    - ossl_quic_bind_channel()
    - ch_on_new_conn_common()

  Remove the cur_remote_dcid = *peer_scid assignment in
  ch_on_new_conn_common() as it wrote dead data.

  CWE-457

  Reported-by: Sunwoo Lee <[email protected]>

  CLA: trivial

Reviewed-by: Saša Nedvědický <[email protected]>
Reviewed-by: Frederik Wedel-Heinen <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
MergeDate: Wed Apr  8 10:21:55 2026
(Merged from #30611)

(cherry picked from commit 275dab5)
openssl-machine pushed a commit that referenced this pull request Apr 8, 2026
Sunwoo Lee @t8m
quic: remove unused scid from port_default_packet_handler
275dab5 
  Remove the scid variable entirely from port_default_packet_handler()
  and all functions that accept it as a parameter. The scid was never
  used meaningfully — cur_remote_dcid is set later during the handshake.

  Remove scid parameter from:
    - port_bind_channel()
    - port_validate_token()
    - ossl_quic_channel_on_new_conn()
    - ossl_quic_bind_channel()
    - ch_on_new_conn_common()

  Remove the cur_remote_dcid = *peer_scid assignment in
  ch_on_new_conn_common() as it wrote dead data.

  CWE-457

  Reported-by: Sunwoo Lee <[email protected]>

  CLA: trivial

Reviewed-by: Saša Nedvědický <[email protected]>
Reviewed-by: Frederik Wedel-Heinen <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
MergeDate: Wed Apr  8 10:21:55 2026
(Merged from #30611)
openssl-machine pushed a commit that referenced this pull request Apr 8, 2026
Sunwoo Lee @t8m
quic: remove unused scid from port_default_packet_handler
0fd5f56 
  Remove the scid variable entirely from port_default_packet_handler()
  and all functions that accept it as a parameter. The scid was never
  used meaningfully — cur_remote_dcid is set later during the handshake.

  Remove scid parameter from:
    - port_bind_channel()
    - port_validate_token()
    - ossl_quic_channel_on_new_conn()
    - ossl_quic_bind_channel()
    - ch_on_new_conn_common()

  Remove the cur_remote_dcid = *peer_scid assignment in
  ch_on_new_conn_common() as it wrote dead data.

  CWE-457

  Reported-by: Sunwoo Lee <[email protected]>

  CLA: trivial

Reviewed-by: Saša Nedvědický <[email protected]>
Reviewed-by: Frederik Wedel-Heinen <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
MergeDate: Wed Apr  8 10:21:55 2026
(Merged from #30611)

(cherry picked from commit 275dab5)
This was referenced Apr 13, 2026
Closed
Closed
Closed
rickyringler pushed a commit to rickyringler/openssl that referenced this pull request Aug 21, 2026
Sunwoo Lee Ricky Ringler
quic: remove unused scid from port_default_packet_handler
f403919 
  Remove the scid variable entirely from port_default_packet_handler()
  and all functions that accept it as a parameter. The scid was never
  used meaningfully — cur_remote_dcid is set later during the handshake.

  Remove scid parameter from:
    - port_bind_channel()
    - port_validate_token()
    - ossl_quic_channel_on_new_conn()
    - ossl_quic_bind_channel()
    - ch_on_new_conn_common()

  Remove the cur_remote_dcid = *peer_scid assignment in
  ch_on_new_conn_common() as it wrote dead data.

  CWE-457

  Reported-by: Sunwoo Lee <[email protected]>

  CLA: trivial

Reviewed-by: Saša Nedvědický <[email protected]>
Reviewed-by: Frederik Wedel-Heinen <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
MergeDate: Wed Apr  8 10:21:55 2026
(Merged from openssl#30611)
@esyr esyr mentioned this pull request Sep 9, 2026
Closed
pr000000f added a commit to pr000000f/tongsuo-dev that referenced this pull request Sep 23, 2026
@pr000000f
Backport from OpenSSL: remove unused scid from QUIC port/channel
d2025a9 
Imported from OpenSSL (openssl/openssl).
Upstream commit: 9648d58ba6ed7b5973c0c9afaec8fa3a3f9808c6
Upstream PR: openssl/openssl#30611
This is a prerequisite for backporting CVE-2026-18798.
OpenSSL original commit message
From 9648d58ba6ed7b5973c0c9afaec8fa3a3f9808c6 Mon Sep 17 00:00:00 2001
From: Sunwoo Lee <[email protected]>
Date: Sat, 28 Mar 2026 08:58:41 +0900
Subject: [PATCH] quic: remove unused scid from port_default_packet_handler
  Remove the scid variable entirely from port_default_packet_handler()
  and all functions that accept it as a parameter. The scid was never
  used meaningfully — cur_remote_dcid is set later during the handshake.
  Remove scid parameter from:
    - port_bind_channel()
    - port_validate_token()
    - ossl_quic_channel_on_new_conn()
    - ossl_quic_bind_channel()
    - ch_on_new_conn_common()
  Remove the cur_remote_dcid = *peer_scid assignment in
  ch_on_new_conn_common() as it wrote dead data.
  CWE-457
  Reported-by: Sunwoo Lee <[email protected]>
  CLA: trivial
Reviewed-by: Saša Nedvědický <[email protected]>
Reviewed-by: Frederik Wedel-Heinen <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
MergeDate: Wed Apr  8 10:21:55 2026
(Merged from openssl/openssl#30611)
(cherry picked from commit 275dab58f07deb08e148e954c096a6d005e2c548)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Sashan Sashan Sashan approved these changes
@t8m t8m t8m approved these changes
@fwh-dc fwh-dc fwh-dc approved these changes

Assignees

No one assigned

Labels

approval: ready to merge The 24 hour grace period has passed, ready to merge branch: master Applies to master branch branch: 3.5 Applies to openssl-3.5 branch: 3.6 Applies to openssl-3.6 (EOL) branch: 4.0 Applies to openssl-4.0 cla: trivial One of the commits is marked as 'CLA: trivial'. Review for non-trivial change in such commit. tests: exempted The PR is exempt from requirements for testing triaged: bug The issue/pr is/fixes a bug triaged: cleanup The issue/pr deals with cleanup of comments/docs not altering code significantly

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@programsurf @openssl-machine @t8m @Sashan @fwh-dc