cookies: getlist() now holds deep copies of all cookies

Previously it only held references to them, which was reckless as the
thread lock was released so the cookies could get modified by other
handles that share the same cookie jar over the share interface.

CVE-2016-8623

Bug: https://curl.haxx.se/docs/adv_20161102I.html
Reported-by: Cure53

diff --git a/lib/cookie.c b/lib/cookie.c
index 0f05da2000b3d48bf12b2661572692e735492f2c..8607ce3e765c772d30bfc7a908175835e0c8da7e 100644 (file)
--- a/lib/cookie.c
+++ b/lib/cookie.c
@@ -1024,6 +1024,40 @@ static int cookie_sort(const void *p1, const void *p2)
   return 0;
 }
 
+#define CLONE(field)                     \
+  do {                                   \
+    if(src->field) {                     \
+      dup->field = strdup(src->field);   \
+      if(!dup->field)                    \
+        goto fail;                       \
+    }                                    \
+  } while(0)
+
+static struct Cookie *dup_cookie(struct Cookie *src)
+{
+  struct Cookie *dup = calloc(sizeof(struct Cookie), 1);
+  if(dup) {
+    CLONE(expirestr);
+    CLONE(domain);
+    CLONE(path);
+    CLONE(spath);
+    CLONE(name);
+    CLONE(value);
+    CLONE(maxage);
+    CLONE(version);
+    dup->expires = src->expires;
+    dup->tailmatch = src->tailmatch;
+    dup->secure = src->secure;
+    dup->livecookie = src->livecookie;
+    dup->httponly = src->httponly;
+  }
+  return dup;
+
+  fail:
+  freecookie(dup);
+  return NULL;
+}
+
 /*****************************************************************************
  *
  * Curl_cookie_getlist()
@@ -1079,11 +1113,8 @@ struct Cookie *Curl_cookie_getlist(struct CookieInfo *c,
           /* and now, we know this is a match and we should create an
              entry for the return-linked-list */
 
-          newco = malloc(sizeof(struct Cookie));
+          newco = dup_cookie(co);
           if(newco) {
-            /* first, copy the whole source cookie: */
-            memcpy(newco, co, sizeof(struct Cookie));
-
             /* then modify our next */
             newco->next = mainco;
 
@@ -1095,12 +1126,7 @@ struct Cookie *Curl_cookie_getlist(struct CookieInfo *c,
           else {
             fail:
             /* failure, clear up the allocated chain and return NULL */
-            while(mainco) {
-              co = mainco->next;
-              free(mainco);
-              mainco = co;
-            }
-
+            Curl_cookie_freelist(mainco);
             return NULL;
           }
         }
@@ -1152,7 +1178,7 @@ struct Cookie *Curl_cookie_getlist(struct CookieInfo *c,
 void Curl_cookie_clearall(struct CookieInfo *cookies)
 {
   if(cookies) {
-    Curl_cookie_freelist(cookies->cookies, TRUE);
+    Curl_cookie_freelist(cookies->cookies);
     cookies->cookies = NULL;
     cookies->numcookies = 0;
   }
@@ -1164,21 +1190,14 @@ void Curl_cookie_clearall(struct CookieInfo *cookies)
  *
  * Free a list of cookies previously returned by Curl_cookie_getlist();
  *
- * The 'cookiestoo' argument tells this function whether to just free the
- * list or actually also free all cookies within the list as well.
- *
  ****************************************************************************/
 
-void Curl_cookie_freelist(struct Cookie *co, bool cookiestoo)
+void Curl_cookie_freelist(struct Cookie *co)
 {
   struct Cookie *next;
   while(co) {
     next = co->next;
-    if(cookiestoo)
-      freecookie(co);
-    else
-      free(co); /* we only free the struct since the "members" are all just
-                   pointed out in the main cookie list! */
+    freecookie(co);
     co = next;
   }
 }
@@ -1233,7 +1252,7 @@ void Curl_cookie_cleanup(struct CookieInfo *c)
 {
   if(c) {
     free(c->filename);
-    Curl_cookie_freelist(c->cookies, TRUE);
+    Curl_cookie_freelist(c->cookies);
     free(c); /* free the base struct as well */
   }
 }

diff --git a/lib/cookie.h b/lib/cookie.h
index cd7c54a28cd0a89c31b42bc59c73af9ebff826d4..a9a45785c692261c9da6ccd2daf7cf0251e07bec 100644 (file)
--- a/lib/cookie.h
+++ b/lib/cookie.h
@@ -7,7 +7,7 @@
  *                            | (__| |_| |  _ <| |___
  *                             \___|\___/|_| \_\_____|
  *
- * Copyright (C) 1998 - 2011, Daniel Stenberg, <daniel@haxx.se>, et al.
+ * Copyright (C) 1998 - 2016, Daniel Stenberg, <daniel@haxx.se>, et al.
  *
  * This software is licensed as described in the file COPYING, which
  * you should have received as part of this distribution. The terms
@@ -82,7 +82,7 @@ struct Cookie *Curl_cookie_add(struct Curl_easy *data,
 
 struct Cookie *Curl_cookie_getlist(struct CookieInfo *, const char *,
                                    const char *, bool);
-void Curl_cookie_freelist(struct Cookie *cookies, bool cookiestoo);
+void Curl_cookie_freelist(struct Cookie *cookies);
 void Curl_cookie_clearall(struct CookieInfo *cookies);
 void Curl_cookie_clearsess(struct CookieInfo *cookies);
 

diff --git a/lib/http.c b/lib/http.c
index 65c145a13fb16862e7ebc3e86b5c2c79c36761f7..e6e7d377a4b9b865d6ea0f4bf4ede39b6afe4ab2 100644 (file)
--- a/lib/http.c
+++ b/lib/http.c
@@ -2384,7 +2384,7 @@ CURLcode Curl_http(struct connectdata *conn, bool *done)
         }
         co = co->next; /* next cookie please */
       }
-      Curl_cookie_freelist(store, FALSE); /* free the cookie list */
+      Curl_cookie_freelist(store);
     }
     if(addcookies && !result) {
       if(!count)