From eac573ea9c368f5e3c07de4d5ec5c5d0f84a021a Mon Sep 17 00:00:00 2001

From: Tim Ruehsen <[email protected]>

Date: Tue, 19 Aug 2014 21:01:28 +0200

Subject: [PATCH 1/2] cookies: only use full host matches for hosts used as IP

 address

By not detecting and rejecting domain names for partial literal IP

addresses properly when parsing received HTTP cookies, libcurl can be

fooled to both send cookies to wrong sites and to allow arbitrary sites

to set cookies for others.

Bug: http://curl.haxx.se/docs/adv_20140910.html

---

 lib/cookie.c        | 50 ++++++++++++++++++++++++++++++++++++++----------

 tests/data/test1105 |  3 +--

 tests/data/test31   | 55 +++++++++++++++++++++++++++--------------------------

 tests/data/test8    |  3 ++-

 4 files changed, 71 insertions(+), 40 deletions(-)

This problem has been fixed upstream in curl version 7.38.0

--- lib/cookie.c.orig	2014-09-04 10:25:26.404578422 -0700

+++ lib/cookie.c	2014-09-04 10:40:04.769726955 -0700

@@ -97,6 +97,7 @@

 #include "strtoofft.h"

 #include "rawstr.h"

 #include "curl_memrchr.h"

+#include "inet_pton.h"

 /* The last #include file should be: */

 #include "memdebug.h"

@@ -181,6 +182,27 @@

   *str = strdup(newstr);

 }

+/*

+ * Return true if the given string is an IP(v4|v6) address.

+ */

+static bool isip(const char *domain)

+{

+  struct in_addr addr;

+#ifdef ENABLE_IPV6

+  struct in6_addr addr6;

+#endif

+

+  if(Curl_inet_pton(AF_INET, domain, &addr)

+#ifdef ENABLE_IPV6

+     || Curl_inet_pton(AF_INET6, domain, &addr6)

+#endif

+    ) {

+    /* domain name given as IP address */

+    return TRUE;

+  }

+

+  return FALSE;

+}

 /****************************************************************************

  *

@@ -280,6 +302,8 @@

             }

           }

           else if(Curl_raw_equal("domain", name)) {

+            bool is_ip;

+

             /* note that this name may or may not have a preceeding dot, but

                we don't care about that, we treat the names the same anyway */

@@ -321,18 +345,19 @@

               if('.' == whatptr[0])

                 whatptr++; /* ignore preceeding dot */

-              if(!domain || tailmatch(whatptr, domain)) {

-                const char *tailptr=whatptr;

-                if(tailptr[0] == '.')

-                  tailptr++;

-                strstore(&co->domain, tailptr); /* don't prefix w/dots

-                                                   internally */

+              is_ip = isip(domain ? domain : whatptr);

+

+              if(!domain

+                 || (is_ip && !strcmp(whatptr, domain))

+                 || (!is_ip && tailmatch(whatptr, domain))) {

+                strstore(&co->domain, whatptr);

                 if(!co->domain) {

                   badcookie = TRUE;

                   break;

                 }

-                co->tailmatch=TRUE; /* we always do that if the domain name was

-                                       given */

+                if(!is_ip)

+                  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

@@ -821,10 +846,14 @@

   time_t now = time(NULL);

   struct Cookie *mainco=NULL;

   size_t matches = 0;

+  bool is_ip;

   if(!c || !c->cookies)

     return NULL; /* no cookie struct or no cookies in the struct */

+  /* check if host is an IP(v4|v6) address */

+  is_ip = isip(host);

+

   co = c->cookies;

   while(co) {

@@ -836,8 +865,8 @@

       /* now check if the domain is correct */

       if(!co->domain ||

-         (co->tailmatch && tailmatch(co->domain, host)) ||

-         (!co->tailmatch && Curl_raw_equal(host, co->domain)) ) {

+         (co->tailmatch && !is_ip && tailmatch(co->domain, host)) ||

+         ((!co->tailmatch || is_ip) && Curl_raw_equal(host, co->domain)) ) {

         /* the right part of the host matches the domain stuff in the

            cookie data */

--- tests/data/test1105.orig	2014-09-04 10:27:20.052223915 -0700

+++ tests/data/test1105	2014-09-04 10:41:18.310085197 -0700

@@ -56,8 +56,7 @@

 # This file was generated by libcurl! Edit at your own risk.

 127.0.0.1	FALSE	/we/want/	FALSE	0	foobar	name

-.127.0.0.1	TRUE	"/silly/"	FALSE	0	mismatch	this

-.0.0.1	TRUE	/	FALSE	0	partmatch	present

+127.0.0.1	FALSE	"/silly/"	FALSE	0	mismatch	this

 </file>

 </verify>

 </testcase>

--- tests/data/test31.orig	2014-09-04 10:27:10.395450839 -0700

+++ tests/data/test31	2014-09-04 11:27:38.685969246 -0700

@@ -18,6 +18,30 @@

 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: overwrite=this  ; domain=127.0.0.1; path=/overwrite/

+Set-Cookie: overwrite=this2  ; domain=127.0.0.1; path=/overwrite

+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;

@@ -27,7 +51,8 @@

 Set-Cookie: test=yes; domain=foo.com; expires=Sat Feb 2 11:56:27 GMT 2030

 Set-Cookie: test2=yes; domain=se; expires=Sat Feb 2 11:56:27 GMT 2030

 Set-Cookie: magic=yessir; path=/silly/; HttpOnly

-Set-Cookie: blexp=yesyes; domain=.0.0.1; domain=.0.0.1; expiry=totally bad;

+Set-Cookie: blexp=yesyes; domain=127.0.0.1; domain=127.0.0.1; expiry=totally bad;

+Set-Cookie: partialip=nono; domain=.0.0.1;

 boo

 </data>

@@ -50,6 +75,9 @@

  <command>

 http://%HOSTIP:%HTTPPORT/we/want/31 -b none -c log/jar31.txt

 </command>

+<precheck>

+perl -e 'if ("%HOSTIP" !~ /127\.0\.0\.1$/) {print "Test only works for HOSTIP 127.0.0.1"; exit(1)}'

+</precheck>

 </client>

 # Verify data after the test has been "shot"

@@ -68,11 +96,35 @@

 # http://curl.haxx.se/rfc/cookie_spec.html

 # 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	/	FALSE	0	partmatch	present

+127.0.0.1	FALSE	/silly/	FALSE	0	ismatch	this

+127.0.0.1	FALSE	/overwrite/	FALSE	0	overwrite	this

+127.0.0.1	FALSE	/overwrite	FALSE	0	overwrite	this2

+127.0.0.1	FALSE	/secure1/	TRUE	0	sec1value	secure1

+127.0.0.1	FALSE	/secure2/	FALSE	0	sec2value	secure2

+127.0.0.1	FALSE	/secure3/	FALSE	0	sec3value	secure3

+127.0.0.1	FALSE	/secure4/	FALSE	0	sec4value	secure4

+127.0.0.1	FALSE	/secure5/	TRUE	0	sec5value	secure5

+127.0.0.1	FALSE	/secure6/	FALSE	0	sec6value	secure6

+127.0.0.1	FALSE	/secure7/	FALSE	0	sec7value	secure7

+127.0.0.1	FALSE	/secure8/	FALSE	0	sec8value	secure8

+127.0.0.1	FALSE	/secure9/	FALSE	0	secure	very1

+#HttpOnly_127.0.0.1	FALSE	/p1/	FALSE	0	httpo1	value1

+127.0.0.1	FALSE	/p2/	FALSE	0	httpo2	value2

+#HttpOnly_127.0.0.1	FALSE	/p3/	FALSE	0	httpo3	value3

+127.0.0.1	FALSE	/p4/	FALSE	0	httpo4	value4

+#HttpOnly_127.0.0.1	FALSE	/p4/	FALSE	0	httponly	myvalue1

+#HttpOnly_127.0.0.1	FALSE	/p4/	TRUE	0	httpandsec	myvalue2

+127.0.0.1	FALSE	/p4/	TRUE	0	httpandsec2	myvalue3

+#HttpOnly_127.0.0.1	FALSE	/p4/	FALSE	0	httpandsec3	myvalue4

+127.0.0.1	FALSE	/p4/	FALSE	0	httpandsec4	myvalue5

+127.0.0.1	FALSE	/p4/	TRUE	0	httpandsec5	myvalue6

+127.0.0.1	FALSE	/p4/	FALSE	0	httpandsec6	myvalue7

+#HttpOnly_127.0.0.1	FALSE	/p4/	TRUE	0	httpandsec7	myvalue8

+#HttpOnly_127.0.0.1	FALSE	/p4/	FALSE	0	httpandsec8	myvalue9

+127.0.0.1	FALSE	/	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

-.0.0.1	TRUE	/we/want/	FALSE	0	blexp	yesyes

+127.0.0.1	FALSE	/we/want/	FALSE	0	blexp	yesyes

 </file>

 </verify>

 </testcase>

--- tests/data/test8.orig	2014-09-04 10:27:04.885231628 -0700

+++ tests/data/test8	2014-09-04 10:41:44.133914601 -0700

@@ -35,16 +35,20 @@

 Server: test-server/fake

 Content-Type: text/html

 Funny-head: yesyes

-Set-Cookie: foobar=name; domain=127.0.0.1; path=/;

-Set-Cookie: mismatch=this; domain=127.0.0.1; path="/silly/";

+Set-Cookie: foobar=name; domain=%HOSTIP; path=/;

+Set-Cookie: mismatch=this; domain=%HOSTIP; path="/silly/";

 Set-Cookie: partmatch=present; domain=.0.0.1; path=/w;

 Set-Cookie: duplicate=test; domain=.0.0.1; domain=.0.0.1; path=/donkey;

 Set-Cookie: cookie=yes; path=/we;

 Set-Cookie: cookie=perhaps; path=/we/want;

 Set-Cookie: nocookie=yes; path=/WE;

-Set-Cookie: blexp=yesyes; domain=.0.0.1; domain=.0.0.1; expiry=totally bad;

+Set-Cookie: blexp=yesyes; domain=%HOSTIP; domain=%HOSTIP; expiry=totally bad;

+Set-Cookie: partialip=nono; domain=.0.0.1;

 </file>

+<precheck>

+perl -e 'if ("%HOSTIP" !~ /\.0\.0\.1$/) {print "Test only works for HOSTIPs ending with .0.0.1"; exit(1)}'

+</precheck>

 </client>

 # Verify data after the test has been "shot"

@@ -56,7 +60,7 @@

 GET /we/want/8 HTTP/1.1

 Host: %HOSTIP:%HTTPPORT

 Accept: */*

-Cookie: cookie=perhaps; cookie=yes; partmatch=present; foobar=name; blexp=yesyes

+Cookie: cookie=perhaps; cookie=yes; foobar=name; blexp=yesyes

 </protocol>

 </verify>