Skip to content

Fix use-after-free issue in radix test framework for QUIC. - #31421

Closed
Sashan wants to merge 6 commits into
openssl:masterfrom
Sashan:radx.ub_san
Closed

Sashan wants to merge 6 commits into
openssl:masterfrom
Sashan:radx.ub_san

Conversation

@Sashan

@Sashan Sashan commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

hf_accept_conn() tests if SSL_accept() works for QUIC. Currently the function accepts connection, then binds the conn object to radix framework and then proceeds to further tests. If further tests fail, then connection is freed and function returns to caller without ubbinding the conn object from radix framework. The framework then keeps dengling pointer.

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

@Sashan

Sashan commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

the issue has been triggered by UB-san in CI, details are here:
https://github.com/openssl/security/actions/runs/27146150766/job/80124350115

Comment thread test/radix/quic_ops.c
@t8m t8m added branch: master Applies to master branch 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 branch: 4.0 Applies to openssl-4.0 approval: review pending This pull request needs review by a committer and removed approval: review pending This pull request needs review by a committer labels Jun 9, 2026
@openssl-machine openssl-machine added the approval: review pending This pull request needs review by a committer label Jun 10, 2026
@Sashan
Sashan requested review from t8m and tom-cosgrove-arm June 10, 2026 23:08
@Sashan Sashan moved this to Waiting Review in Development Board Jun 11, 2026
Comment thread test/radix/quic_tests.c Outdated
Comment thread test/radix/quic_tests.c Outdated
Comment thread test/radix/quic_tests.c
@Sashan
Sashan requested a review from tom-cosgrove-arm June 11, 2026 12:54

@tom-cosgrove-arm tom-cosgrove-arm 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.

LGTM. 3 CI failures are because it doesn't cherry-pick cleanly to earlier branches

Comment thread test/radix/quic_tests.c Outdated
@t8m

t8m commented Jun 11, 2026

Copy link
Copy Markdown
Member

Also this needs a rebase.

Sashan added 4 commits June 12, 2026 08:45
The test for client_hello and new_pending connection should
be using its own dedicated SSL context. The thing is we should
not be arming (and testing) those callbacks for every listener
the RADIX test framework creates.

This changeset moves the test from test/radix/quic_ops.c
to test/radix/quic_tests.c. The test uses check_ctx_cbks
RADIX script now.
Comment thread test/crltest.c Outdated
@Sashan
Sashan requested a review from tom-cosgrove-arm June 12, 2026 08:20
@t8m

t8m commented Jun 12, 2026

Copy link
Copy Markdown
Member

ping @tom-cosgrove-arm

@tom-cosgrove-arm tom-cosgrove-arm 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.

LGTM - thanks

@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 Jun 12, 2026
@github-project-automation github-project-automation Bot moved this from Waiting Review to Waiting Merge in Development Board Jun 12, 2026
@openssl-machine openssl-machine 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 Jun 13, 2026
@openssl-machine

Copy link
Copy Markdown
Collaborator

This pull request is ready to merge

openssl-machine pushed a commit that referenced this pull request Jun 15, 2026
The test for client_hello and new_pending connection should
be using its own dedicated SSL context. The thing is we should
not be arming (and testing) those callbacks for every listener
the RADIX test framework creates.

This changeset moves the test from test/radix/quic_ops.c
to test/radix/quic_tests.c. The test uses check_ctx_cbks
RADIX script now.

Reviewed-by: Tom Cosgrove <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
MergeDate: Mon Jun 15 07:29:02 2026
(Merged from #31421)
openssl-machine pushed a commit that referenced this pull request Jun 15, 2026
The test for client_hello and new_pending connection should
be using its own dedicated SSL context. The thing is we should
not be arming (and testing) those callbacks for every listener
the RADIX test framework creates.

This changeset moves the test from test/radix/quic_ops.c
to test/radix/quic_tests.c. The test uses check_ctx_cbks
RADIX script now.

Reviewed-by: Tom Cosgrove <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
MergeDate: Mon Jun 15 07:29:02 2026
(Merged from #31421)

(cherry picked from commit 461b688)
openssl-machine pushed a commit that referenced this pull request Jun 15, 2026
The test for client_hello and new_pending connection should
be using its own dedicated SSL context. The thing is we should
not be arming (and testing) those callbacks for every listener
the RADIX test framework creates.

This changeset moves the test from test/radix/quic_ops.c
to test/radix/quic_tests.c. The test uses check_ctx_cbks
RADIX script now.

Reviewed-by: Tom Cosgrove <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
MergeDate: Mon Jun 15 07:29:02 2026
(Merged from #31421)

(cherry picked from commit 461b688)
@t8m

t8m commented Jun 15, 2026

Copy link
Copy Markdown
Member

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

@t8m t8m closed this Jun 15, 2026
@github-project-automation github-project-automation Bot moved this from Waiting Merge to Done in Development Board Jun 15, 2026
openssl-machine pushed a commit that referenced this pull request Jun 15, 2026
The test for client_hello and new_pending connection should
be using its own dedicated SSL context. The thing is we should
not be arming (and testing) those callbacks for every listener
the RADIX test framework creates.

This changeset moves the test from test/radix/quic_ops.c
to test/radix/quic_tests.c. The test uses check_ctx_cbks
RADIX script now.

Reviewed-by: Tom Cosgrove <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
MergeDate: Mon Jun 15 07:29:02 2026
(Merged from #31421)

(cherry picked from commit 461b688)
pr000000f added a commit to pr000000f/tongsuo-dev that referenced this pull request Jul 22, 2026
OpenSSL original commit message
From 11fb223813931724096c1b3f704c290b5231ef31 Mon Sep 17 00:00:00 2001
From: Alexandr Nedvedicky <[email protected]>
Date: Tue, 9 Jun 2026 10:58:52 +0200
Subject: [PATCH] Fix use-after-free issue in radix test framework for QUIC.
The test for client_hello and new_pending connection should
be using its own dedicated SSL context. The thing is we should
not be arming (and testing) those callbacks for every listener
the RADIX test framework creates.
This changeset moves the test from test/radix/quic_ops.c
to test/radix/quic_tests.c. The test uses check_ctx_cbks
RADIX script now.
Reviewed-by: Tom Cosgrove <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
MergeDate: Mon Jun 15 07:29:02 2026
(Merged from openssl/openssl#31421)
(cherry picked from commit 461b6886285afe174a2a7385541a1ac98c08e35f)
n13l pushed a commit to n13l/openssl that referenced this pull request Jul 25, 2026
The test for client_hello and new_pending connection should
be using its own dedicated SSL context. The thing is we should
not be arming (and testing) those callbacks for every listener
the RADIX test framework creates.

This changeset moves the test from test/radix/quic_ops.c
to test/radix/quic_tests.c. The test uses check_ctx_cbks
RADIX script now.

Reviewed-by: Tom Cosgrove <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
MergeDate: Mon Jun 15 07:29:02 2026
(Merged from openssl#31421)
rickyringler pushed a commit to rickyringler/openssl that referenced this pull request Aug 21, 2026
The test for client_hello and new_pending connection should
be using its own dedicated SSL context. The thing is we should
not be arming (and testing) those callbacks for every listener
the RADIX test framework creates.

This changeset moves the test from test/radix/quic_ops.c
to test/radix/quic_tests.c. The test uses check_ctx_cbks
RADIX script now.

Reviewed-by: Tom Cosgrove <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
MergeDate: Mon Jun 15 07:29:02 2026
(Merged from openssl#31421)
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

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants