From fba010d33d2302ec2b7356da16a6bb71a1c11fdf Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Fri, 16 Aug 2019 13:11:40 -0500 Subject: [PATCH] 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 --- .../eclipse/jetty/server/CookieCutter.java | 49 ++++++++++++++++--- .../server/CookieCutter_LenientTest.java | 19 ++++--- 2 files changed, 55 insertions(+), 13 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/CookieCutter.java b/jetty-server/src/main/java/org/eclipse/jetty/server/CookieCutter.java index 8248795b14d..148e55ebe9c 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/CookieCutter.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/CookieCutter.java @@ -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; } } diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/CookieCutter_LenientTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/CookieCutter_LenientTest.java index 9dbafbe8ad6..6e3996679a8 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/CookieCutter_LenientTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/CookieCutter_LenientTest.java @@ -60,12 +60,17 @@ public class CookieCutter_LenientTest // quoted-string = ( <"> *(qdtext) <"> ) // qdtext = > + // 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''!--\"=&{()}", "rToken", "F_TOKEN''!--\"=&{()}"), // Commas that were not commas