From ee59dd919c89accc6f4362737e3ed5968f71a205 Mon Sep 17 00:00:00 2001 From: Alex Bogdanovski Date: Thu, 30 Dec 2021 13:55:26 +0200 Subject: [PATCH] fixed possible (low-severity) CSRF involving space cookies by using SameSite=Strict flag --- .../controllers/LanguagesController.java | 2 +- .../controllers/QuestionsController.java | 4 +- .../com/erudika/scoold/utils/HttpUtils.java | 47 +++++++++---------- .../com/erudika/scoold/utils/ScooldUtils.java | 2 +- 4 files changed, 27 insertions(+), 28 deletions(-) diff --git a/src/main/java/com/erudika/scoold/controllers/LanguagesController.java b/src/main/java/com/erudika/scoold/controllers/LanguagesController.java index 89f34136..c1269a68 100755 --- a/src/main/java/com/erudika/scoold/controllers/LanguagesController.java +++ b/src/main/java/com/erudika/scoold/controllers/LanguagesController.java @@ -62,7 +62,7 @@ public String post(@PathVariable String langkey, HttpServletRequest req, HttpSer Locale locale = utils.getCurrentLocale(langkey); if (locale != null) { int maxAge = 60 * 60 * 24 * 365; //1 year - HttpUtils.setRawCookie(LOCALE_COOKIE, locale.toString(), req, res, false, maxAge); + HttpUtils.setRawCookie(LOCALE_COOKIE, locale.toString(), req, res, false, "Strict", maxAge); } return "redirect:" + LANGUAGESLINK; } diff --git a/src/main/java/com/erudika/scoold/controllers/QuestionsController.java b/src/main/java/com/erudika/scoold/controllers/QuestionsController.java index 60b3f2ea..fdcf42a0 100755 --- a/src/main/java/com/erudika/scoold/controllers/QuestionsController.java +++ b/src/main/java/com/erudika/scoold/controllers/QuestionsController.java @@ -172,7 +172,7 @@ public String applyFilter(@RequestParam(required = false) String sortby, @Reques } savePagerToCookie(req, res, p); HttpUtils.setRawCookie("questions-view-compact", compactViewEnabled, - req, res, false, (int) TimeUnit.DAYS.toSeconds(365)); + req, res, false, "Strict", (int) TimeUnit.DAYS.toSeconds(365)); } return "redirect:" + QUESTIONSLINK + (StringUtils.isBlank(sortby) ? "" : "?sortby=" + Optional.ofNullable(StringUtils.trimToNull(tab)).orElse(sortby)); @@ -403,7 +403,7 @@ public String getName() { private void savePagerToCookie(HttpServletRequest req, HttpServletResponse res, Pager p) { try { HttpUtils.setRawCookie("questions-filter", Utils.base64enc(ParaObjectUtils.getJsonWriterNoIdent(). - writeValueAsBytes(p)), req, res, false, (int) TimeUnit.DAYS.toSeconds(365)); + writeValueAsBytes(p)), req, res, false, "Strict", (int) TimeUnit.DAYS.toSeconds(365)); } catch (JsonProcessingException ex) { } } diff --git a/src/main/java/com/erudika/scoold/utils/HttpUtils.java b/src/main/java/com/erudika/scoold/utils/HttpUtils.java index 95627a0e..b1fb52ea 100644 --- a/src/main/java/com/erudika/scoold/utils/HttpUtils.java +++ b/src/main/java/com/erudika/scoold/utils/HttpUtils.java @@ -124,7 +124,7 @@ public static void setStateParam(String name, String value, HttpServletRequest r */ public static void setStateParam(String name, String value, HttpServletRequest req, HttpServletResponse res, boolean httpOnly) { - setRawCookie(name, value, req, res, httpOnly, -1); + setRawCookie(name, value, req, res, httpOnly, null, -1); } /** @@ -145,7 +145,7 @@ public static String getStateParam(String name, HttpServletRequest req) { */ public static void removeStateParam(String name, HttpServletRequest req, HttpServletResponse res) { - setRawCookie(name, "", req, res, false, 0); + setRawCookie(name, "", req, res, false, null, 0); } /** @@ -155,19 +155,32 @@ public static void removeStateParam(String name, HttpServletRequest req, * @param req HTTP request * @param res HTTP response * @param httpOnly HTTP only flag + * @param sameSite SameSite flag * @param maxAge max age */ public static void setRawCookie(String name, String value, HttpServletRequest req, - HttpServletResponse res, boolean httpOnly, int maxAge) { + HttpServletResponse res, boolean httpOnly, String sameSite, int maxAge) { if (StringUtils.isBlank(name) || value == null || req == null || res == null) { return; } - Cookie cookie = new Cookie(name, value); - cookie.setHttpOnly(httpOnly); - cookie.setMaxAge(maxAge < 0 ? Config.SESSION_TIMEOUT_SEC : maxAge); - cookie.setPath(CONTEXT_PATH.isEmpty() ? "/" : CONTEXT_PATH); - cookie.setSecure(StringUtils.startsWithIgnoreCase(ScooldServer.getServerURL(), "https://") || req.isSecure()); - res.addCookie(cookie); + String expires = DateFormatUtils.format(System.currentTimeMillis() + (maxAge * 1000), + "EEE, dd-MMM-yyyy HH:mm:ss z", TimeZone.getTimeZone("GMT")); + String path = CONTEXT_PATH.isEmpty() ? "/" : CONTEXT_PATH; + StringBuilder sb = new StringBuilder(); + sb.append(name).append("=").append(value).append(";"); + sb.append("Path=").append(path).append(";"); + sb.append("Expires=").append(expires).append(";"); + sb.append("Max-Age=").append(maxAge < 0 ? Config.SESSION_TIMEOUT_SEC : maxAge).append(";"); + if (httpOnly) { + sb.append("HttpOnly;"); + } + if (StringUtils.startsWithIgnoreCase(ScooldServer.getServerURL(), "https://") || req.isSecure()) { + sb.append("Secure;"); + } + if (!StringUtils.isBlank(sameSite)) { + sb.append("SameSite=").append(sameSite); + } + res.addHeader(javax.ws.rs.core.HttpHeaders.SET_COOKIE, sb.toString()); } /** @@ -291,21 +304,7 @@ public static void setAuthCookie(String jwt, HttpServletRequest req, HttpServlet if (StringUtils.isBlank(jwt)) { return; } - int maxAge = Config.SESSION_TIMEOUT_SEC; - String expires = DateFormatUtils.format(System.currentTimeMillis() + (maxAge * 1000), - "EEE, dd-MMM-yyyy HH:mm:ss z", TimeZone.getTimeZone("GMT")); - String path = CONTEXT_PATH.isEmpty() ? "/" : CONTEXT_PATH; - StringBuilder sb = new StringBuilder(); - sb.append(AUTH_COOKIE).append("=").append(jwt).append(";"); - sb.append("Path=").append(path).append(";"); - sb.append("Expires=").append(expires).append(";"); - sb.append("Max-Age=").append(maxAge).append(";"); - sb.append("HttpOnly;"); - if (StringUtils.startsWithIgnoreCase(ScooldServer.getServerURL(), "https://") || req.isSecure()) { - sb.append("Secure;"); - } - sb.append("SameSite=Lax"); - res.addHeader(javax.ws.rs.core.HttpHeaders.SET_COOKIE, sb.toString()); + setRawCookie(AUTH_COOKIE, jwt, req, res, true, "Lax", Config.SESSION_TIMEOUT_SEC); } /** diff --git a/src/main/java/com/erudika/scoold/utils/ScooldUtils.java b/src/main/java/com/erudika/scoold/utils/ScooldUtils.java index 1281afac..ad4a12c1 100755 --- a/src/main/java/com/erudika/scoold/utils/ScooldUtils.java +++ b/src/main/java/com/erudika/scoold/utils/ScooldUtils.java @@ -1192,7 +1192,7 @@ public void storeSpaceIdInCookie(String space, HttpServletRequest req, HttpServl // used for setting the space from a direct URL to a particular space req.setAttribute(SPACE_COOKIE, space); HttpUtils.setRawCookie(SPACE_COOKIE, Utils.base64encURL(space.getBytes()), - req, res, false, StringUtils.isBlank(space) ? 0 : 365 * 24 * 60 * 60); + req, res, true, "Strict", StringUtils.isBlank(space) ? 0 : 365 * 24 * 60 * 60); } public String verifyExistingSpace(Profile authUser, String space) {