From d9d57fe0da6f25d05570fd583520ecd321ed9c3f Mon Sep 17 00:00:00 2001

From: Daniel Stenberg <[email protected]>

Date: Tue, 4 Oct 2016 23:26:13 +0200

Subject: [PATCH] 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

---

 lib/cookie.c | 61 +++++++++++++++++++++++++++++++++++++++---------------------

 lib/cookie.h |  4 ++--

 lib/http.c   |  2 +-

 3 files changed, 43 insertions(+), 24 deletions(-)

--- lib/cookie.c

+++ lib/cookie.c

@@ -1022,10 +1022,44 @@ static int cookie_sort(const void *p1, const void *p2)

   /* sorry, can't be more deterministic */

   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()

  *

  * For a given host and path, return a linked list of cookies that the

@@ -1077,15 +1111,12 @@ struct Cookie *Curl_cookie_getlist(struct CookieInfo *c,

         if(!co->spath || pathmatch(co->spath, path) ) {

           /* 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;

             /* point the main to us */

             mainco = newco;

@@ -1093,16 +1124,11 @@ struct Cookie *Curl_cookie_getlist(struct CookieInfo *c,

             matches++;

           }

           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;

           }

         }

       }

     }

@@ -1150,11 +1176,11 @@ 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;

   }

 }

@@ -1162,25 +1188,18 @@ void Curl_cookie_clearall(struct CookieInfo *cookies)

  *

  * Curl_cookie_freelist()

  *

  * 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;

   }

 }

@@ -1231,11 +1250,11 @@ void Curl_cookie_clearsess(struct CookieInfo *cookies)

  ****************************************************************************/

 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 */

   }

 }

 /* get_netscape_format()

--- lib/cookie.h

+++ lib/cookie.h

@@ -5,11 +5,11 @@

  *  Project                     ___| | | |  _ \| |

  *                             / __| | | | |_) | |

  *                            | (__| |_| |  _ <| |___

  *                             \___|\___/|_| \_\_____|

  *

- * Copyright (C) 1998 - 2011, Daniel Stenberg, <[email protected]>, et al.

+ * Copyright (C) 1998 - 2016, Daniel Stenberg, <[email protected]>, et al.

  *

  * This software is licensed as described in the file COPYING, which

  * you should have received as part of this distribution. The terms

  * are also available at https://curl.haxx.se/docs/copyright.html.

  *

@@ -80,11 +80,11 @@ struct Cookie *Curl_cookie_add(struct Curl_easy *data,

                                struct CookieInfo *, bool header, char *lineptr,

                                const char *domain, const char *path);

 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);

 #if defined(CURL_DISABLE_HTTP) || defined(CURL_DISABLE_COOKIES)

 #define Curl_cookie_list(x) NULL

--- lib/http.c

+++ lib/http.c

@@ -2382,11 +2382,11 @@ CURLcode Curl_http(struct connectdata *conn, bool *done)

             break;

           count++;

         }

         co = co->next; /* next cookie please */

       }

-      Curl_cookie_freelist(store, FALSE); /* free the cookie list */

+      Curl_cookie_freelist(store);

     }

     if(addcookies && !result) {

       if(!count)

         result = Curl_add_bufferf(req_buffer, "Cookie: ");

       if(!result) {

-- 

2.9.3