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 bcbe6e9e7526bc288d9ae13c2912e699d335fca4..d62f3aff4a8f7c91c5071e7e9ff6110c5fa248b3 100644 (file)
--- 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)