fixed possible (low-severity) CSRF involving space cookies by using S…

…ameSite=Strict flag
Erudika · Dec 30, 2021 · ee59dd9 · ee59dd9
1 parent c251891
commit ee59dd9
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 28 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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) {