How it was found

Use-after-free in curl_easy_reset() with HTTP/2 stream-dependency tree

Hi all,

We've found an issue in lib/easy.c where curl_easy_reset() bypasses data_priority_cleanup() before clearing data->set, leaving the HTTP/2 stream-dependency tree with dangling pointers to the reset handle.

The current curl_easy_reset() at lib/easy.c:1090-1098 does:

Curl_freeset(data);
memset(&data->set, 0, sizeof(struct UserDefined));
Curl_init_userdefined(data);

This zeroes data->set.priority.{parent,children} without first walking the dependency tree to remove this handle from its peers' children lists. After a subsequent curl_easy_cleanup(), peers still reference the freed handle through their priority.children list and dereference it from data_priority_cleanup → priority_remove_child (lib/url.c:3094, 3022).

The defect dates to commit 71b7e01610 (2022-12-30, "lib: connect/h2/h3 refactor"), which converted set.priority into a linked tree and introduced Curl_data_priority_cleanup() in lib/url.c, but did not add a call to it from curl_easy_reset(). Reachable through documented public APIs only, on any USE_NGHTTP2 build.

Reproducer

Build curl with ASan:

autoreconf -fi
mkdir build && cd build
CFLAGS='-O1 -g -fsanitize=address' LDFLAGS='-fsanitize=address' CC=clang \
  ../configure --with-openssl --enable-debug --enable-maintainer-mode --disable-shared
make -j

Save as poc.c:

#include <curl/curl.h>
int main(void) {
    curl_global_init(CURL_GLOBAL_DEFAULT);
    CURL *A = curl_easy_init();
    CURL *B = curl_easy_init();
    curl_easy_setopt(B, CURLOPT_STREAM_DEPENDS, A);
    curl_easy_reset(B);
    curl_easy_cleanup(B);
    curl_easy_cleanup(A);
    curl_global_cleanup();
    return 0;
}

Compile and run:

clang -fsanitize=address -g -I../include poc.c \
  ./lib/.libs/libcurl.a -lssl -lcrypto -lz -lnghttp2 -lidn2 -lzstd \
  -lbrotlidec -lldap -llber -o poc
./poc

Observed output

=================================================================
==654355==ERROR: AddressSanitizer: heap-use-after-free on address 0x7ac36ebe2148 at pc 0x614bec3b354f bp 0x7fff212fbea0 sp 0x7fff212fbe98
READ of size 8 at 0x7ac36ebe2148 thread T0
    #0 priority_remove_child   lib/url.c:3022:3
    #1 data_priority_cleanup   lib/url.c:3094:5
    #2 Curl_close              lib/url.c:278:3
    #3 curl_easy_cleanup       lib/easy.c:844:5
    #4 main                    poc.c

0x7ac36ebe2148 is located 2120 bytes inside of 5096-byte region [0x7ac36ebe1900,0x7ac36ebe2ce8)
freed by thread T0 here:
    #0 free
    #1 Curl_close              lib/url.c:311:3
    #2 curl_easy_cleanup       lib/easy.c:844:5
    #3 main                    poc.c  (curl_easy_cleanup(B))

previously allocated by thread T0 here:
    #0 calloc
    #1 curl_dbg_calloc         lib/memdebug.c:254:9
    #2 Curl_open               lib/url.c:463:10
    #3 curl_easy_init          lib/easy.c:350:12
    #4 main                    poc.c

SUMMARY: AddressSanitizer: heap-use-after-free lib/url.c:3022:3 in priority_remove_child

The 5096-byte region freed by the cleanup of B is the entire struct Curl_easy. The subsequent cleanup of A then walks A->set.priority.children, finds the stale node whose ->data points into the freed region, and reads through it at lib/url.c:3022 and writes through it at lib/url.c:3034-3035.

Suggested fix

Expose the existing static data_priority_cleanup helper and call it from curl_easy_reset, the same way Curl_close already does:

--- a/lib/url.c
+++ b/lib/url.c
@@ -120,9 +120,9 @@
 #ifdef USE_NGHTTP2
-static void data_priority_cleanup(struct Curl_easy *data);
+void Curl_data_priority_cleanup(struct Curl_easy *data);
 #else
-#define data_priority_cleanup(x)
+#define Curl_data_priority_cleanup(x)
 #endif
@@ -275,7 +275,7 @@
   /* destroy data->state.priority for HTTP/2 dep tree */
-  data_priority_cleanup(data);
+  Curl_data_priority_cleanup(data);
@@ -3087,7 +3087,7 @@
-static void data_priority_cleanup(struct Curl_easy *data)
+void Curl_data_priority_cleanup(struct Curl_easy *data)

--- a/lib/easy.c
+++ b/lib/easy.c
@@ -1093,6 +1093,7 @@ void curl_easy_reset(CURL *curl)
   /* clear all meta data */
   Curl_meta_reset(data);
+  Curl_data_priority_cleanup(data);
   /* zero out UserDefined data: */
   Curl_freeset(data);
   memset(&data->set, 0, sizeof(struct UserDefined));

Cheers, AISLE Research Team