From 7c21c1c4f981a947f9f91ff685f898d0306589f7 Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Tue, 9 Aug 2011 14:02:05 +0200 Subject: cookie parser: handle 'secure=' There are two keywords in cookie headers that don't follow the regular name=value style: secure and httponly. Still we must support that they are written like 'secure=' and then treat them as if they were written 'secure'. Test case 31 was much extended by Rob Ward to test this. Bug: http://curl.haxx.se/bug/view.cgi?id=3349227 Reported by: "gnombat" --- lib/cookie.c | 322 +++++++++++++++++++++++++++--------------------------- tests/data/test31 | 44 ++++++++ 2 files changed, 205 insertions(+), 161 deletions(-) diff --git a/lib/cookie.c b/lib/cookie.c index 86d264c45..0553efb98 100644 --- a/lib/cookie.c +++ b/lib/cookie.c @@ -206,7 +206,6 @@ Curl_cookie_add(struct SessionHandle *data, if(httpheader) { /* This line was read off a HTTP-header */ const char *ptr; - const char *sep; const char *semiptr; char *what; @@ -223,185 +222,186 @@ Curl_cookie_add(struct SessionHandle *data, ptr = lineptr; do { - /* we have a = pair or a 'secure' word here */ - sep = strchr(ptr, '='); - if(sep && (!semiptr || (semiptr>sep)) ) { - /* - * There is a = sign and if there was a semicolon too, which make sure - * that the semicolon comes _after_ the equal sign. - */ - - name[0]=what[0]=0; /* init the buffers */ - if(1 <= sscanf(ptr, "%" MAX_NAME_TXT "[^;=]=%" - MAX_COOKIE_LINE_TXT "[^;\r\n]", - name, what)) { - /* this is a = pair. We use strstore() below to properly - deal with received cookie headers that have the same string - property set more than once, and then we use the last one. */ - - const char *whatptr; - - /* Strip off trailing whitespace from the 'what' */ - size_t len=strlen(what); - while(len && ISBLANK(what[len-1])) { - what[len-1]=0; - len--; - } + /* we have a = pair or a stand-alone word here */ + name[0]=what[0]=0; /* init the buffers */ + if(1 <= sscanf(ptr, "%" MAX_NAME_TXT "[^;\r\n =]=%" + MAX_COOKIE_LINE_TXT "[^;\r\n]", + name, what)) { + /* Use strstore() below to properly deal with received cookie + headers that have the same string property set more than once, + and then we use the last one. */ + const char *whatptr; + bool done = FALSE; + bool sep; + size_t len=strlen(what); + const char *endofn = &ptr[ strlen(name) ]; + + /* skip trailing spaces in name */ + while(*endofn && ISBLANK(*endofn)) + endofn++; + + /* name ends with a '=' ? */ + sep = *endofn == '='?TRUE:FALSE; + + /* Strip off trailing whitespace from the 'what' */ + while(len && ISBLANK(what[len-1])) { + what[len-1]=0; + len--; + } - /* Skip leading whitespace from the 'what' */ - whatptr=what; - while(*whatptr && ISBLANK(*whatptr)) { - whatptr++; - } + /* Skip leading whitespace from the 'what' */ + whatptr=what; + while(*whatptr && ISBLANK(*whatptr)) + whatptr++; - if(Curl_raw_equal("path", name)) { - strstore(&co->path, whatptr); - if(!co->path) { - badcookie = TRUE; /* out of memory bad */ - break; + if(!len) { + /* this was a "=" with no content, and we must allow + 'secure' and 'httponly' specified this weirdly */ + done = TRUE; + if(Curl_raw_equal("secure", name)) + co->secure = TRUE; + else if(Curl_raw_equal("httponly", name)) + co->httponly = TRUE; + else if(sep) + /* there was a '=' so we're not done parsing this field */ + done = FALSE; + } + if(done) + ; + else if(Curl_raw_equal("path", name)) { + strstore(&co->path, whatptr); + if(!co->path) { + badcookie = TRUE; /* out of memory bad */ + break; + } + } + else if(Curl_raw_equal("domain", name)) { + /* note that this name may or may not have a preceding dot, but + we don't care about that, we treat the names the same anyway */ + + const char *domptr=whatptr; + const char *nextptr; + int dotcount=1; + + /* Count the dots, we need to make sure that there are enough + of them. */ + + if('.' == whatptr[0]) + /* don't count the initial dot, assume it */ + domptr++; + + do { + nextptr = strchr(domptr, '.'); + if(nextptr) { + if(domptr != nextptr) + dotcount++; + domptr = nextptr+1; } + } while(nextptr); + + /* The original Netscape cookie spec defined that this domain name + MUST have three dots (or two if one of the seven holy TLDs), + but it seems that these kinds of cookies are in use "out there" + so we cannot be that strict. I've therefore lowered the check + to not allow less than two dots. */ + + if(dotcount < 2) { + /* Received and skipped a cookie with a domain using too few + dots. */ + badcookie=TRUE; /* mark this as a bad cookie */ + infof(data, "skipped cookie with illegal dotcount domain: %s\n", + whatptr); } - else if(Curl_raw_equal("domain", name)) { - /* note that this name may or may not have a preceding dot, but - we don't care about that, we treat the names the same anyway */ - - const char *domptr=whatptr; - const char *nextptr; - int dotcount=1; - - /* Count the dots, we need to make sure that there are enough - of them. */ + else { + /* Now, we make sure that our host is within the given domain, + or the given domain is not valid and thus cannot be set. */ if('.' == whatptr[0]) - /* don't count the initial dot, assume it */ - domptr++; - - do { - nextptr = strchr(domptr, '.'); - if(nextptr) { - if(domptr != nextptr) - dotcount++; - domptr = nextptr+1; + whatptr++; /* ignore preceding dot */ + + if(!domain || tailmatch(whatptr, domain)) { + const char *tailptr=whatptr; + if(tailptr[0] == '.') + tailptr++; + strstore(&co->domain, tailptr); /* don't prefix w/dots + internally */ + if(!co->domain) { + badcookie = TRUE; + break; } - } while(nextptr); - - /* The original Netscape cookie spec defined that this domain name - MUST have three dots (or two if one of the seven holy TLDs), - but it seems that these kinds of cookies are in use "out there" - so we cannot be that strict. I've therefore lowered the check - to not allow less than two dots. */ - - if(dotcount < 2) { - /* Received and skipped a cookie with a domain using too few - dots. */ - badcookie=TRUE; /* mark this as a bad cookie */ - infof(data, "skipped cookie with illegal dotcount domain: %s\n", - whatptr); + co->tailmatch=TRUE; /* we always do that if the domain name was + given */ } else { - /* Now, we make sure that our host is within the given domain, - or the given domain is not valid and thus cannot be set. */ - - if('.' == whatptr[0]) - whatptr++; /* ignore preceding dot */ - - if(!domain || tailmatch(whatptr, domain)) { - const char *tailptr=whatptr; - if(tailptr[0] == '.') - tailptr++; - strstore(&co->domain, tailptr); /* don't prefix w/dots - internally */ - if(!co->domain) { - badcookie = TRUE; - break; - } - co->tailmatch=TRUE; /* we always do that if the domain name was - given */ - } - else { - /* we did not get a tailmatch and then the attempted set domain - is not a domain to which the current host belongs. Mark as - bad. */ - badcookie=TRUE; - infof(data, "skipped cookie with bad tailmatch domain: %s\n", - whatptr); - } - } - } - else if(Curl_raw_equal("version", name)) { - strstore(&co->version, whatptr); - if(!co->version) { - badcookie = TRUE; - break; + /* we did not get a tailmatch and then the attempted set domain + is not a domain to which the current host belongs. Mark as + bad. */ + badcookie=TRUE; + infof(data, "skipped cookie with bad tailmatch domain: %s\n", + whatptr); } } - else if(Curl_raw_equal("max-age", name)) { - /* Defined in RFC2109: - - Optional. The Max-Age attribute defines the lifetime of the - cookie, in seconds. The delta-seconds value is a decimal non- - negative integer. After delta-seconds seconds elapse, the - client should discard the cookie. A value of zero means the - cookie should be discarded immediately. - - */ - strstore(&co->maxage, whatptr); - if(!co->maxage) { - badcookie = TRUE; - break; - } - co->expires = - strtol((*co->maxage=='\"')?&co->maxage[1]:&co->maxage[0],NULL,10) - + (long)now; + } + else if(Curl_raw_equal("version", name)) { + strstore(&co->version, whatptr); + if(!co->version) { + badcookie = TRUE; + break; } - else if(Curl_raw_equal("expires", name)) { - strstore(&co->expirestr, whatptr); - if(!co->expirestr) { - badcookie = TRUE; - break; - } - /* Note that if the date couldn't get parsed for whatever reason, - the cookie will be treated as a session cookie */ - co->expires = curl_getdate(what, &now); - - /* Session cookies have expires set to 0 so if we get that back - from the date parser let's add a second to make it a - non-session cookie */ - if(co->expires == 0) - co->expires = 1; - else if(co->expires < 0) - co->expires = 0; + } + else if(Curl_raw_equal("max-age", name)) { + /* Defined in RFC2109: + + Optional. The Max-Age attribute defines the lifetime of the + cookie, in seconds. The delta-seconds value is a decimal non- + negative integer. After delta-seconds seconds elapse, the + client should discard the cookie. A value of zero means the + cookie should be discarded immediately. + + */ + strstore(&co->maxage, whatptr); + if(!co->maxage) { + badcookie = TRUE; + break; } - else if(!co->name) { - co->name = strdup(name); - co->value = strdup(whatptr); - if(!co->name || !co->value) { - badcookie = TRUE; - break; - } + co->expires = + strtol((*co->maxage=='\"')?&co->maxage[1]:&co->maxage[0],NULL,10) + + (long)now; + } + else if(Curl_raw_equal("expires", name)) { + strstore(&co->expirestr, whatptr); + if(!co->expirestr) { + badcookie = TRUE; + break; } - /* - else this is the second (or more) name we don't know - about! */ + /* Note that if the date couldn't get parsed for whatever reason, + the cookie will be treated as a session cookie */ + co->expires = curl_getdate(what, &now); + + /* Session cookies have expires set to 0 so if we get that back + from the date parser let's add a second to make it a + non-session cookie */ + if(co->expires == 0) + co->expires = 1; + else if(co->expires < 0) + co->expires = 0; } - else { - /* this is an "illegal" = pair */ + else if(!co->name) { + co->name = strdup(name); + co->value = strdup(whatptr); + if(!co->name || !co->value) { + badcookie = TRUE; + break; + } } + /* + else this is the second (or more) name we don't know + about! */ } else { - if(sscanf(ptr, "%" MAX_COOKIE_LINE_TXT "[^;\r\n]", - what)) { - if(Curl_raw_equal("secure", what)) { - co->secure = TRUE; - } - else if(Curl_raw_equal("httponly", what)) { - co->httponly = TRUE; - } - /* else, - unsupported keyword without assign! */ - - } + /* this is an "illegal" = pair */ } + if(!semiptr || !*semiptr) { /* we already know there are no more cookies */ semiptr = NULL; diff --git a/tests/data/test31 b/tests/data/test31 index d06bc1180..5afe81df6 100644 --- a/tests/data/test31 +++ b/tests/data/test31 @@ -18,6 +18,28 @@ Content-Type: text/html Funny-head: yesyes Set-Cookie: foobar=name; domain=anything.com; path=/ ; secure Set-Cookie:ismatch=this ; domain=127.0.0.1; path=/silly/ +Set-Cookie: sec1value=secure1 ; domain=127.0.0.1; path=/secure1/ ; secure +Set-Cookie: sec2value=secure2 ; domain=127.0.0.1; path=/secure2/ ; secure= +Set-Cookie: sec3value=secure3 ; domain=127.0.0.1; path=/secure3/ ; secure= +Set-Cookie: sec4value=secure4 ; secure=; domain=127.0.0.1; path=/secure4/ ; +Set-Cookie: sec5value=secure5 ; secure; domain=127.0.0.1; path=/secure5/ ; +Set-Cookie: sec6value=secure6 ; secure ; domain=127.0.0.1; path=/secure6/ ; +Set-Cookie: sec7value=secure7 ; secure ; domain=127.0.0.1; path=/secure7/ ; +Set-Cookie: sec8value=secure8 ; secure= ; domain=127.0.0.1; path=/secure8/ ; +Set-Cookie: secure=very1 ; secure=; domain=127.0.0.1; path=/secure9/; +Set-Cookie: httpo1=value1 ; domain=127.0.0.1; path=/p1/; httponly +Set-Cookie: httpo2=value2 ; domain=127.0.0.1; path=/p2/; httponly= +Set-Cookie: httpo3=value3 ; httponly; domain=127.0.0.1; path=/p3/; +Set-Cookie: httpo4=value4 ; httponly=; domain=127.0.0.1; path=/p4/; +Set-Cookie: httponly=myvalue1 ; domain=127.0.0.1; path=/p4/; httponly +Set-Cookie: httpandsec=myvalue2 ; domain=127.0.0.1; path=/p4/; httponly; secure +Set-Cookie: httpandsec2=myvalue3; domain=127.0.0.1; path=/p4/; httponly=; secure +Set-Cookie: httpandsec3=myvalue4 ; domain=127.0.0.1; path=/p4/; httponly; secure= +Set-Cookie: httpandsec4=myvalue5 ; domain=127.0.0.1; path=/p4/; httponly=; secure= +Set-Cookie: httpandsec5=myvalue6 ; domain=127.0.0.1; path=/p4/; secure; httponly= +Set-Cookie: httpandsec6=myvalue7 ; domain=127.0.0.1; path=/p4/; secure=; httponly= +Set-Cookie: httpandsec7=myvalue8 ; domain=127.0.0.1; path=/p4/; secure; httponly +Set-Cookie: httpandsec8=myvalue9; domain=127.0.0.1; path=/p4/; secure=; httponly Set-Cookie: partmatch=present; domain=127.0.0.1 ; path=/; Set-Cookie:eat=this; domain=moo.foo.moo; Set-Cookie: eat=this-too; domain=.foo.moo; @@ -69,6 +91,28 @@ Accept: */* # This file was generated by libcurl! Edit at your own risk. .127.0.0.1 TRUE /silly/ FALSE 0 ismatch this +.127.0.0.1 TRUE /secure1/ TRUE 0 sec1value secure1 +.127.0.0.1 TRUE /secure2/ TRUE 0 sec2value secure2 +.127.0.0.1 TRUE /secure3/ TRUE 0 sec3value secure3 +.127.0.0.1 TRUE /secure4/ TRUE 0 sec4value secure4 +.127.0.0.1 TRUE /secure5/ TRUE 0 sec5value secure5 +.127.0.0.1 TRUE /secure6/ TRUE 0 sec6value secure6 +.127.0.0.1 TRUE /secure7/ TRUE 0 sec7value secure7 +.127.0.0.1 TRUE /secure8/ TRUE 0 sec8value secure8 +.127.0.0.1 TRUE /secure9/ TRUE 0 secure very1 +#HttpOnly_.127.0.0.1 TRUE /p1/ FALSE 0 httpo1 value1 +#HttpOnly_.127.0.0.1 TRUE /p2/ FALSE 0 httpo2 value2 +#HttpOnly_.127.0.0.1 TRUE /p3/ FALSE 0 httpo3 value3 +#HttpOnly_.127.0.0.1 TRUE /p4/ FALSE 0 httpo4 value4 +#HttpOnly_.127.0.0.1 TRUE /p4/ FALSE 0 httponly myvalue1 +#HttpOnly_.127.0.0.1 TRUE /p4/ TRUE 0 httpandsec myvalue2 +#HttpOnly_.127.0.0.1 TRUE /p4/ TRUE 0 httpandsec2 myvalue3 +#HttpOnly_.127.0.0.1 TRUE /p4/ TRUE 0 httpandsec3 myvalue4 +#HttpOnly_.127.0.0.1 TRUE /p4/ TRUE 0 httpandsec4 myvalue5 +#HttpOnly_.127.0.0.1 TRUE /p4/ TRUE 0 httpandsec5 myvalue6 +#HttpOnly_.127.0.0.1 TRUE /p4/ TRUE 0 httpandsec6 myvalue7 +#HttpOnly_.127.0.0.1 TRUE /p4/ TRUE 0 httpandsec7 myvalue8 +#HttpOnly_.127.0.0.1 TRUE /p4/ TRUE 0 httpandsec8 myvalue9 .127.0.0.1 TRUE / FALSE 0 partmatch present 127.0.0.1 FALSE /we/want/ FALSE 2054030187 nodomain value #HttpOnly_127.0.0.1 FALSE /silly/ FALSE 0 magic yessir -- cgit v1.2.3