From: Daniel Stenberg Date: Sun, 7 Dec 2025 22:44:31 +0000 (+0100) Subject: cookie: cleanups and improvements X-Git-Tag: rc-8_18_0-2~104 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a78a07d3a9dc808a51f464d0dd297939e544e2a4;p=thirdparty%2Fcurl.git cookie: cleanups and improvements - Stricter cookie validation with earlier rejection of empty/invalid cookie names - secure and httponly attributes no longer accept = with empty values (only bare keywords) - Validation checks (length, TAB, prefixes) moved into the first name/value pair block for better code organization - Deferred time(NULL) calls for better performance when expires/max-age aren't used - Simplified loop control flow by removing done flag - The cookie size restriction now only applies to name + value, not other parts of the header line. - Fixed a gcc 4.8.1 quirk Closes #19868 --- diff --git a/lib/cookie.c b/lib/cookie.c index 7efce5611f..716a487750 100644 --- a/lib/cookie.c +++ b/lib/cookie.c @@ -441,85 +441,79 @@ parse_cookie_header(struct Curl_easy *data, origin */ { /* This line was read off an HTTP-header */ - time_t now; + time_t now = 0; size_t linelength = strlen(ptr); - CURLcode result; - struct Curl_str cookie[COOKIE_PIECES] = { 0 }; + CURLcode result = CURLE_OK; + struct Curl_str cookie[COOKIE_PIECES]; *okay = FALSE; if(linelength > MAX_COOKIE_LINE) /* discard overly long lines at once */ return CURLE_OK; - now = time(NULL); + /* memset instead of initializer because gcc 4.8.1 is silly */ + memset(cookie, 0, sizeof(cookie)); do { struct Curl_str name; struct Curl_str val; /* we have a = pair or a stand-alone word here */ if(!curlx_str_cspn(&ptr, &name, ";\t\r\n=")) { - bool done = FALSE; bool sep = FALSE; curlx_str_trimblanks(&name); if(!curlx_str_single(&ptr, '=')) { sep = TRUE; /* a '=' was used */ - if(!curlx_str_cspn(&ptr, &val, ";\r\n")) { + if(!curlx_str_cspn(&ptr, &val, ";\r\n")) curlx_str_trimblanks(&val); - - /* Reject cookies with a TAB inside the value */ - if(memchr(curlx_str(&val), '\t', curlx_strlen(&val))) { - infof(data, "cookie contains TAB, dropping"); - return CURLE_OK; - } - } } - else { + else curlx_str_init(&val); - } - - /* - * Check for too long individual name or contents, or too long - * combination of name + contents. Chrome and Firefox support 4095 or - * 4096 bytes combo - */ - if(curlx_strlen(&name) >= (MAX_NAME - 1) || - curlx_strlen(&val) >= (MAX_NAME - 1) || - ((curlx_strlen(&name) + curlx_strlen(&val)) > MAX_NAME)) { - infof(data, "oversized cookie dropped, name/val %zu + %zu bytes", - curlx_strlen(&name), curlx_strlen(&val)); - return CURLE_OK; - } - - /* - * Check if we have a reserved prefix set before anything else, as we - * otherwise have to test for the prefix in both the cookie name and - * "the rest". Prefixes must start with '__' and end with a '-', so - * only test for names where that can possibly be true. - */ - if(!strncmp("__Secure-", curlx_str(&name), 9)) - co->prefix_secure = TRUE; - else if(!strncmp("__Host-", curlx_str(&name), 7)) - co->prefix_host = TRUE; if(!curlx_strlen(&cookie[COOKIE_NAME])) { /* The first name/value pair is the actual cookie name */ if(!sep || /* Bad name/value pair. */ invalid_octets(curlx_str(&name), curlx_strlen(&name)) || - invalid_octets(curlx_str(&val), curlx_strlen(&val))) { + invalid_octets(curlx_str(&val), curlx_strlen(&val)) || + !curlx_strlen(&name)) { infof(data, "invalid octets in name/value, cookie dropped"); return CURLE_OK; } + + /* + * Check for too long individual name or contents, or too long + * combination of name + contents. Chrome and Firefox support 4095 or + * 4096 bytes combo + */ + if(curlx_strlen(&name) >= (MAX_NAME - 1) || + curlx_strlen(&val) >= (MAX_NAME - 1) || + ((curlx_strlen(&name) + curlx_strlen(&val)) > MAX_NAME)) { + infof(data, "oversized cookie dropped, name/val %zu + %zu bytes", + curlx_strlen(&name), curlx_strlen(&val)); + return CURLE_OK; + } + + /* Reject cookies with a TAB inside the value */ + if(curlx_strlen(&val) && + memchr(curlx_str(&val), '\t', curlx_strlen(&val))) { + infof(data, "cookie contains TAB, dropping"); + return CURLE_OK; + } + + /* Check if we have a reserved prefix set. */ + if(!strncmp("__Secure-", curlx_str(&name), 9)) + co->prefix_secure = TRUE; + else if(!strncmp("__Host-", curlx_str(&name), 7)) + co->prefix_host = TRUE; + cookie[COOKIE_NAME] = name; cookie[COOKIE_VALUE] = val; - done = TRUE; } - else if(!curlx_strlen(&val)) { + else if(!sep) { /* - * this was a "=" with no content, and we must allow - * 'secure' and 'httponly' specified this weirdly + * this is a "" with no content */ - done = 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 @@ -529,18 +523,13 @@ parse_cookie_header(struct Curl_easy *data, if(secure || !ci->running) co->secure = TRUE; else { - infof(data, "skipped cookie %s because not 'secure'", co->name); + infof(data, "skipped cookie because not 'secure'"); return CURLE_OK; } } else if(curlx_str_casecompare(&name, "httponly")) co->httponly = TRUE; - else if(sep) - /* there was a '=' so we are not done parsing this field */ - done = FALSE; } - if(done) - ; else if(curlx_str_casecompare(&name, "path")) { cookie[COOKIE_PATH] = val; } @@ -588,9 +577,6 @@ parse_cookie_header(struct Curl_easy *data, return CURLE_OK; } } - else if(curlx_str_casecompare(&name, "version")) { - /* just ignore */ - } else if(curlx_str_casecompare(&name, "max-age") && curlx_strlen(&val)) { /* * Defined in RFC2109: @@ -606,6 +592,8 @@ parse_cookie_header(struct Curl_easy *data, if(*maxage == '\"') maxage++; rc = curlx_str_number(&maxage, &co->expires, CURL_OFF_T_MAX); + if(!now) + now = time(NULL); switch(rc) { case STRE_OVERFLOW: /* overflow, used max value */ @@ -627,46 +615,38 @@ parse_cookie_header(struct Curl_easy *data, } cap_expires(now, co); } - else if(curlx_str_casecompare(&name, "expires") && curlx_strlen(&val)) { - if(!co->expires && (curlx_strlen(&val) < MAX_DATE_LENGTH)) { - /* - * Let max-age have priority. - * - * If the date cannot get parsed for whatever reason, the cookie - * will be treated as a session cookie - */ - char dbuf[MAX_DATE_LENGTH + 1]; - time_t date = 0; - memcpy(dbuf, curlx_str(&val), curlx_strlen(&val)); - dbuf[curlx_strlen(&val)] = 0; - if(!Curl_getdate_capped(dbuf, &date)) { - if(!date) - date++; - co->expires = (curl_off_t)date; - } - else - co->expires = 0; - cap_expires(now, co); + else if(curlx_str_casecompare(&name, "expires") && curlx_strlen(&val) && + !co->expires && (curlx_strlen(&val) < MAX_DATE_LENGTH)) { + /* + * Let max-age have priority. + * + * If the date cannot get parsed for whatever reason, the cookie + * will be treated as a session cookie + */ + char dbuf[MAX_DATE_LENGTH + 1]; + time_t date = 0; + memcpy(dbuf, curlx_str(&val), curlx_strlen(&val)); + dbuf[curlx_strlen(&val)] = 0; + if(!Curl_getdate_capped(dbuf, &date)) { + if(!date) + date++; + co->expires = (curl_off_t)date; } + else + co->expires = 0; + if(!now) + now = time(NULL); + cap_expires(now, co); } - - /* - * Else, this is the second (or more) name we do not know about! - */ } + } while(!curlx_str_single(&ptr, ';')); - if(curlx_str_single(&ptr, ';')) - break; - } while(1); - - if(!curlx_strlen(&cookie[COOKIE_NAME])) - /* If we did not get a cookie name, or a bad one, bail out. */ - return CURLE_OK; - - /* the header was fine, now store the data */ - result = storecookie(co, &cookie[0], path, domain); - if(!result) - *okay = TRUE; + if(curlx_strlen(&cookie[COOKIE_NAME])) { + /* the header was fine, now store the data */ + result = storecookie(co, &cookie[0], path, domain); + if(!result) + *okay = TRUE; + } return result; } diff --git a/tests/data/test1160 b/tests/data/test1160 index 9eca524c65..d26ad4133f 100644 --- a/tests/data/test1160 +++ b/tests/data/test1160 @@ -14,7 +14,7 @@ cookies HTTP/1.1 200 OK Date: Tue, 09 Nov 2010 14:49:00 GMT Content-Length: 0 -Set-Cookie: ____________%hex[%ff]hex%= ;%repeat[117 x %20]%%hex[%ff]hex%%repeat[2602 x %20]%%repeat[434 x z]%%hex[%86%85%85%80]hex%%repeat[49 x z]%%hex[%fa]hex%%repeat[540 x z]%%hex[%f3%a0%81%96]hex%zzzzzzzzzzzz~%repeat[82 x z]%%hex[%b6]hex%%repeat[364 x z]% +Set-Cookie: %repeat[4000 x n]%=%repeat[4096 x v]% diff --git a/tests/data/test31 b/tests/data/test31 index 8353b709fc..e13fed55c6 100644 --- a/tests/data/test31 +++ b/tests/data/test31 @@ -18,6 +18,8 @@ Server: test-server/fake%CR Content-Length: 4%CR Content-Type: text/html%CR Funny-head: yesyes%CR +Set-Cookie: +Set-Cookie: ; Set-Cookie: blankdomain=sure; domain=; path=/ Set-Cookie: foobar=name; domain=anything.com; path=/ ; secure%CR Set-Cookie:ismatch=this ; domain=test31.curl; path=/silly/%CR @@ -25,27 +27,27 @@ Set-Cookie:ISMATCH=this ; domain=test31.curl; path=/silly/%CR Set-Cookie: overwrite=this ; domain=test31.curl; path=/overwrite/%CR Set-Cookie: overwrite=this2 ; domain=test31.curl; path=/overwrite%CR Set-Cookie: sec1value=secure1 ; domain=test31.curl; path=/secure1/ ; secure%CR -Set-Cookie: sec2value=secure2 ; domain=test31.curl; path=/secure2/ ; secure=%CR -Set-Cookie: sec3value=secure3 ; domain=test31.curl; path=/secure3/ ; secure=%CR -Set-Cookie: sec4value=secure4 ; secure=; domain=test31.curl; path=/secure4/ ; %CR +Set-Cookie: sec2value=secure2 ; domain=test31.curl; path=/secure2/ ; secure%CR +Set-Cookie: sec3value=secure3 ; domain=test31.curl; path=/secure3/ ; secure%CR +Set-Cookie: sec4value=secure4 ; secure; domain=test31.curl; path=/secure4/ ; %CR Set-Cookie: sec5value=secure5 ; secure; domain=test31.curl; path=/secure5/ ; %CR Set-Cookie: sec6value=secure6 ; secure ; domain=test31.curl; path=/secure6/ ; %CR Set-Cookie: sec7value=secure7 ; secure ; domain=test31.curl; path=/secure7/ ; %CR -Set-Cookie: sec8value=secure8 ; secure= ; domain=test31.curl; path=/secure8/ ; %CR -Set-Cookie: secure=very1 ; secure=; domain=test31.curl; path=/secure9/; %CR +Set-Cookie: sec8value=secure8 ; secure ; domain=test31.curl; path=/secure8/ ; %CR +Set-Cookie: secure=very1 ; secure; domain=test31.curl; path=/secure9/; %CR Set-Cookie: httpo1=value1 ; domain=test31.curl; path=/p1/; httponly%CR -Set-Cookie: httpo2=value2 ; domain=test31.curl; path=/p2/; httponly=%CR +Set-Cookie: httpo2=value2 ; domain=test31.curl; path=/p2/; httponly%CR Set-Cookie: httpo3=value3 ; httponly; domain=test31.curl; path=/p3/;%CR -Set-Cookie: httpo4=value4 ; httponly=; domain=test31.curl; path=/p4/; %CR +Set-Cookie: httpo4=value4 ; httponly; domain=test31.curl; path=/p4/; %CR Set-Cookie: httponly=myvalue1 ; domain=test31.curl; path=/p4/; httponly%CR Set-Cookie: httpandsec=myvalue2 ; domain=test31.curl; path=/p4/; httponly; secure%CR -Set-Cookie: httpandsec2=myvalue3; domain=test31.curl; path=/p4/; httponly=; secure%CR -Set-Cookie: httpandsec3=myvalue4 ; domain=test31.curl; path=/p4/; httponly; secure=%CR -Set-Cookie: httpandsec4=myvalue5 ; domain=test31.curl; path=/p4/; httponly=; secure=%CR -Set-Cookie: httpandsec5=myvalue6 ; domain=test31.curl; path=/p4/; secure; httponly=%CR -Set-Cookie: httpandsec6=myvalue7 ; domain=test31.curl; path=/p4/; secure=; httponly=%CR +Set-Cookie: httpandsec2=myvalue3; domain=test31.curl; path=/p4/; httponly; secure%CR +Set-Cookie: httpandsec3=myvalue4 ; domain=test31.curl; path=/p4/; httponly; secure%CR +Set-Cookie: httpandsec4=myvalue5 ; domain=test31.curl; path=/p4/; httponly; secure%CR +Set-Cookie: httpandsec5=myvalue6 ; domain=test31.curl; path=/p4/; secure; httponly%CR +Set-Cookie: httpandsec6=myvalue7 ; domain=test31.curl; path=/p4/; secure; httponly%CR Set-Cookie: httpandsec7=myvalue8 ; domain=test31.curl; path=/p4/; secure; httponly%CR -Set-Cookie: httpandsec8=myvalue9; domain=test31.curl; path=/p4/; secure=; httponly%CR +Set-Cookie: httpandsec8=myvalue9; domain=test31.curl; path=/p4/; secure; httponly%CR Set-Cookie: partmatch=present; domain=test31.curl ; path=/;%CR Set-Cookie:eat=this; domain=moo.foo.moo;%CR Set-Cookie: eat=this-too; domain=.foo.moo;%CR @@ -67,7 +69,7 @@ Set-Cookie: partialip=nono; domain=.0.0.1;%CR Set-Cookie: withspaces= yes within and around ;%CR Set-Cookie: withspaces2 =before equals;%CR Set-Cookie: prespace= yes before;%CR -Set-Cookie: securewithspace=after ; secure =%CR +Set-Cookie: securewithspace=after ; secure %CR Set-Cookie: %hex[%c3%82%c2%b3%c3%83%5c%78%39%32%c3%83%5c%78%39%61%c3%83%5c%78%38%64%c3%83%5c%78%39%37]hex%=%96%A6g%9Ay%B0%A5g%A7tm%7C%95%9A %CR boo