From 2a125e17ad8f61dbe2b4fbc5df50e24abf8afc93 Mon Sep 17 00:00:00 2001 From: delta456 Date: Wed, 11 Sep 2024 16:45:47 +0530 Subject: [PATCH 1/5] java/cookie: escape cookie values when required --- .../environment/webserver/CookieHandler.java | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/java/test/org/openqa/selenium/environment/webserver/CookieHandler.java b/java/test/org/openqa/selenium/environment/webserver/CookieHandler.java index a6abb8ca9c10a..2cbd3daf8d4b9 100644 --- a/java/test/org/openqa/selenium/environment/webserver/CookieHandler.java +++ b/java/test/org/openqa/selenium/environment/webserver/CookieHandler.java @@ -116,9 +116,9 @@ private Collection getCookies(HttpRequest request) { private void addCookie(HttpResponse response, Cookie cook) { StringBuilder cookie = new StringBuilder(); - // TODO: escape string as necessary - String name = cook.getName(); - cookie.append(name).append("=").append(cook.getValue()).append("; "); + String name = escapeCookieValue(cook.getName()); + String value = escapeCookieValue(cook.getValue()); + cookie.append(name).append("=").append(value).append("; "); append(cookie, cook.getDomain(), str -> "Domain=" + str); append(cookie, cook.getPath(), str -> "Path=" + str); @@ -191,4 +191,16 @@ private Cookie parse(String cookieString) { return builder.build(); } + private String escapeCookieValue(String value) { + if (value == null) { + return ""; + } + + return value.replace("\\", "\\\\") + .replace("\"", "\\\"") + .replace(";", "\\;") + .replace(",", "\\,") + .replace("\r", "") + .replace("\n", ""); + } } From f45a4b6a151b67bd7c4fa29ce94a0ebfcc349df6 Mon Sep 17 00:00:00 2001 From: delta456 Date: Wed, 11 Sep 2024 22:08:00 +0530 Subject: [PATCH 2/5] add xss escaping --- .../selenium/environment/webserver/CookieHandler.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/java/test/org/openqa/selenium/environment/webserver/CookieHandler.java b/java/test/org/openqa/selenium/environment/webserver/CookieHandler.java index 2cbd3daf8d4b9..8514781951cf5 100644 --- a/java/test/org/openqa/selenium/environment/webserver/CookieHandler.java +++ b/java/test/org/openqa/selenium/environment/webserver/CookieHandler.java @@ -192,7 +192,7 @@ private Cookie parse(String cookieString) { return builder.build(); } private String escapeCookieValue(String value) { - if (value == null) { + if (value == null || value.isEmpty()) { return ""; } @@ -201,6 +201,9 @@ private String escapeCookieValue(String value) { .replace(";", "\\;") .replace(",", "\\,") .replace("\r", "") - .replace("\n", ""); + .replace("\n", "") + .replace("<", "<") + .replace(">", ">") + .replace("&", "&"); } } From c8167f56634ad5e0bcd8ba10092dab0893c12fb0 Mon Sep 17 00:00:00 2001 From: Puja Jagani Date: Mon, 16 Sep 2024 10:57:44 +0530 Subject: [PATCH 3/5] Fix formatting --- .../environment/webserver/CookieHandler.java | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/java/test/org/openqa/selenium/environment/webserver/CookieHandler.java b/java/test/org/openqa/selenium/environment/webserver/CookieHandler.java index 8514781951cf5..47152ed6d0380 100644 --- a/java/test/org/openqa/selenium/environment/webserver/CookieHandler.java +++ b/java/test/org/openqa/selenium/environment/webserver/CookieHandler.java @@ -191,19 +191,21 @@ private Cookie parse(String cookieString) { return builder.build(); } + private String escapeCookieValue(String value) { if (value == null || value.isEmpty()) { return ""; } - return value.replace("\\", "\\\\") - .replace("\"", "\\\"") - .replace(";", "\\;") - .replace(",", "\\,") - .replace("\r", "") - .replace("\n", "") - .replace("<", "<") - .replace(">", ">") - .replace("&", "&"); + return value + .replace("\\", "\\\\") + .replace("\"", "\\\"") + .replace(";", "\\;") + .replace(",", "\\,") + .replace("\r", "") + .replace("\n", "") + .replace("<", "<") + .replace(">", ">") + .replace("&", "&"); } } From 1146b4c475ed7277c0ec7e8e94240f36d9b1890c Mon Sep 17 00:00:00 2001 From: delta456 Date: Mon, 16 Sep 2024 14:36:52 +0530 Subject: [PATCH 4/5] use string builder --- .../environment/webserver/CookieHandler.java | 45 ++++++++++++++----- 1 file changed, 34 insertions(+), 11 deletions(-) diff --git a/java/test/org/openqa/selenium/environment/webserver/CookieHandler.java b/java/test/org/openqa/selenium/environment/webserver/CookieHandler.java index 47152ed6d0380..7f6b55fc01532 100644 --- a/java/test/org/openqa/selenium/environment/webserver/CookieHandler.java +++ b/java/test/org/openqa/selenium/environment/webserver/CookieHandler.java @@ -191,21 +191,44 @@ private Cookie parse(String cookieString) { return builder.build(); } - private String escapeCookieValue(String value) { if (value == null || value.isEmpty()) { return ""; } - return value - .replace("\\", "\\\\") - .replace("\"", "\\\"") - .replace(";", "\\;") - .replace(",", "\\,") - .replace("\r", "") - .replace("\n", "") - .replace("<", "<") - .replace(">", ">") - .replace("&", "&"); + StringBuilder cookieValue = new StringBuilder(); + + for (char c : value.toCharArray()) { + switch (c) { + case '\\': + cookieValue.append("\\\\"); + break; + case '"': + cookieValue.append("\\\""); + break; + case ';': + cookieValue.append("\\;"); + break; + case ',': + cookieValue.append("\\,"); + break; + case '\r': + case '\n': + // Skip carriage return and newline characters + break; + case '<': + cookieValue.append("<"); + break; + case '>': + cookieValue.append(">"); + break; + case '&': + cookieValue.append("&"); + break; + default: + cookieValue.append(c); // Append safe characters as they are + } + } + return cookieValue.toString(); } } From b5d379780ecb4c93ca8d6e964fc07fe6f00f86a7 Mon Sep 17 00:00:00 2001 From: delta456 Date: Mon, 16 Sep 2024 17:21:37 +0530 Subject: [PATCH 5/5] fmt --- .../org/openqa/selenium/environment/webserver/CookieHandler.java | 1 + 1 file changed, 1 insertion(+) diff --git a/java/test/org/openqa/selenium/environment/webserver/CookieHandler.java b/java/test/org/openqa/selenium/environment/webserver/CookieHandler.java index 7f6b55fc01532..b28d09bb15072 100644 --- a/java/test/org/openqa/selenium/environment/webserver/CookieHandler.java +++ b/java/test/org/openqa/selenium/environment/webserver/CookieHandler.java @@ -191,6 +191,7 @@ private Cookie parse(String cookieString) { return builder.build(); } + private String escapeCookieValue(String value) { if (value == null || value.isEmpty()) { return "";