Skip to content

quic: fix channel leak when ossl_quic_provide_initial_secret fails - #30612

Closed
programsurf wants to merge 1 commit into
openssl:masterfrom
programsurf:fix/quic-channel-leak
Closed

programsurf wants to merge 1 commit into
openssl:masterfrom
programsurf:fix/quic-channel-leak

Conversation

@programsurf

Copy link
Copy Markdown

In port_bind_channel(), when ossl_quic_provide_initial_secret()
fails, the function returns without freeing the QUIC_CHANNEL
that was just created by port_make_channel(). The caller sees
new_ch == NULL and cannot free it, leaking the channel and all
its sub-allocations (QRX, QTX, TXP, ACKM).

Add ossl_quic_channel_free(ch) before the early return, matching
the cleanup pattern already used by the other error paths in the
same function (lines 864, 873).

CWE-401

Reported-by: Sunwoo Lee [email protected]

CLA: trivial

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

  In port_bind_channel(), when ossl_quic_provide_initial_secret()
  fails, the function returns without freeing the QUIC_CHANNEL
  that was just created by port_make_channel(). The caller sees
  new_ch == NULL and cannot free it, leaking the channel and all
  its sub-allocations (QRX, QTX, TXP, ACKM).

  Add ossl_quic_channel_free(ch) before the early return, matching
  the cleanup pattern already used by the other error paths in the
  same function (lines 864, 873).

  CWE-401

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

  CLA: trivial
@programsurf
programsurf force-pushed the fix/quic-channel-leak branch from 2337f2c to 31c270a Compare March 27, 2026 23:32
@openssl-machine openssl-machine added the approval: review pending This pull request needs review by a committer label Mar 27, 2026

@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.

yes, looks good. thank you.

@Sashan Sashan added branch: master Applies to master branch branch: 3.3 Applies to openssl-3.3 (EOL) branch: 3.4 Applies to openssl-3.4 (EOL) branch: 3.5 Applies to openssl-3.5 branch: 4.0 Applies to openssl-4.0 branch: 3.6 Applies to openssl-3.6 (EOL) and removed branch: 3.3 Applies to openssl-3.3 (EOL) branch: 3.4 Applies to openssl-3.4 (EOL) labels Mar 28, 2026
@t8m t8m added triaged: bug The issue/pr is/fixes a bug tests: exempted The PR is exempt from requirements for testing labels Apr 8, 2026
Comment thread ssl/quic/quic_port.c Resolved
@openssl-machine openssl-machine added approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer labels Apr 8, 2026
@esyr

esyr commented Apr 8, 2026

Copy link
Copy Markdown
Member

Fixes: 8f74d8cee3630 "If our server channel creates its own qrx, set its initial secret"

@openssl-machine

Copy link
Copy Markdown
Collaborator

24 hours has passed since 'approval: done' was set, but as this PR has been updated in that time the label 'approval: ready to merge' is not being automatically set. Please review the updates and set the label manually.

@esyr esyr added approval: ready to merge The 24 hour grace period has passed, ready to merge and removed approval: done This pull request has the required number of approvals labels Apr 9, 2026
openssl-machine pushed a commit that referenced this pull request Apr 15, 2026
  In port_bind_channel(), when ossl_quic_provide_initial_secret()
  fails, the function returns without freeing the QUIC_CHANNEL
  that was just created by port_make_channel(). The caller sees
  new_ch == NULL and cannot free it, leaking the channel and all
  its sub-allocations (QRX, QTX, TXP, ACKM).

  Add ossl_quic_channel_free(ch) before the early return, matching
  the cleanup pattern already used by the other error paths in the
  same function (lines 864, 873).

  CWE-401

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

  CLA: trivial

Reviewed-by: Eugene Syromiatnikov <[email protected]>
Reviewed-by: Saša Nedvědický <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
MergeDate: Wed Apr 15 10:44:51 2026
(Merged from #30612)
openssl-machine pushed a commit that referenced this pull request Apr 15, 2026
  In port_bind_channel(), when ossl_quic_provide_initial_secret()
  fails, the function returns without freeing the QUIC_CHANNEL
  that was just created by port_make_channel(). The caller sees
  new_ch == NULL and cannot free it, leaking the channel and all
  its sub-allocations (QRX, QTX, TXP, ACKM).

  Add ossl_quic_channel_free(ch) before the early return, matching
  the cleanup pattern already used by the other error paths in the
  same function (lines 864, 873).

  CWE-401

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

  CLA: trivial

Reviewed-by: Eugene Syromiatnikov <[email protected]>
Reviewed-by: Saša Nedvědický <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
MergeDate: Wed Apr 15 10:47:50 2026
(Merged from #30612)
openssl-machine pushed a commit that referenced this pull request Apr 15, 2026
  In port_bind_channel(), when ossl_quic_provide_initial_secret()
  fails, the function returns without freeing the QUIC_CHANNEL
  that was just created by port_make_channel(). The caller sees
  new_ch == NULL and cannot free it, leaking the channel and all
  its sub-allocations (QRX, QTX, TXP, ACKM).

  Add ossl_quic_channel_free(ch) before the early return, matching
  the cleanup pattern already used by the other error paths in the
  same function (lines 864, 873).

  CWE-401

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

  CLA: trivial

Reviewed-by: Eugene Syromiatnikov <[email protected]>
Reviewed-by: Saša Nedvědický <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
MergeDate: Wed Apr 15 10:48:05 2026
(Merged from #30612)
@jogme

jogme commented Apr 15, 2026

Copy link
Copy Markdown
Contributor

Merged to labeled branches. Thank you for the contribution!

@jogme jogme closed this Apr 15, 2026
openssl-machine pushed a commit that referenced this pull request Apr 15, 2026
  In port_bind_channel(), when ossl_quic_provide_initial_secret()
  fails, the function returns without freeing the QUIC_CHANNEL
  that was just created by port_make_channel(). The caller sees
  new_ch == NULL and cannot free it, leaking the channel and all
  its sub-allocations (QRX, QTX, TXP, ACKM).

  Add ossl_quic_channel_free(ch) before the early return, matching
  the cleanup pattern already used by the other error paths in the
  same function (lines 864, 873).

  CWE-401

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

  CLA: trivial

Reviewed-by: Eugene Syromiatnikov <[email protected]>
Reviewed-by: Saša Nedvědický <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
MergeDate: Wed Apr 15 10:48:21 2026
(Merged from #30612)
daunabomba pushed a commit to daunabomba/openssl that referenced this pull request Jul 24, 2026
  In port_bind_channel(), when ossl_quic_provide_initial_secret()
  fails, the function returns without freeing the QUIC_CHANNEL
  that was just created by port_make_channel(). The caller sees
  new_ch == NULL and cannot free it, leaking the channel and all
  its sub-allocations (QRX, QTX, TXP, ACKM).

  Add ossl_quic_channel_free(ch) before the early return, matching
  the cleanup pattern already used by the other error paths in the
  same function (lines 864, 873).

  CWE-401

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

  CLA: trivial

Reviewed-by: Eugene Syromiatnikov <[email protected]>
Reviewed-by: Saša Nedvědický <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
MergeDate: Wed Apr 15 10:48:21 2026
(Merged from openssl#30612)
rickyringler pushed a commit to rickyringler/openssl that referenced this pull request Aug 21, 2026
  In port_bind_channel(), when ossl_quic_provide_initial_secret()
  fails, the function returns without freeing the QUIC_CHANNEL
  that was just created by port_make_channel(). The caller sees
  new_ch == NULL and cannot free it, leaking the channel and all
  its sub-allocations (QRX, QTX, TXP, ACKM).

  Add ossl_quic_channel_free(ch) before the early return, matching
  the cleanup pattern already used by the other error paths in the
  same function (lines 864, 873).

  CWE-401

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

  CLA: trivial

Reviewed-by: Eugene Syromiatnikov <[email protected]>
Reviewed-by: Saša Nedvědický <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
MergeDate: Wed Apr 15 10:44:51 2026
(Merged from openssl#30612)
pr000000f added a commit to pr000000f/tongsuo-dev that referenced this pull request Sep 23, 2026
…ails

Imported from OpenSSL (openssl/openssl).
Upstream commit: c88801515da4f328cb063c472d3727144eadc0c7
Upstream PR: openssl/openssl#30612
This is a prerequisite for backporting CVE-2026-18798.
OpenSSL original commit message
From c88801515da4f328cb063c472d3727144eadc0c7 Mon Sep 17 00:00:00 2001
From: Sunwoo Lee <[email protected]>
Date: Sat, 28 Mar 2026 08:22:02 +0900
Subject: [PATCH] quic: fix channel leak when ossl_quic_provide_initial_secret
 fails
  In port_bind_channel(), when ossl_quic_provide_initial_secret()
  fails, the function returns without freeing the QUIC_CHANNEL
  that was just created by port_make_channel(). The caller sees
  new_ch == NULL and cannot free it, leaking the channel and all
  its sub-allocations (QRX, QTX, TXP, ACKM).
  Add ossl_quic_channel_free(ch) before the early return, matching
  the cleanup pattern already used by the other error paths in the
  same function (lines 864, 873).
  CWE-401
  Reported-by: Sunwoo Lee <[email protected]>
  CLA: trivial
Reviewed-by: Eugene Syromiatnikov <[email protected]>
Reviewed-by: Saša Nedvědický <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
MergeDate: Wed Apr 15 10:47:50 2026
(Merged from openssl/openssl#30612)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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 tests: exempted The PR is exempt from requirements for testing triaged: bug The issue/pr is/fixes a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants