From: Daniel Stenberg Date: Thu, 6 Aug 2026 07:13:30 +0000 (+0200) Subject: cookie: refuse to load cookies set against a PSL domain X-Git-Url: http://git.ipfire.org/gitweb/?a=commitdiff_plain;p=thirdparty%2Fcurl.git cookie: refuse to load cookies set against a PSL domain Verified by test 409 Reported-by: 1rhino2 on hackerone Closes #22500 --- diff --git a/docs/libcurl/opts/CURLOPT_COOKIELIST.md b/docs/libcurl/opts/CURLOPT_COOKIELIST.md index 9b9456ec12..e55699b498 100644 --- a/docs/libcurl/opts/CURLOPT_COOKIELIST.md +++ b/docs/libcurl/opts/CURLOPT_COOKIELIST.md @@ -47,6 +47,18 @@ future transfers to that server, likely not what you intended. To address these issues set a domain in `Set-Cookie` (doing that includes subdomains) or much better: use the Netscape file format. +Cookies added through this API bypass automatic Public Suffix List (PSL) +checking because the handle's internal PSL engine has not yet been initialized +when the call is made. Under normal transfer operations, PSL validation +prevents cookies from being set on broad or shared domains - such as `.com`, +`.co.uk`, or `.github.io` - which would otherwise create security +vulnerabilities by allowing unrelated subdomains to access sensitive cookie +data. Because the library skips this safety check during manual cookie +insertion, the caller assumes full responsibility for domain validation. +Applications using this interface must independently verify that the target +domain attribute represents a valid host and does not match a public suffix +before injecting the cookie into the handle. + Additionally, there are commands available that perform actions if you pass in these exact strings: @@ -64,7 +76,9 @@ writes all known cookies to the file specified by CURLOPT_COOKIEJAR(3) ## `RELOAD` -loads all cookies from the files specified by CURLOPT_COOKIEFILE(3) +loads all cookies from the files specified by CURLOPT_COOKIEFILE(3). If +CURLOPT_COOKIESESSION(3) is enabled before this reload, it is applied to this +load operation as well and all session cookies are discarded. # DEFAULT diff --git a/lib/cookie.c b/lib/cookie.c index 83baf435f1..c1a28bf8b1 100644 --- a/lib/cookie.c +++ b/lib/cookie.c @@ -803,17 +803,23 @@ static bool is_public_suffix(struct Curl_easy *data, { #ifdef USE_LIBPSL /* - * Check if the domain is a Public Suffix and if yes, ignore the cookie. We - * must also check that the data handle is not NULL since the psl code will - * dereference it. + * Check if the domain is a Public Suffix and if yes, ignore the cookie. + * 'domain' is NULL when the cookie is loaded from file or + * CURLOPT_COOKIELIST. */ + DEBUGASSERT(data); + DEBUGASSERT(co); DEBUGF(infof(data, "PSL check set-cookie '%s' for domain=%s in %s", - co->name, co->domain, domain)); - if(data && (domain && co->domain && !Curl_host_is_ipnum(co->domain))) { + co->name, co->domain ? co->domain : "[blank]", + domain ? domain : "[file]")); + if(!co->domain || Curl_host_is_ipnum(co->domain)) + return FALSE; + + else { bool acceptable = FALSE; char lcase[256]; char lcookie[256]; - size_t dlen = strlen(domain); + size_t dlen = domain ? strlen(domain) : 0; size_t clen = strlen(co->domain); /* trim trailing dots */ @@ -826,11 +832,17 @@ static bool is_public_suffix(struct Curl_easy *data, const psl_ctx_t *psl = Curl_psl_use(data); if(psl) { /* the PSL check requires lowercase domain name and pattern */ - Curl_strntolower(lcase, domain, dlen); - lcase[dlen] = 0; Curl_strntolower(lcookie, co->domain, clen); lcookie[clen] = 0; - acceptable = psl_is_cookie_domain_acceptable(psl, lcase, lcookie); + if(domain) { + Curl_strntolower(lcase, domain, dlen); + lcase[dlen] = 0; + acceptable = psl_is_cookie_domain_acceptable(psl, lcase, lcookie); + } + else + /* note that this PSL function returns the opposite value than + psl_is_cookie_domain_acceptable() does */ + acceptable = !psl_is_public_suffix(psl, lcookie); Curl_psl_release(data); } else @@ -839,7 +851,8 @@ static bool is_public_suffix(struct Curl_easy *data, if(!acceptable) { infof(data, "cookie '%s' dropped, domain '%s' must not " - "set cookies for '%s'", co->name, domain, co->domain); + "set cookies for '%s'", co->name, + domain ? domain : "[file]", co->domain); return TRUE; } } @@ -848,7 +861,7 @@ static bool is_public_suffix(struct Curl_easy *data, (void)co; (void)domain; DEBUGF(infof(data, "NO PSL to check set-cookie '%s' for domain=%s in %s", - co->name, co->domain, domain)); + co->name, co->domain, domain ? domain : "[file]")); #endif return FALSE; } @@ -968,16 +981,15 @@ static bool replace_existing(struct Curl_easy *data, * IPv6 address. * */ -CURLcode Curl_cookie_add( - struct Curl_easy *data, - struct CookieInfo *ci, - bool httpheader, /* TRUE if HTTP header-style line */ - bool noexpire, /* if TRUE, skip remove_expired() */ - const char *lineptr, /* first character of the line */ - const char *domain, /* default domain */ - 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 */ +CURLcode Curl_cookie_add(struct Curl_easy *data, + struct CookieInfo *ci, + const char *lineptr, /* first character of the line */ + const char *domain, /* default domain */ + const char *path, /* full path used when this + cookie is set, used to get + default path for the cookie + unless set */ + const int flags) { struct Cookie comem; struct Cookie *co; @@ -994,11 +1006,11 @@ CURLcode Curl_cookie_add( co = &comem; memset(co, 0, sizeof(comem)); - if(httpheader) + if(flags & COOKIE_HTTPHEADER) result = parse_cookie_header(data, co, ci, &okay, - lineptr, domain, path, secure); + lineptr, domain, path, flags & COOKIE_SECURE); else - result = parse_netscape(co, ci, &okay, lineptr, secure); + result = parse_netscape(co, ci, &okay, lineptr, flags & COOKIE_SECURE); if(result || !okay) goto fail; @@ -1026,20 +1038,19 @@ CURLcode Curl_cookie_add( co->livecookie = ci->running; co->creationtime = ++ci->lastct; - /* - * Now we have parsed the incoming line, we must now check if this supersedes - * an already existing cookie, which it may if the previous have the same - * domain and path as this. - */ - /* remove expired cookies */ - if(!noexpire) + if(!(flags & COOKIE_NOEXPIRE)) remove_expired(ci); - if(is_public_suffix(data, co, domain)) + if(!(flags & COOKIE_NOPSL) && is_public_suffix(data, co, domain)) goto fail; - if(!replace_existing(data, co, ci, secure, &replaces)) + /* + * Now we have parsed the incoming line, we must now check if this supersedes + * an already existing cookie, which it may if the previous have the same + * domain and path as this. + */ + if(!replace_existing(data, co, ci, flags & COOKIE_SECURE, &replaces)) goto fail; /* clone the stack struct into heap */ @@ -1071,7 +1082,7 @@ CURLcode Curl_cookie_add( if(co->expires && (co->expires < ci->next_expiration)) ci->next_expiration = co->expires; - if(httpheader) + if(flags & COOKIE_HTTPHEADER) data->req.setcookies++; return result; @@ -1120,11 +1131,11 @@ struct CookieInfo *Curl_cookie_init(void) * Reads cookies from a local file. This is always called before any cookies * are set. If file is "-" then STDIN is read. * - * If 'newsession' is TRUE, discard all "session cookies" on read from file. - * + * If 'flags' has the COOKIE_NOSESSION bit set, discard all "session cookies" + * read from file. */ static CURLcode cookie_load(struct Curl_easy *data, const char *file, - struct CookieInfo *ci, bool newsession) + struct CookieInfo *ci, int flags) { FILE *handle = NULL; CURLcode result = CURLE_OK; @@ -1133,7 +1144,7 @@ static CURLcode cookie_load(struct Curl_easy *data, const char *file, DEBUGASSERT(data); DEBUGASSERT(file); - ci->newsession = newsession; /* new session? */ + ci->newsession = !!(flags & COOKIE_NOSESSION); /* new session? */ ci->running = FALSE; /* this is not running, this is init */ if(file && *file) { @@ -1173,8 +1184,10 @@ static CURLcode cookie_load(struct Curl_easy *data, const char *file, curlx_str_passblanks(&lineptr); } - result = Curl_cookie_add(data, ci, headerline, TRUE, lineptr, NULL, - NULL, TRUE); + result = Curl_cookie_add(data, ci, lineptr, NULL, NULL, + (headerline ? COOKIE_HTTPHEADER : 0) | + COOKIE_NOEXPIRE | COOKIE_SECURE | + (flags & COOKIE_NOPSL)); /* File reading cookie failures are not propagated back to the caller because there is no way to do that */ } @@ -1199,7 +1212,8 @@ static CURLcode cookie_load(struct Curl_easy *data, const char *file, /* * Load cookies from all given cookie files (CURLOPT_COOKIEFILE). */ -CURLcode Curl_cookie_loadfiles(struct Curl_easy *data) +CURLcode Curl_cookie_loadfiles(struct Curl_easy *data, + int flags) { CURLcode result = CURLE_OK; struct curl_slist *list = data->state.cookielist; @@ -1212,8 +1226,7 @@ CURLcode Curl_cookie_loadfiles(struct Curl_easy *data) else { data->state.cookie_engine = TRUE; while(list) { - result = cookie_load(data, list->data, data->cookies, - (bool)data->set.cookiesession); + result = cookie_load(data, list->data, data->cookies, flags); if(result) break; list = list->next; diff --git a/lib/cookie.h b/lib/cookie.h index 286c47fd3a..973ec09547 100644 --- a/lib/cookie.h +++ b/lib/cookie.h @@ -104,20 +104,25 @@ struct CookieInfo { struct Curl_easy; struct connectdata; +bool Curl_secure_context(struct Curl_easy *data, const char *host); + /* * Add a cookie to the internal list of cookies. The domain and path arguments - * are only used if the header boolean is TRUE. + * are only used if the COOKIE_HTTPHEADER bit is set in the flags. */ -bool Curl_secure_context(struct Curl_easy *data, const char *host); +#define COOKIE_HTTPHEADER (1<<0) /* if HTTP header-style line */ +#define COOKIE_NOEXPIRE (1<<1) /* skip remove_expired() */ +#define COOKIE_SECURE (1<<2) /* connection is over secure origin */ +#define COOKIE_NOPSL (1<<3) /* skip PSL check */ +#define COOKIE_NOSESSION (1<<6) /* drop session cookies */ + CURLcode Curl_cookie_add(struct Curl_easy *data, struct CookieInfo *ci, - bool httpheader, - bool noexpire, const char *lineptr, const char *domain, const char *path, - bool secure) WARN_UNUSED_RESULT; + const int flags) WARN_UNUSED_RESULT; CURLcode Curl_cookie_getlist(struct Curl_easy *data, bool *okay, const char *host, struct Curl_llist *list) WARN_UNUSED_RESULT; @@ -126,7 +131,7 @@ void Curl_cookie_clearsess(struct CookieInfo *ci); #if defined(CURL_DISABLE_HTTP) || defined(CURL_DISABLE_COOKIES) #define Curl_cookie_list(x) NULL -#define Curl_cookie_loadfiles(x) CURLE_OK +#define Curl_cookie_loadfiles(x, y) CURLE_OK #define Curl_cookie_init() NULL #define Curl_cookie_run(x) Curl_nop_stmt #define Curl_cookie_cleanup(x) Curl_nop_stmt @@ -136,7 +141,8 @@ void Curl_flush_cookies(struct Curl_easy *data, bool cleanup); void Curl_cookie_cleanup(struct CookieInfo *ci); struct CookieInfo *Curl_cookie_init(void); struct curl_slist *Curl_cookie_list(struct Curl_easy *data); -CURLcode Curl_cookie_loadfiles(struct Curl_easy *data) WARN_UNUSED_RESULT; +CURLcode Curl_cookie_loadfiles(struct Curl_easy *data, + int flags) WARN_UNUSED_RESULT; void Curl_cookie_run(struct Curl_easy *data); #endif diff --git a/lib/http.c b/lib/http.c index 98d01c4305..d3cb5f72b5 100644 --- a/lib/http.c +++ b/lib/http.c @@ -3584,11 +3584,13 @@ static CURLcode http_header_s(struct Curl_easy *data, * real peer hostname. */ const char *host = data->req.cookiehost ? data->req.cookiehost : data->state.origin->hostname; - const bool secure_context = Curl_secure_context(data, host); + const unsigned char secure_context = Curl_secure_context(data, host) ? + COOKIE_SECURE : 0; CURLcode result; Curl_share_lock(data, CURL_LOCK_DATA_COOKIE, CURL_LOCK_ACCESS_SINGLE); - result = Curl_cookie_add(data, data->cookies, TRUE, FALSE, v, host, - data->state.up.path, secure_context); + result = Curl_cookie_add(data, data->cookies, v, host, + data->state.up.path, + COOKIE_HTTPHEADER | secure_context); Curl_share_unlock(data, CURL_LOCK_DATA_COOKIE); return result; } diff --git a/lib/setopt.c b/lib/setopt.c index 2e70c77b99..924263478b 100644 --- a/lib/setopt.c +++ b/lib/setopt.c @@ -1527,7 +1527,9 @@ static CURLcode cookielist(struct Curl_easy *data, const char *ptr) } else if(curl_strequal(ptr, "RELOAD")) { /* reload cookies from file */ - return Curl_cookie_loadfiles(data); + return Curl_cookie_loadfiles(data, COOKIE_NOPSL | + (data->set.cookiesession ? + COOKIE_NOSESSION : 0)); } else { if(!data->cookies) { @@ -1542,15 +1544,20 @@ static CURLcode cookielist(struct Curl_easy *data, const char *ptr) if(strlen(ptr) > CURL_MAX_INPUT_LENGTH) return CURLE_BAD_FUNCTION_ARGUMENT; + /* Adding these cookies without the PSL check, because the PSL is not + initialized until *perform() time, and this might be called before + that */ Curl_share_lock(data, CURL_LOCK_DATA_COOKIE, CURL_LOCK_ACCESS_SINGLE); if(checkprefix("Set-Cookie:", ptr)) /* HTTP Header format line */ - result = Curl_cookie_add(data, data->cookies, TRUE, FALSE, ptr + 11, - NULL, NULL, TRUE); + result = Curl_cookie_add(data, data->cookies, ptr + 11, + NULL, NULL, + COOKIE_HTTPHEADER | COOKIE_SECURE | + COOKIE_NOPSL); else /* Netscape format line */ - result = Curl_cookie_add(data, data->cookies, FALSE, FALSE, ptr, NULL, - NULL, TRUE); + result = Curl_cookie_add(data, data->cookies, ptr, NULL, + NULL, COOKIE_SECURE | COOKIE_NOPSL); Curl_share_unlock(data, CURL_LOCK_DATA_COOKIE); } return result; diff --git a/lib/transfer.c b/lib/transfer.c index 1834c6606e..673d79837d 100644 --- a/lib/transfer.c +++ b/lib/transfer.c @@ -525,7 +525,9 @@ CURLcode Curl_pretransfer(struct Curl_easy *data) data->state.infilesize = 0; /* If there is a list of cookie files to read, do it now! */ - result = Curl_cookie_loadfiles(data); + result = Curl_cookie_loadfiles(data, + data->set.cookiesession ? + COOKIE_NOSESSION : 0); if(!result) Curl_cookie_run(data); /* activate */ diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am index 76ac5be1bf..4993864a5a 100644 --- a/tests/data/Makefile.am +++ b/tests/data/Makefile.am @@ -70,7 +70,7 @@ test370 test371 test372 test373 test374 test375 test376 test378 test379 \ test380 test381 test383 test384 test385 test386 test387 test388 test389 \ test390 test391 test392 test393 test394 test395 test396 test397 test398 \ test399 test400 test401 test402 test403 test404 test405 test406 test407 \ -test408 test410 test411 test412 test413 test414 test415 test416 \ +test408 test409 test410 test411 test412 test413 test414 test415 test416 \ test417 test418 test419 test420 test421 test422 test423 test424 test425 \ test426 test427 test428 test429 test430 test431 test432 test433 test434 \ test435 test436 test437 test438 test439 test440 test441 test442 test443 \ diff --git a/tests/data/test409 b/tests/data/test409 new file mode 100644 index 0000000000..f9a7131ce2 --- /dev/null +++ b/tests/data/test409 @@ -0,0 +1,60 @@ + + + + +HTTP +cookies + + + +# Server-side + + +HTTP/1.1 200 OK +Date: Tue, 09 Nov 2010 14:49:00 GMT +Server: test-server/fake +Last-Modified: Tue, 13 Jun 2000 12:10:00 GMT +ETag: "21025-dc7-39462498" +Accept-Ranges: bytes +Content-Length: 6 +Connection: something-close, close-something, close +Content-Type: text/html +Funny-head: yesyes + +-foo- + + + +# Client-side + + +cookies +PSL + + +http + + +Load cookies from file where some are PSL domains + + +http://foo.co.uk:%HTTPPORT/%TESTNUMBER -b %LOGDIR/cookies.txt --resolve foo.co.uk:%HTTPPORT:%HOSTIP + + +.co.uk%TABTRUE%TAB/%TABFALSE%TAB0%TABbad%TABnot-allowed +foo.co.uk%TABTRUE%TAB/%TABFALSE%TAB0%TABgood%TABallowed + + + +# Verify data after the test has been "shot" + + +GET /%TESTNUMBER HTTP/1.1 +Host: foo.co.uk:%HTTPPORT +User-Agent: curl/%VERSION +Accept: */* +Cookie: good=allowed + + + +