Issue #1546 - more cookie leniency fixes

This commit is contained in:
Greg Wilkins 2017-05-15 21:01:15 +02:00
parent c5a0c5e761
commit 166736db55
3 changed files with 50 additions and 44 deletions

View File

@ -126,8 +126,8 @@ public class CookieCutter
Cookie cookie = null;
boolean invalue=false;
boolean inQuoted=false;
boolean quoted=false;
boolean unquotedToken=false;
boolean escaped=false;
int tokenstart=-1;
int tokenend=-1;
@ -135,8 +135,10 @@ public class CookieCutter
{
char c = hdr.charAt(i);
// System.err.printf("i=%d c=%s v=%b q=%b e=%b u=%s s=%d e=%d%n" ,i,""+c,invalue,inQuoted,escaped,unquoted,tokenstart,tokenend);
// Handle quoted values for name or value
if (quoted)
if (inQuoted)
{
if (escaped)
{
@ -148,25 +150,43 @@ public class CookieCutter
switch (c)
{
case '"':
quoted=false;
inQuoted=false;
if (i==last)
{
value = unquoted.toString();
}
else
{
unquotedToken=true;
quoted=true;
tokenstart=i;
tokenend=-1;
}
break;
case '\\':
if (i==last)
{
unquoted.setLength(0);
inQuoted = false;
i--;
}
else
{
escaped=true;
}
continue;
default:
if (i==last)
{
unquoted.setLength(0);
inQuoted = false;
i--;
}
else
{
unquoted.append(c);
}
continue;
}
}
@ -183,16 +203,14 @@ public class CookieCutter
continue;
case ';':
if (unquotedToken)
if (quoted)
{
value = unquoted.toString();
unquoted.setLength(0);
unquotedToken = false;
quoted = false;
}
else if(tokenstart>=0 && tokenend>=0)
value = hdr.substring(tokenstart, tokenend+1);
else
value="";
tokenstart = -1;
invalue=false;
@ -201,7 +219,8 @@ public class CookieCutter
case '"':
if (tokenstart<0)
{
quoted=true;
tokenstart=i;
inQuoted=true;
if (unquoted==null)
unquoted=new StringBuilder();
continue;
@ -209,12 +228,12 @@ public class CookieCutter
// fall through to default case
default:
if (unquotedToken)
if (quoted)
{
// must have been a bad internal quote. let's fix as best we can
unquoted.append(hdr.substring(tokenstart,i));
quoted = true;
unquotedToken = false;
inQuoted = true;
quoted = false;
i--;
continue;
}
@ -239,27 +258,26 @@ public class CookieCutter
continue;
case ';':
if (unquotedToken)
if (quoted)
{
name = unquoted.toString();
unquoted.setLength(0);
unquotedToken = false;
quoted = false;
}
else if(tokenstart>=0 && tokenend>=0)
{
name = hdr.substring(tokenstart, tokenend+1);
}
value = "";
tokenstart = -1;
break;
case '=':
if (unquotedToken)
if (quoted)
{
name = unquoted.toString();
unquoted.setLength(0);
unquotedToken = false;
quoted = false;
}
else if(tokenstart>=0 && tokenend>=0)
{
@ -271,12 +289,12 @@ public class CookieCutter
continue;
default:
if (unquotedToken)
if (quoted)
{
// must have been a bad internal quote. let's fix as best we can
unquoted.append(hdr.substring(tokenstart,i));
quoted = true;
unquotedToken = false;
inQuoted = true;
quoted = false;
i--;
continue;
}
@ -286,7 +304,6 @@ public class CookieCutter
if (i==last)
{
name = hdr.substring(tokenstart, tokenend+1);
value = "";
break;
}
continue;

View File

@ -193,21 +193,4 @@ public class CookieCutterTest
assertThat("Cookies.length", cookies.length, is(1));
assertCookie("Cookies[0]", cookies[0], "key", "value", 0, null);
}
/**
* Multiple name=value, heavy abuse, badly terminated quotes, lenient behavior test
*/
@Test
public void testMultiName_BadQuoteTerminate()
{
// TODO: this seems very hokey, and allowing this as 3 separate entries is probably a security issue.
String rawCookie = "a=\"b; $Path=/a; c=d; $PATH=/c; e=f\"; $Path=/e/";
Cookie cookies[] = parseCookieHeaders(rawCookie);
assertThat("Cookies.length", cookies.length, is(3));
assertCookie("Cookies[0]", cookies[0], "a", "\"b", 0, "/a");
assertCookie("Cookies[1]", cookies[1], "c", "d", 0, "/c");
assertCookie("Cookies[2]", cookies[2], "e", "f\"", 0, "/e/");
}
}

View File

@ -64,6 +64,8 @@ public class CookieCutter_LenientTest
// RFC2109 - quoted-string values
// quoted-string = ( <"> *(qdtext) <"> )
// qdtext = <any TEXT except <">>
ret.add(new String[]{"abc=\"\"", "abc", ""});
// The backslash character ("\") may be used as a single-character quoting
// mechanism only within quoted-string and comment constructs.
// quoted-pair = "\" CHAR
@ -98,7 +100,9 @@ public class CookieCutter_LenientTest
// Unterminated Quotes
ret.add(new String[]{"x=\"abc", "x", "\"abc"});
// Unterminated Quotes with valid cookie params after it
ret.add(new String[]{"x=\"abc $Path=/", "x", "\"abc"});
ret.add(new String[]{"x=\"abc $Path=/", "x", "\"abc $Path=/"});
// Unterminated Quotes with trailing escape
ret.add(new String[]{"x=\"abc\\", "x", "\"abc\\"});
// UTF-8 values
ret.add(new String[]{"2sides=\u262F", "2sides", "\u262f"}); // 2 byte
@ -111,9 +115,9 @@ public class CookieCutter_LenientTest
ret.add(new String[]{"z=a b c d e f g", "z", "a b c d e f g"});
// Bad tspecials usage
ret.add(new String[]{"foo=bar;baz", "foo", "bar;baz"}); // TODO: not sure supporting this is sane
ret.add(new String[]{"foo=bar;baz", "foo", "bar"});
ret.add(new String[]{"foo=\"bar;baz\"", "foo", "bar;baz"});
ret.add(new String[]{"z=a;b,c:d;e/f[g]", "z", "a;b,c:d;e/f[g]"});
ret.add(new String[]{"z=a;b,c:d;e/f[g]", "z", "a"});
ret.add(new String[]{"z=\"a;b,c:d;e/f[g]\"", "z", "a;b,c:d;e/f[g]"});
// Quoted with other Cookie keywords
@ -121,6 +125,7 @@ public class CookieCutter_LenientTest
ret.add(new String[]{"x=\"$Path=/\"", "x", "$Path=/"});
ret.add(new String[]{"x=\"$Path=/ $Domain=.foo.com\"", "x", "$Path=/ $Domain=.foo.com"});
ret.add(new String[]{"x=\" $Path=/ $Domain=.foo.com \"", "x", " $Path=/ $Domain=.foo.com "});
ret.add(new String[]{"a=\"b; $Path=/a; c=d; $PATH=/c; e=f\"; $Path=/e/", "a", "b; $Path=/a; c=d; $PATH=/c; e=f"});
// Lots of equals signs
ret.add(new String[]{"query=b=c&d=e", "query", "b=c&d=e"});
@ -161,4 +166,5 @@ public class CookieCutter_LenientTest
assertThat("Cookie.value", cookies[0].getValue(), is(expectedValue));
}
}
}