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

Crash when configuring with DNS over HTTPS, but without threading or ares #16645

Closed
larskarlitski opened this issue Mar 10, 2025 · 6 comments
Closed
Assignees
Labels

Comments

@larskarlitski
Copy link

I did this

I have curl compiled with DNS over HTTPS, but without threads or ares. It crashes while connecting to a name that it found in its DNS cache.

I believe this is due to the easy handle's conn->dns_entry field being NULL. Curl_once_resolved sets it from state.async.dns, but only if CURL_USE_ASYNC is defined (lib/hostip.c:1439):

#ifdef USE_CURL_ASYNC
  if(data->state.async.dns) {
    conn->dns_entry = data->state.async.dns;
    data->state.async.dns = NULL;
  }
#endif

However, state_resolving only sets state.async.dns when CURLRES_ASYNCH is defined (lib/multi.c:2090):

#ifdef CURLRES_ASYNCH
    data->state.async.dns = dns;
    data->state.async.done = TRUE;
#endif

In my configuration, CURLRES_ASYNCH is not set, but USE_CURL_ASYNC is set. Thus, conn->dns_entry stays NULL and curl crashes accessing this field while connecting (in cf_he_connect:973).

lib/urldata.h:566 defines USE_CURL_ASYNC when CURLRES_ASYNCH is defined or when CURL_DISABLE_DOH is not defined.

I don't know curl well enough to suggest a fix, but changing the #ifdef CURLRES_ASYNCH in multi.c to #ifdef USE_CURL_ASYNC works and seems correct from my understanding.

I expected the following

No response

curl/libcurl version

I have seen this with curl 8.12.1 and with latest main as of now (f8e7a4d).

operating system

Linux fedora 6.13.5-200.fc41.aarch64 #1 SMP PREEMPT_DYNAMIC Thu Feb 27 15:30:52 UTC 2025 aarch64 GNU/Linux

@bagder bagder self-assigned this Mar 10, 2025
@bagder bagder added name lookup DNS and related tech crash regression labels Mar 10, 2025
@bagder
Copy link
Member

bagder commented Mar 10, 2025

What does curl -V show you?

How do you reproduce this crash? I built master with the blocking resolver just now and this works fine:

curl --doh-url https://cloudflare-dns.com/dns-query curl.se -v

@bagder
Copy link
Member

bagder commented Mar 10, 2025

Ah right, you said it needs to find the name in the cache. I need a better command line.

@bagder
Copy link
Member

bagder commented Mar 10, 2025

This works for me, getting the name from the cache in the second transfer:

curl --doh-url https://cloudflare-dns.com/dns-query curl.se curl.se --http1.0 -vv

@larskarlitski
Copy link
Author

Sorry – I should've mentioned that I am using libcurl. Cannot share the code at the moment and did not find time to write a minimal reproducer.

When running with CURL_OPT_VERBOSE, the last message before the crash is Hostname XXX was found in DNS cache.

bagder added a commit that referenced this issue Mar 10, 2025
CURLRES_ASYNCH - is for when built to use an async name resolver; threaded or
c-ares

USE_CURL_ASYNC - is for when built to use either an async name resolver OR DoH

Reported-by: Lars Karlitski
Fixes #16645
@bagder
Copy link
Member

bagder commented Mar 10, 2025

I could not reproduce, but by reading code I made #16648

(I suspect there might be some other issues still lurking when using the odd combo of blocking resolver + DoH.)

@larskarlitski
Copy link
Author

Works like a charm. Thank you for the swift fix!

@bagder bagder closed this as completed in 3c2948e Mar 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging a pull request may close this issue.

2 participants