From 7a09b52c98ac8d840a8a9907b1a1d9a9e684bcf5 Mon Sep 17 00:00:00 2001 From: Daniel Gustafsson Date: Thu, 13 Dec 2018 09:57:58 +0100 Subject: cookies: leave secure cookies alone Only allow secure origins to be able to write cookies with the 'secure' flag set. This reduces the risk of non-secure origins to influence the state of secure origins. This implements IETF Internet-Draft draft-ietf-httpbis-cookie-alone-01 which updates RFC6265. Closes #2956 Reviewed-by: Daniel Stenberg --- lib/cookie.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++------- lib/cookie.h | 5 +++-- lib/http.c | 4 +++- lib/setopt.c | 4 ++-- 4 files changed, 56 insertions(+), 12 deletions(-) (limited to 'lib') diff --git a/lib/cookie.c b/lib/cookie.c index 3dc85ee5c..bc0ab0dfe 100644 --- a/lib/cookie.c +++ b/lib/cookie.c @@ -433,9 +433,10 @@ Curl_cookie_add(struct Curl_easy *data, bool noexpire, /* if TRUE, skip remove_expired() */ char *lineptr, /* first character of the line */ const char *domain, /* default domain */ - const char *path) /* full path used when this cookie is set, + const char *path, /* full path used when this cookie is set, used to get default path for the cookie unless set */ + bool secure) /* TRUE if connection is over secure origin */ { struct Cookie *clist; struct Cookie *co; @@ -546,8 +547,20 @@ Curl_cookie_add(struct Curl_easy *data, /* this was a "=" with no content, and we must allow 'secure' and 'httponly' specified this weirdly */ done = TRUE; - if(strcasecompare("secure", name)) - co->secure = TRUE; + /* + * secure cookies are only allowed to be set when the connection is + * using a secure protocol, or when the cookie is being set by + * reading from file + */ + if(strcasecompare("secure", name)) { + if(secure || !c->running) { + co->secure = TRUE; + } + else { + badcookie = TRUE; + break; + } + } else if(strcasecompare("httponly", name)) co->httponly = TRUE; else if(sep) @@ -831,7 +844,13 @@ Curl_cookie_add(struct Curl_easy *data, fields++; /* add a field and fall down to secure */ /* FALLTHROUGH */ case 3: - co->secure = strcasecompare(ptr, "TRUE")?TRUE:FALSE; + co->secure = FALSE; + if(strcasecompare(ptr, "TRUE")) { + if(secure || c->running) + co->secure = TRUE; + else + badcookie = TRUE; + } break; case 4: if(curlx_strtoofft(ptr, NULL, 10, &co->expires)) @@ -929,9 +948,31 @@ Curl_cookie_add(struct Curl_easy *data, /* the domains were identical */ if(clist->spath && co->spath) { - if(strcasecompare(clist->spath, co->spath)) { - replace_old = TRUE; + if(clist->secure && !co->secure) { + size_t cllen; + const char *sep; + + /* + * A non-secure cookie may not overlay an existing secure cookie. + * For an existing cookie "a" with path "/login", refuse a new + * cookie "a" with for example path "/login/en", while the path + * "/loginhelper" is ok. + */ + + sep = strchr(clist->spath + 1, '/'); + + if(sep) + cllen = sep - clist->spath; + else + cllen = strlen(clist->spath); + + if(strncasecompare(clist->spath, co->spath, cllen)) { + freecookie(co); + return NULL; + } } + else if(strcasecompare(clist->spath, co->spath)) + replace_old = TRUE; else replace_old = FALSE; } @@ -1103,7 +1144,7 @@ struct CookieInfo *Curl_cookie_init(struct Curl_easy *data, while(*lineptr && ISBLANK(*lineptr)) lineptr++; - Curl_cookie_add(data, c, headerline, TRUE, lineptr, NULL, NULL); + Curl_cookie_add(data, c, headerline, TRUE, lineptr, NULL, NULL, TRUE); } free(line); /* free the line buffer */ remove_expired(c); /* run this once, not on every cookie */ diff --git a/lib/cookie.h b/lib/cookie.h index a9f90ca71..3ee457c62 100644 --- a/lib/cookie.h +++ b/lib/cookie.h @@ -7,7 +7,7 @@ * | (__| |_| | _ <| |___ * \___|\___/|_| \_\_____| * - * Copyright (C) 1998 - 2017, Daniel Stenberg, , et al. + * Copyright (C) 1998 - 2018, Daniel Stenberg, , et al. * * This software is licensed as described in the file COPYING, which * you should have received as part of this distribution. The terms @@ -85,7 +85,8 @@ struct Curl_easy; struct Cookie *Curl_cookie_add(struct Curl_easy *data, struct CookieInfo *, bool header, bool noexpiry, char *lineptr, - const char *domain, const char *path); + const char *domain, const char *path, + bool secure); struct Cookie *Curl_cookie_getlist(struct CookieInfo *, const char *, const char *, bool); diff --git a/lib/http.c b/lib/http.c index 345100f6c..0a3e46243 100644 --- a/lib/http.c +++ b/lib/http.c @@ -3873,7 +3873,9 @@ CURLcode Curl_http_readwrite_headers(struct Curl_easy *data, here, or else use real peer host name. */ conn->allocptr.cookiehost? conn->allocptr.cookiehost:conn->host.name, - data->state.up.path); + data->state.up.path, + (conn->handler->protocol&CURLPROTO_HTTPS)? + TRUE:FALSE); Curl_share_unlock(data, CURL_LOCK_DATA_COOKIE); } #endif diff --git a/lib/setopt.c b/lib/setopt.c index 1627aba6d..01a890b39 100644 --- a/lib/setopt.c +++ b/lib/setopt.c @@ -803,12 +803,12 @@ CURLcode Curl_vsetopt(struct Curl_easy *data, CURLoption option, if(checkprefix("Set-Cookie:", argptr)) /* HTTP Header format line */ Curl_cookie_add(data, data->cookies, TRUE, FALSE, argptr + 11, NULL, - NULL); + NULL, TRUE); else /* Netscape format line */ Curl_cookie_add(data, data->cookies, FALSE, FALSE, argptr, NULL, - NULL); + NULL, TRUE); Curl_share_unlock(data, CURL_LOCK_DATA_COOKIE); free(argptr); -- cgit v1.2.3