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

quic: fix channel leak when ossl_quic_provide_initial_secret fails#30612
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 sunwoolee@kentech.ac.kr

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 <sunwoolee@kentech.ac.kr>

  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 branch: 3.5 Applies to openssl-3.5 branch: 4.0 Applies to openssl-4.0 branch: 3.6 Applies to openssl-3.6 and removed branch: 3.3 Applies to openssl-3.3 (EOL) branch: 3.4 Applies to openssl-3.4 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
@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 <sunwoolee@kentech.ac.kr>

  CLA: trivial

Reviewed-by: Eugene Syromiatnikov <esyr@openssl.org>
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.foundation>
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 <sunwoolee@kentech.ac.kr>

  CLA: trivial

Reviewed-by: Eugene Syromiatnikov <esyr@openssl.org>
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.foundation>
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 <sunwoolee@kentech.ac.kr>

  CLA: trivial

Reviewed-by: Eugene Syromiatnikov <esyr@openssl.org>
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.foundation>
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 <sunwoolee@kentech.ac.kr>

  CLA: trivial

Reviewed-by: Eugene Syromiatnikov <esyr@openssl.org>
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.foundation>
MergeDate: Wed Apr 15 10:48:21 2026
(Merged from #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 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