Issue #3985 - Updates to CookieCutter to reject no-equal cookies

* If a cookie has no value it is rejected and not stored.
  - `name` is rejected
  - `name=` is accepted, with empty value

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
This commit is contained in:
Joakim Erdfelt 2019-08-16 13:11:40 -05:00
parent eaf2263053
commit fba010d33d
2 changed files with 55 additions and 13 deletions

View File

@ -144,14 +144,13 @@ public class CookieCutter
boolean inQuoted = false;
boolean quoted = false;
boolean escaped = false;
boolean reject = false;
int tokenstart = -1;
int tokenend = -1;
for (int i = 0, length = hdr.length(); i <= length; i++)
{
char c = i == length ? 0 : hdr.charAt(i);
// System.err.printf("i=%d/%d c=%s v=%b q=%b/%b e=%b u=%s s=%d e=%d \t%s=%s%n" ,i,length,c==0?"|":(""+c),invalue,inQuoted,quoted,escaped,unquoted,tokenstart,tokenend,name,value);
// Handle quoted values for name or value
if (inQuoted)
{
@ -199,7 +198,7 @@ public class CookieCutter
// Handle name and value state machines
if (invalue)
{
// parse the value
// parse the cookie-value
switch (c)
{
case ' ':
@ -273,7 +272,10 @@ public class CookieCutter
cookie = new Cookie(name, value);
if (version > 0)
cookie.setVersion(version);
cookies.add(cookie);
if (!reject)
{
cookies.add(cookie);
}
}
}
catch (Exception e)
@ -284,6 +286,7 @@ public class CookieCutter
name = null;
tokenstart = -1;
invalue = false;
reject = false;
break;
}
@ -308,6 +311,19 @@ public class CookieCutter
quoted = false;
continue;
}
if (_compliance == CookieCompliance.RFC6265)
{
// Rejected cookie-octet characters
// US-ASCII characters excluding CTLs,
// whitespace DQUOTE, comma, semicolon,
// and backslash
if (Character.isISOControl(c) || Character.isWhitespace(c) || c == '\\')
{
reject = true;
}
}
if (tokenstart < 0)
tokenstart = i;
tokenend = i;
@ -316,13 +332,20 @@ public class CookieCutter
}
else
{
// parse the name
// parse the cookie-name
switch (c)
{
case ' ':
case '\t':
continue;
case ';':
// a cookie terminated with no '=' sign.
tokenstart = -1;
invalue = false;
reject = false;
continue;
case '=':
if (quoted)
{
@ -346,6 +369,20 @@ public class CookieCutter
quoted = false;
continue;
}
if (_compliance == CookieCompliance.RFC6265)
{
// Rejected cookie-octet characters
// US-ASCII characters excluding CTLs,
// whitespace DQUOTE, comma, semicolon,
// and backslash
if (Character.isISOControl(c) || Character.isWhitespace(c) ||
c == ',' || c == '\\')
{
reject = true;
}
}
if (tokenstart < 0)
tokenstart = i;
tokenend = i;
@ -356,7 +393,7 @@ public class CookieCutter
}
}
_cookies = cookies.toArray(new Cookie[cookies.size()]);
_cookies = cookies.toArray(new Cookie[0]);
_lastCookies = _cookies;
}
}

View File

@ -60,12 +60,17 @@ public class CookieCutter_LenientTest
// quoted-string = ( <"> *(qdtext) <"> )
// qdtext = <any TEXT except <">>
// rejected, as name cannot be DQUOTED
Arguments.of("\"a\"=bcd", null, null),
Arguments.of("\"a\"=\"b c d\"", null, null),
// lenient with spaces and EOF
Arguments.of("abc=", "abc", ""),
Arguments.of("abc = ", "abc", ""),
Arguments.of("abc = ;", "abc", ""),
Arguments.of("abc = ; ", "abc", ""),
Arguments.of("abc = x ", "abc", "x"),
Arguments.of("abc = e f g ", "abc", "e f g"),
Arguments.of("abc=\"\"", "abc", ""),
Arguments.of("abc= \"\" ", "abc", ""),
Arguments.of("abc= \"x\" ", "abc", "x"),
@ -110,17 +115,17 @@ public class CookieCutter_LenientTest
// Unterminated Quotes with trailing escape
Arguments.of("x=\"abc\\", "x", "\"abc\\"),
// UTF-8 values
Arguments.of("2sides=\u262F", "2sides", "\u262f"), // 2 byte
Arguments.of("currency=\"\u20AC\"", "currency", "\u20AC"), // 3 byte
Arguments.of("gothic=\"\uD800\uDF48\"", "gothic", "\uD800\uDF48"), // 4 byte
// UTF-8 raw values (not encoded) - VIOLATION of RFC6265
Arguments.of("2sides=\u262F", "2sides", "\u262f"), // 2 byte (YIN YANG)
Arguments.of("currency=\"\u20AC\"", "currency", "\u20AC"), // 3 byte (EURO SIGN)
Arguments.of("gothic=\"\uD800\uDF48\"", "gothic", "\uD800\uDF48"), // 4 byte (GOTHIC LETTER HWAIR)
// Spaces
Arguments.of("foo=bar baz", "foo", "bar baz"),
Arguments.of("foo=\"bar baz\"", "foo", "bar baz"),
Arguments.of("z=a b c d e f g", "z", "a b c d e f g"),
// Bad tspecials usage
// Bad tspecials usage - VIOLATION of RFC6265
Arguments.of("foo=bar;baz", "foo", "bar"),
Arguments.of("foo=\"bar;baz\"", "foo", "bar;baz"),
Arguments.of("z=a;b,c:d;e/f[g]", "z", "a"),
@ -131,7 +136,7 @@ public class CookieCutter_LenientTest
Arguments.of("x=\"$Path=/\"", "x", "$Path=/"),
Arguments.of("x=\"$Path=/ $Domain=.foo.com\"", "x", "$Path=/ $Domain=.foo.com"),
Arguments.of("x=\" $Path=/ $Domain=.foo.com \"", "x", " $Path=/ $Domain=.foo.com "),
Arguments.of("a=\"b; $Path=/a; c=d; $PATH=/c; e=f\"; $Path=/e/", "a", "b; $Path=/a; c=d; $PATH=/c; e=f"),
Arguments.of("a=\"b; $Path=/a; c=d; $PATH=/c; e=f\"; $Path=/e/", "a", "b; $Path=/a; c=d; $PATH=/c; e=f"), // VIOLATES RFC6265
// Lots of equals signs
Arguments.of("query=b=c&d=e", "query", "b=c&d=e"),
@ -142,7 +147,7 @@ public class CookieCutter_LenientTest
// Google cookies (seen in wild, has `tspecials` of ':' in value)
Arguments.of("GAPS=1:A1aaaAaAA1aaAAAaa1a11a:aAaaAa-aaA1-", "GAPS", "1:A1aaaAaAA1aaAAAaa1a11a:aAaaAa-aaA1-"),
// Strong abuse of cookie spec (lots of tspecials)
// Strong abuse of cookie spec (lots of tspecials) - VIOLATION of RFC6265
Arguments.of("$Version=0; rToken=F_TOKEN''!--\"</a>=&{()}", "rToken", "F_TOKEN''!--\"</a>=&{()}"),
// Commas that were not commas