curl: CVE-2023-28320: siglongjmp race condition

Summary:

If the system has no POSIX or Windows threading support, USE_ALARM_TIMEOUT codepath will be used in lib/hostip.c. If two threads will perform DNS resolving, a wrong register context can be used on the signal handlersiglongjmp call if DNS timeout occurs. Typically this results in segmentation fault, but depending on platform specifics other impacts might be possible (but unlikely).

The documentation warns against this very issue in https://curl.se/libcurl/c/threadsafe.html It is important that libcurl can find and use thread safe versions of these and other system calls, as otherwise it cannot function fully thread safe. The issue is that there is no way for the application using libcurl to know if the library is MT safe for DNS resolution or not. CURL_VERSION_THREADSAFE is mentioned, but this checks availability of atomic init, not MT safety of DNS resolution.

A remote attacker in a privileged network position is able to selectively block the DNS responses and may thus induce the affected target application to crash.

Steps To Reproduce:

1. For quick testing on POSIX systems add #define USE_ALARM_TIMEOUT to lib/hostip.c, for example:

diff --git a/lib/hostip.c b/lib/hostip.c
index 2381290fd..0148f2861 100644
--- a/lib/hostip.c
+++ b/lib/hostip.c
@@ -75,6 +75,7 @@
/* alarm-based timeouts can only be used with all the dependencies satisfied */
#define USE_ALARM_TIMEOUT
#endif
+#define USE_ALARM_TIMEOUT

#define MAX_HOSTCACHE_LEN (255 + 7) /* max FQDN + colon + port number + zero */

2. Compile libcurl
3. Compile version of https://curl.se/libcurl/c/multithread.html but add curl_easy_setopt(curl, CURLOPT_TIMEOUT, 2); to pull_one_url function.
4. Change DNS config to point to blackhole DNS server at 3.219.212.117 (blackhole.webpagetest.org)
5. Execute the compiled multithread and the application will segfault.

$ LD_LIBRARY_PATH=./lib/.libs:$LD_LIBRARY_PATH gdb ./multithread
GNU gdb (Debian 13.1-2) 13.1
Copyright (C) 2023 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<https://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from ./multithread...
(No debugging symbols found in ./multithread)
(gdb) r
Starting program: /home/user/curl/multithread
/home/user/curl/multithread: ./lib/.libs/libcurl.so.4: no version information available (required by /home/user/curl/multithread)
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
[New Thread 0x7ffff6ffc6c0 (LWP 2733684)]
Thread 0, gets http://curl.haxx.se/
[New Thread 0x7ffff67fb6c0 (LWP 2733685)]
Thread 1, gets ftp://cool.haxx.se/
[New Thread 0x7ffff5ffa6c0 (LWP 2733686)]
[New Thread 0x7ffff57f96c0 (LWP 2733687)]
Thread 2, gets http://www.contactor.se/
[New Thread 0x7ffff4ff86c0 (LWP 2733688)]
[New Thread 0x7fffe77fe6c0 (LWP 2733690)]
[New Thread 0x7fffe7fff6c0 (LWP 2733689)]
Thread 3, gets www.haxx.se
[New Thread 0x7fffe6ffd6c0 (LWP 2733691)]

Thread 1 "multithread" received signal SIGSEGV, Segmentation fault.
0x00007ffff7f42b32 in Curl_failf () from ./lib/.libs/libcurl.so.4
(gdb) bt
#0  0x00007ffff7f42b32 in Curl_failf () from ./lib/.libs/libcurl.so.4
#1  0x00007ffff7f546dd in Curl_resolv_timeout () from ./lib/.libs/libcurl.so.4
#2  0x0000000000000000 in ?? ()

Risk discussion

I don’t consider this issue a major risk since it likely will affect only small percentage of target platforms. Some rare windows configurations might be affected, as discussed in lib/curl_setup.h.

Remediation

Add an atomic locking to USE_ALARM_TIMEOUT code path to prevent multiple threads from using sigjmp_buf at the same time. The downside of this is that it makes DNS resolving serial, but at least it won’t crash. Here’s a crude proposal for this workaround:

diff --git a/lib/hostip.c b/lib/hostip.c
index 2381290fd..0c0c3b258 100644
--- a/lib/hostip.c
+++ b/lib/hostip.c
@@ -76,6 +76,10 @@
 #define USE_ALARM_TIMEOUT
 #endif
 
+#ifdef USE_ALARM_TIMEOUT
+#include "easy_lock.h"
+#endif
+
 #define MAX_HOSTCACHE_LEN (255 + 7) /* max FQDN + colon + port number + zero */
 
 /*
@@ -254,11 +258,14 @@ void Curl_hostcache_prune(struct Curl_easy *data)
     Curl_share_unlock(data, CURL_LOCK_DATA_DNS);
 }
 
-#ifdef HAVE_SIGSETJMP
+#ifdef USE_ALARM_TIMEOUT
 /* Beware this is a global and unique instance. This is used to store the
    return address that we can jump back to from inside a signal handler. This
    is not thread-safe stuff. */
 sigjmp_buf curl_jmpenv;
+#ifdef GLOBAL_INIT_IS_THREADSAFE
+curl_simple_lock curl_jmpenv_lock;
+#endif
 #endif
 
 /* lookup address, returns entry if found and not stale */
@@ -912,8 +919,15 @@ enum resolve_t Curl_resolv_timeout(struct Curl_easy *data,
      This should be the last thing we do before calling Curl_resolv(),
      as otherwise we'd have to worry about variables that get modified
      before we invoke Curl_resolv() (and thus use "volatile"). */
+#ifdef GLOBAL_INIT_IS_THREADSAFE
+  curl_simple_lock_lock(&curl_jmpenv_lock);
+#endif
+
   if(sigsetjmp(curl_jmpenv, 1)) {
     /* this is coming from a siglongjmp() after an alarm signal */
+#ifdef GLOBAL_INIT_IS_THREADSAFE
+    curl_simple_lock_unlock(&curl_jmpenv_lock);
+#endif
     failf(data, "name lookup timed out");
     rc = CURLRESOLV_ERROR;
     goto clean_up;
@@ -980,6 +994,10 @@ clean_up:
 #endif
 #endif /* HAVE_SIGACTION */
 
+#ifdef GLOBAL_INIT_IS_THREADSAFE
+  curl_simple_lock_unlock(&curl_jmpenv_lock);
+#endif
+
   /* switch back the alarm() to either zero or to what it was before minus
      the time we spent until now! */
   if(prev_alarm) {

This fix isn’t very optimal however as curl_simple_lock_lock is intended for short held locks. It maybe should also refuse to build if GLOBAL_INIT_IS_THREADSAFE is not available.

Another (easier) option would be to make the Curl_resolv_timeout USE_ALARM_TIMEOUT codepath fail if some other call of the function is in flight. This is quite crude, however.

Impact

Denial of service.