cookie: fix crash in netscape cookie parsing

- Parse the input string without modifying it.

Prior to this change a segfault could occur if the input string was
const because the tokenizer modified the input string. For example if
the user set CURLOPT_COOKIELIST to a const string then libcurl would
likely cause a crash when modifying that string. Even if the string was
not const or a crash did not occur there was still the incorrect and
unexpected modification of the user's input string.

This issue was caused by 30da1f59 (precedes 8.11.0) which refactored
some options parsing and eliminated the copy of the input string. Also,
an earlier commit f88cc654 incorrectly cast the input pointer when
passing it to strtok.

Co-authored-by: Daniel Stenberg

Closes https://github.com/curl/curl/pull/15826
This commit is contained in:
Jay Satiro 2024-12-24 02:18:37 -05:00
parent fabfa8e402
commit 39e21794a7
6 changed files with 157 additions and 29 deletions

View file

@ -815,10 +815,9 @@ parse_netscape(struct Cookie *co,
* This line is NOT an HTTP header style line, we do offer support for
* reading the odd netscape cookies-file format here
*/
char *ptr;
char *firstptr;
char *tok_buf = NULL;
const char *ptr, *next;
int fields;
size_t len;
/*
* In 2008, Internet Explorer introduced HTTP-only cookies to prevent XSS
@ -835,29 +834,22 @@ parse_netscape(struct Cookie *co,
/* do not even try the comments */
return CERR_COMMENT;
/* strip off the possible end-of-line characters */
ptr = strchr(lineptr, '\r');
if(ptr)
*ptr = 0; /* clear it */
ptr = strchr(lineptr, '\n');
if(ptr)
*ptr = 0; /* clear it */
/* tokenize on TAB */
firstptr = Curl_strtok_r((char *)lineptr, "\t", &tok_buf);
/*
* Now loop through the fields and init the struct we already have
* allocated
*/
fields = 0;
for(ptr = firstptr; ptr;
ptr = Curl_strtok_r(NULL, "\t", &tok_buf), fields++) {
for(next = lineptr; next; fields++) {
ptr = next;
len = strcspn(ptr, "\t\r\n");
next = (ptr[len] == '\t' ? &ptr[len + 1] : NULL);
switch(fields) {
case 0:
if(ptr[0]=='.') /* skip preceding dots */
if(ptr[0]=='.') { /* skip preceding dots */
ptr++;
co->domain = strdup(ptr);
len--;
}
co->domain = Curl_memdup0(ptr, len);
if(!co->domain)
return CERR_OUT_OF_MEMORY;
break;
@ -867,13 +859,13 @@ parse_netscape(struct Cookie *co,
* domain can access the variable. Set TRUE when the cookie says
* .domain.com and to false when the domain is complete www.domain.com
*/
co->tailmatch = !!strcasecompare(ptr, "TRUE");
co->tailmatch = !!strncasecompare(ptr, "TRUE", len);
break;
case 2:
/* The file format allows the path field to remain not filled in */
if(strcmp("TRUE", ptr) && strcmp("FALSE", ptr)) {
if(strncmp("TRUE", ptr, len) && strncmp("FALSE", ptr, len)) {
/* only if the path does not look like a boolean option! */
co->path = strdup(ptr);
co->path = Curl_memdup0(ptr, len);
if(!co->path)
return CERR_OUT_OF_MEMORY;
else {
@ -894,7 +886,7 @@ parse_netscape(struct Cookie *co,
FALLTHROUGH();
case 3:
co->secure = FALSE;
if(strcasecompare(ptr, "TRUE")) {
if(strncasecompare(ptr, "TRUE", len)) {
if(secure || ci->running)
co->secure = TRUE;
else
@ -902,11 +894,19 @@ parse_netscape(struct Cookie *co,
}
break;
case 4:
if(curlx_strtoofft(ptr, NULL, 10, &co->expires))
return CERR_RANGE;
{
char *endp;
const char *p;
/* make sure curlx_strtoofft won't read past the current field */
for(p = ptr; p < &ptr[len] && ISDIGIT(*p); ++p)
;
if(p == ptr || p != &ptr[len] ||
curlx_strtoofft(ptr, &endp, 10, &co->expires) || endp != &ptr[len])
return CERR_RANGE;
}
break;
case 5:
co->name = strdup(ptr);
co->name = Curl_memdup0(ptr, len);
if(!co->name)
return CERR_OUT_OF_MEMORY;
else {
@ -918,7 +918,7 @@ parse_netscape(struct Cookie *co,
}
break;
case 6:
co->value = strdup(ptr);
co->value = Curl_memdup0(ptr, len);
if(!co->value)
return CERR_OUT_OF_MEMORY;
break;