Skip to content
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.

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

connection bundle use-after-free in Curl_cpool_check_limits when CURLMOPT_MAX_HOST_CONNECTIONS=1 #15185

Closed
syntax-case opened this issue Oct 8, 2024 · 0 comments
Assignees

Comments

@syntax-case
Copy link

syntax-case commented Oct 8, 2024

I did this

Summary:

In Curl_cpool_check_limits, there's a loop (conncache.c:319) that removes connections from a bundle until the number of connections is less than the configured maximum.
However, if the maximum is 1, the loop will remove all connections, causing the bundle to be deleted in the call to Curl_cpool_disconnect (conncache.c:331).
This leaves the pointer in Curl_cpool_check_limits dangling, and when the loop condition is checked the freed bundle is accessed.

Steps To Reproduce:

We first noticed this in our valgrind testruns.
A simple way to reproduce it is to hardcode the dest_limit to 1 in Curl_cpool_check_limits, and run a test that exercises this, for example 2032.
It should fail with valgrind warnings about invalid reads.
The test stderr should show: * Discarding Connection #0 from 1 to reach destination limit of 1

I expected the following

No response

curl/libcurl version

commit d9a9233

operating system

Linux *** 6.8.0-45-generic #45-Ubuntu SMP PREEMPT_DYNAMIC Fri Aug 30 12:02:04 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux

@bagder bagder self-assigned this Oct 8, 2024
bagder added a commit that referenced this issue Oct 8, 2024
When the pool is cleaned up due to host limits, the bundle may be
cleaned up as well making the old pointer invalid.

Fixes #15185
Reported-by: Moritz Knüsel
bagder added a commit that referenced this issue Oct 8, 2024
When the pool is cleaned up due to host limits, the bundle may be
cleaned up as well making the old pointer invalid.

Fixes #15185
Reported-by: Moritz Knüsel
@bagder bagder closed this as completed in 699a2df Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants