175 lines
5.5 KiB
Diff
175 lines
5.5 KiB
Diff
|
From 86936c59e634a4662912a3b40bcc01d214249e55 Mon Sep 17 00:00:00 2001
|
||
|
From: Daniel Stenberg <daniel@haxx.se>
|
||
|
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
|
||
|
|
||
|
Upstream-commit: c5be3d7267c725dbd093ff3a883e07ee8cf2a1d5
|
||
|
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
|
||
|
---
|
||
|
lib/cookie.c | 61 +++++++++++++++++++++++++++++++++++++++---------------------
|
||
|
lib/cookie.h | 4 ++--
|
||
|
lib/http.c | 2 +-
|
||
|
3 files changed, 43 insertions(+), 24 deletions(-)
|
||
|
|
||
|
diff --git a/lib/cookie.c b/lib/cookie.c
|
||
|
index e5c7b7e..7a60411 100644
|
||
|
--- a/lib/cookie.c
|
||
|
+++ b/lib/cookie.c
|
||
|
@@ -1013,6 +1013,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()
|
||
|
@@ -1068,11 +1102,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;
|
||
|
|
||
|
@@ -1084,12 +1115,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;
|
||
|
}
|
||
|
}
|
||
|
@@ -1141,7 +1167,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;
|
||
|
}
|
||
|
@@ -1153,21 +1179,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;
|
||
|
}
|
||
|
}
|
||
|
@@ -1222,7 +1241,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 74a9224..ecfece2 100644
|
||
|
--- 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 SessionHandle *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 53c6cf8..f9a82c8 100644
|
||
|
--- 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)
|
||
|
--
|
||
|
2.7.4
|
||
|
|