]> git.ipfire.org Git - thirdparty/curl.git/commitdiff
cookie: fix crash in netscape cookie parsing
authorJay Satiro <raysatiro@yahoo.com>
Tue, 24 Dec 2024 07:18:37 +0000 (02:18 -0500)
committerJay Satiro <raysatiro@yahoo.com>
Fri, 27 Dec 2024 18:16:08 +0000 (13:16 -0500)
- 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

docs/libcurl/opts/CURLOPT_COOKIELIST.md
lib/cookie.c
tests/data/Makefile.am
tests/data/test3104 [new file with mode: 0644]
tests/libtest/Makefile.inc
tests/libtest/lib3104.c [new file with mode: 0644]

index d7a6c0b12965eecba041849a98d7eec45969f4d7..138a927a56442978d11cd4340a70430c295b8642 100644 (file)
@@ -81,7 +81,7 @@ NULL
 
 int main(void)
 {
-  char *my_cookie =
+  const char *my_cookie =
     "example.com"    /* Hostname */
     SEP "FALSE"      /* Include subdomains */
     SEP "/"          /* Path */
index aaa65368cfc76da51d946e9b40e8a5f78de01b9a..e1f1c306a01ea0103de7016ddb1227772ec0153e 100644 (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;
index ec30fdf31c86252310798f3510b7c492f866e5eb..73ef90920afa6261bf3bf3f4cb563f502a452552 100644 (file)
@@ -269,7 +269,7 @@ test3008 test3009 test3010 test3011 test3012 test3013 test3014 test3015 \
 test3016 test3017 test3018 test3019 test3020 test3021 test3022 test3023 \
 test3024 test3025 test3026 test3027 test3028 test3029 test3030 test3031 \
 \
-test3100 test3101 test3102 test3103 \
+test3100 test3101 test3102 test3103 test3104 \
 test3200 \
 test3201 test3202 test3203 test3204 test3205 test3207
 
diff --git a/tests/data/test3104 b/tests/data/test3104
new file mode 100644 (file)
index 0000000..5853898
--- /dev/null
@@ -0,0 +1,60 @@
+<testcase>
+<info>
+<keywords>
+cookies
+</keywords>
+</info>
+
+#
+# Server-side
+<reply>
+<data crlf="yes">
+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: close
+Content-Type: text/html
+Funny-head: yesyes
+
+-foo-
+</data>
+</reply>
+
+#
+# Client-side
+<client>
+<features>
+cookies
+proxy
+</features>
+<server>
+http
+</server>
+<tool>
+lib%TESTNUMBER
+</tool>
+<name>
+CURLOPT_COOKIELIST with netscape format
+</name>
+<command>
+http://%HOSTIP:%HTTPPORT/%TESTNUMBER
+</command>
+</client>
+
+#
+# Verify data after the test has been "shot"
+<verify>
+<protocol crlf="yes">
+GET http://example.com/ HTTP/1.1
+Host: example.com
+Accept: */*
+Proxy-Connection: Keep-Alive
+Cookie: name=value
+
+</protocol>
+</verify>
+</testcase>
index a84886f591998ece9d5b242674fa14c369a700d0..174c7a8da6361f42b3f72d9994fdf229f4b572e4 100644 (file)
@@ -75,7 +75,7 @@ LIBTESTPROGS = libauthretry libntlmconnect libprereq                     \
  lib2402 lib2404 lib2405 \
  lib2502 \
  lib3010 lib3025 lib3026 lib3027 \
- lib3100 lib3101 lib3102 lib3103 lib3207
+ lib3100 lib3101 lib3102 lib3103 lib3104 lib3207
 
 libntlmconnect_SOURCES = libntlmconnect.c $(SUPPORTFILES) $(TESTUTIL) $(WARNLESS)
 libntlmconnect_LDADD = $(TESTUTIL_LIBS)
@@ -718,5 +718,7 @@ lib3102_LDADD = $(TESTUTIL_LIBS)
 lib3103_SOURCES = lib3103.c $(SUPPORTFILES)
 lib3103_LDADD = $(TESTUTIL_LIBS)
 
+lib3104_SOURCES = lib3104.c $(SUPPORTFILES)
+
 lib3207_SOURCES = lib3207.c $(SUPPORTFILES) $(TESTUTIL) $(THREADS) $(WARNLESS) $(MULTIBYTE)
 lib3207_LDADD = $(TESTUTIL_LIBS)
diff --git a/tests/libtest/lib3104.c b/tests/libtest/lib3104.c
new file mode 100644 (file)
index 0000000..7e0e1d8
--- /dev/null
@@ -0,0 +1,66 @@
+/***************************************************************************
+ *                                  _   _ ____  _
+ *  Project                     ___| | | |  _ \| |
+ *                             / __| | | | |_) | |
+ *                            | (__| |_| |  _ <| |___
+ *                             \___|\___/|_| \_\_____|
+ *
+ * Copyright (C) Daniel Stenberg, <daniel@haxx.se>, et al.
+ *
+ * This software is licensed as described in the file COPYING, which
+ * you should have received as part of this distribution. The terms
+ * are also available at https://curl.se/docs/copyright.html.
+ *
+ * You may opt to use, copy, modify, merge, publish, distribute and/or sell
+ * copies of the Software, and permit persons to whom the Software is
+ * furnished to do so, under the terms of the COPYING file.
+ *
+ * This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY
+ * KIND, either express or implied.
+ *
+ * SPDX-License-Identifier: curl
+ *
+ ***************************************************************************/
+#include "test.h"
+
+#include "memdebug.h"
+
+CURLcode test(char *URL)
+{
+  CURLcode res = CURLE_OK;
+  CURLSH *share;
+  CURL *curl;
+
+  curl_global_init(CURL_GLOBAL_ALL);
+
+  share = curl_share_init();
+  curl_share_setopt(share, CURLSHOPT_SHARE, CURL_LOCK_DATA_COOKIE);
+
+  curl = curl_easy_init();
+  test_setopt(curl, CURLOPT_SHARE, share);
+
+  test_setopt(curl, CURLOPT_VERBOSE, 1L);
+  test_setopt(curl, CURLOPT_HEADER, 1L);
+  test_setopt(curl, CURLOPT_PROXY, URL);
+  test_setopt(curl, CURLOPT_URL, "http://example.com/");
+
+  test_setopt(curl, CURLOPT_COOKIEFILE, "");
+
+  test_setopt(curl, CURLOPT_COOKIELIST,
+              "example.com\tFALSE\t/\tFALSE\t0\tname\tvalue");
+
+  res = curl_easy_perform(curl);
+  if(res) {
+    fprintf(stderr, "curl_easy_perform() failed: %s\n",
+            curl_easy_strerror(res));
+  }
+
+test_cleanup:
+
+  /* always cleanup */
+  curl_easy_cleanup(curl);
+  curl_share_cleanup(share);
+  curl_global_cleanup();
+
+  return res;
+}