Resolves: CVE-2016-8623 - fix use-after-free via shared cookies

This commit is contained in:
Kamil Dudka 2016-11-02 16:56:45 +01:00
parent 8cc82f17a1
commit 6e32112b9a
2 changed files with 179 additions and 0 deletions

View File

@ -0,0 +1,174 @@
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

View File

@ -31,6 +31,9 @@ Patch12: 0012-curl-7.47.1-CVE-2016-7167.patch
# urlparse: accept '#' as end of host name (CVE-2016-8624)
Patch13: 0013-curl-7.47.1-CVE-2016-8624.patch
# fix use-after-free via shared cookies (CVE-2016-8623)
Patch14: 0014-curl-7.47.1-CVE-2016-8623.patch
# patch making libcurl multilib ready
Patch101: 0101-curl-7.32.0-multilib.patch
@ -152,6 +155,7 @@ documentation of the library, too.
%patch11 -p1
%patch12 -p1
%patch13 -p1
%patch14 -p1
# Fedora patches
%patch101 -p1
@ -268,6 +272,7 @@ rm -rf $RPM_BUILD_ROOT
%changelog
* Wed Nov 02 2016 Kamil Dudka <kdudka@redhat.com> 7.47.1-9
- fix use-after-free via shared cookies (CVE-2016-8623)
- urlparse: accept '#' as end of host name (CVE-2016-8624)
- run autoreconf in %%prep to avoid patching Makefile.in files from now on