From: David Zhuang Date: Wed, 3 Sep 2025 00:28:21 +0000 (-0700) Subject: http: do the cookie list access under lock X-Git-Tag: curl-8_16_0~38 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=c278c508e2acfad5f2603febfdba80a30427cd3f;p=thirdparty%2Fcurl.git http: do the cookie list access under lock A previous refactor of cookie logic changed Curl_cookie_getlist to no longer return a list of copied cookies, but instead return a linked list pointing to existing cookies. The returned linked list is accessed outside of the scope of the cookie share lock in http_cookies, which leads to issues if the shared cookie list is modified at the same time. This is the relevant commit: be39ed1 Closes #18457 --- diff --git a/lib/http.c b/lib/http.c index bcbe6e9e75..d62f3aff4a 100644 --- a/lib/http.c +++ b/lib/http.c @@ -2410,45 +2410,43 @@ static CURLcode http_cookies(struct Curl_easy *data, if(data->cookies || addcookies) { struct Curl_llist list; int count = 0; - int rc = 1; if(data->cookies && data->state.cookie_engine) { const char *host = data->state.aptr.cookiehost ? data->state.aptr.cookiehost : data->conn->host.name; Curl_share_lock(data, CURL_LOCK_DATA_COOKIE, CURL_LOCK_ACCESS_SINGLE); - rc = Curl_cookie_getlist(data, data->conn, host, &list); - Curl_share_unlock(data, CURL_LOCK_DATA_COOKIE); - } - if(!rc) { - struct Curl_llist_node *n; - size_t clen = 8; /* hold the size of the generated Cookie: header */ - - /* loop through all cookies that matched */ - for(n = Curl_llist_head(&list); n; n = Curl_node_next(n)) { - struct Cookie *co = Curl_node_elem(n); - if(co->value) { - size_t add; - if(!count) { - result = curlx_dyn_addn(r, STRCONST("Cookie: ")); + if(!Curl_cookie_getlist(data, data->conn, host, &list)) { + struct Curl_llist_node *n; + size_t clen = 8; /* hold the size of the generated Cookie: header */ + + /* loop through all cookies that matched */ + for(n = Curl_llist_head(&list); n; n = Curl_node_next(n)) { + struct Cookie *co = Curl_node_elem(n); + if(co->value) { + size_t add; + if(!count) { + result = curlx_dyn_addn(r, STRCONST("Cookie: ")); + if(result) + break; + } + add = strlen(co->name) + strlen(co->value) + 1; + if(clen + add >= MAX_COOKIE_HEADER_LEN) { + infof(data, "Restricted outgoing cookies due to header size, " + "'%s' not sent", co->name); + linecap = TRUE; + break; + } + result = curlx_dyn_addf(r, "%s%s=%s", count ? "; " : "", + co->name, co->value); if(result) break; + clen += add + (count ? 2 : 0); + count++; } - add = strlen(co->name) + strlen(co->value) + 1; - if(clen + add >= MAX_COOKIE_HEADER_LEN) { - infof(data, "Restricted outgoing cookies due to header size, " - "'%s' not sent", co->name); - linecap = TRUE; - break; - } - result = curlx_dyn_addf(r, "%s%s=%s", count ? "; " : "", - co->name, co->value); - if(result) - break; - clen += add + (count ? 2 : 0); - count++; } + Curl_llist_destroy(&list, NULL); } - Curl_llist_destroy(&list, NULL); + Curl_share_unlock(data, CURL_LOCK_DATA_COOKIE); } if(addcookies && !result && !linecap) { if(!count)