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

Setting CURLOPT_HSTSREADFUNCTION without setting CURLOPT_HSTS_CTRL causes a assertion failure during runtime #7710

Closed
JCMais opened this issue Sep 12, 2021 · 2 comments
Assignees

Comments

@JCMais
Copy link
Contributor

JCMais commented Sep 12, 2021

I did this

Created a new easy handle and set the options CURLOPT_URL and CURLOPT_HSTSREADFUNCTION.

Calling curl_easy_perform on that easy handle triggers the following assertion failure:

node: hsts.c:431: hsts_pull: Assertion `h' failed.

I expected the following

I found this when adding support to the HSTS options on node-libcurl and adding some tests.

I expected the CURLOPT_HSTSREADFUNCTION callback to not be called when CURLOPT_HSTS_CTRL is not set, as the docs say that just setting the option is not going to enable HSTS support, but it seems that libcurl still tries to call function anyway.

The assertion is coming from this line:

curl/lib/hsts.c

Line 431 in 1c1d9f1

DEBUGASSERT(h);

hsts_pull is called by Curl_hsts_loadcb during Curl_pretransfer, but it is only checking if data->set.hsts_read is set, however, Curl_easy->hsts is only initialized when CURLOPT_HSTS_CTRL is set:

curl/lib/setopt.c

Lines 2972 to 2983 in de1004e

case CURLOPT_HSTS_CTRL:
arg = va_arg(param, long);
if(arg & CURLHSTS_ENABLE) {
if(!data->hsts) {
data->hsts = Curl_hsts_init();
if(!data->hsts)
return CURLE_OUT_OF_MEMORY;
}
}
else
Curl_hsts_cleanup(&data->hsts);
break;

Not setting it causes the assertion failure inside hsts_pull, as h points to non-initialized memory.

The Curl_hsts_save function handles this in a different way, if h is not set, it just returns:

curl/lib/hsts.c

Lines 331 to 333 in 1c1d9f1

if(!h)
/* no cache activated */
return CURLE_OK;

So I suppose CURLOPT_HSTSWRITEFUNCTION does not have this issue, but I have not tested it yet.

curl/libcurl version

curl 7.78.0 (x86_64-pc-linux-gnu) libcurl/7.78.0 OpenSSL/1.1.1k zlib/1.2.11 brotli/1.0.9 zstd/1.4.9 libidn2/2.1.1 libssh2/1.9.0 nghttp2/1.41.0 OpenLDAP/2.4.47
Release-Date: 2021-07-21
Protocols: dict file ftp ftps gopher gophers http https imap imaps ldap ldaps mqtt pop3 pop3s rtsp scp sftp smb smbs smtp smtps telnet tftp 
Features: alt-svc AsynchDNS brotli Debug HSTS HTTP2 HTTPS-proxy IDN IPv6 Largefile libz NTLM NTLM_WB SSL TLS-SRP TrackMemory UnixSockets zstd

operating system

Linux JCM-PC 4.19.128-microsoft-standard #1 SMP Tue Jun 23 12:58:10 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux

(Ubuntu running under WSL2 on Windows)

@bagder
Copy link
Member

bagder commented Sep 12, 2021

Confirmed. I think it can be as easy as #7711 ?

@bagder bagder self-assigned this Sep 12, 2021
@JCMais
Copy link
Contributor Author

JCMais commented Sep 12, 2021

yep, I just tested your patch and it is working. Thanks!

@bagder bagder closed this as completed in 8822ecf Sep 12, 2021
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