From 54aefe0dce9012b3dba28b828ea2d3b5d35023d2 Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Mon, 29 Sep 2025 15:14:28 +0100 Subject: [PATCH 01/10] Copy experimental query to main --- .../CWE-1004/SensitiveCookieNotHttpOnly.java | 44 ++++ .../CWE-1004/SensitiveCookieNotHttpOnly.qhelp | 27 +++ .../CWE-1004/SensitiveCookieNotHttpOnly.ql | 223 ++++++++++++++++++ 3 files changed, 294 insertions(+) create mode 100644 java/ql/src/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.java create mode 100644 java/ql/src/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.qhelp create mode 100644 java/ql/src/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql diff --git a/java/ql/src/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.java b/java/ql/src/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.java new file mode 100644 index 000000000000..48d80707ff83 --- /dev/null +++ b/java/ql/src/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.java @@ -0,0 +1,44 @@ +class SensitiveCookieNotHttpOnly { + // GOOD - Create a sensitive cookie with the `HttpOnly` flag set. + public void addCookie(String jwt_token, HttpServletRequest request, HttpServletResponse response) { + Cookie jwtCookie =new Cookie("jwt_token", jwt_token); + jwtCookie.setPath("/"); + jwtCookie.setMaxAge(3600*24*7); + jwtCookie.setHttpOnly(true); + response.addCookie(jwtCookie); + } + + // BAD - Create a sensitive cookie without the `HttpOnly` flag set. + public void addCookie2(String jwt_token, String userId, HttpServletRequest request, HttpServletResponse response) { + Cookie jwtCookie =new Cookie("jwt_token", jwt_token); + jwtCookie.setPath("/"); + jwtCookie.setMaxAge(3600*24*7); + response.addCookie(jwtCookie); + } + + // GOOD - Set a sensitive cookie header with the `HttpOnly` flag set. + public void addCookie3(String authId, HttpServletRequest request, HttpServletResponse response) { + response.addHeader("Set-Cookie", "token=" +authId + ";HttpOnly;Secure"); + } + + // BAD - Set a sensitive cookie header without the `HttpOnly` flag set. + public void addCookie4(String authId, HttpServletRequest request, HttpServletResponse response) { + response.addHeader("Set-Cookie", "token=" +authId + ";Secure"); + } + + // GOOD - Set a sensitive cookie header using the class `javax.ws.rs.core.Cookie` with the `HttpOnly` flag set through string concatenation. + public void addCookie5(String accessKey, HttpServletRequest request, HttpServletResponse response) { + response.setHeader("Set-Cookie", new NewCookie("session-access-key", accessKey, "/", null, null, 0, true) + ";HttpOnly"); + } + + // BAD - Set a sensitive cookie header using the class `javax.ws.rs.core.Cookie` without the `HttpOnly` flag set. + public void addCookie6(String accessKey, HttpServletRequest request, HttpServletResponse response) { + response.setHeader("Set-Cookie", new NewCookie("session-access-key", accessKey, "/", null, null, 0, true).toString()); + } + + // GOOD - Set a sensitive cookie header using the class `javax.ws.rs.core.Cookie` with the `HttpOnly` flag set through the constructor. + public void addCookie7(String accessKey, HttpServletRequest request, HttpServletResponse response) { + NewCookie accessKeyCookie = new NewCookie("session-access-key", accessKey, "/", null, null, 0, true, true); + response.setHeader("Set-Cookie", accessKeyCookie.toString()); + } +} diff --git a/java/ql/src/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.qhelp b/java/ql/src/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.qhelp new file mode 100644 index 000000000000..ee3e8a4181a9 --- /dev/null +++ b/java/ql/src/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.qhelp @@ -0,0 +1,27 @@ + + + + +

Cross-Site Scripting (XSS) is categorized as one of the OWASP Top 10 Security Vulnerabilities. The HttpOnly flag directs compatible browsers to prevent client-side script from accessing cookies. Including the HttpOnly flag in the Set-Cookie HTTP response header for a sensitive cookie helps mitigate the risk associated with XSS where an attacker's script code attempts to read the contents of a cookie and exfiltrate information obtained.

+ + + +

Use the HttpOnly flag when generating a cookie containing sensitive information to help mitigate the risk of client side script accessing the protected cookie.

+ + + +

The following example shows two ways of generating sensitive cookies. In the 'BAD' cases, the HttpOnly flag is not set. In the 'GOOD' cases, the HttpOnly flag is set.

+ + + + +
  • + PortSwigger: + Cookie without HttpOnly flag set +
    • +
  • + OWASP: + HttpOnly +
    • +     +     diff --git a/java/ql/src/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql b/java/ql/src/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql new file mode 100644 index 000000000000..d2d596c23fa3 --- /dev/null +++ b/java/ql/src/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql @@ -0,0 +1,223 @@ +/** + * @name Sensitive cookies without the HttpOnly response header set + * @description Sensitive cookies without the 'HttpOnly' flag set leaves session cookies vulnerable to + * an XSS attack. + * @kind path-problem + * @problem.severity warning + * @precision medium + * @id java/sensitive-cookie-not-httponly + * @tags security + * external/cwe/cwe-1004 + */ + +/* + * Sketch of the structure of this query: we track cookie names that appear to be sensitive + * (e.g. `session` or `token`) to a `ServletResponse.addHeader(...)` or `.addCookie(...)` + * method that does not set the `httpOnly` flag. Subsidiary configurations + * `MatchesHttpOnlyConfiguration` and `SetHttpOnlyInCookieConfiguration` are used to establish + * when the `httpOnly` flag is likely to have been set, before configuration + * `MissingHttpOnlyConfiguration` establishes that a non-`httpOnly` cookie has a sensitive-seeming name. + */ + +import java +import semmle.code.java.dataflow.FlowSteps +import semmle.code.java.frameworks.Servlets +import semmle.code.java.dataflow.TaintTracking +import MissingHttpOnlyFlow::PathGraph + +/** Gets a regular expression for matching common names of sensitive cookies. */ +string getSensitiveCookieNameRegex() { result = "(?i).*(auth|session|token|key|credential).*" } + +/** Gets a regular expression for matching CSRF cookies. */ +string getCsrfCookieNameRegex() { result = "(?i).*(csrf).*" } + +/** + * Holds if a string is concatenated with the name of a sensitive cookie. Excludes CSRF cookies since + * they are special cookies implementing the Synchronizer Token Pattern that can be used in JavaScript. + */ +predicate isSensitiveCookieNameExpr(Expr expr) { + exists(string s | s = expr.(CompileTimeConstantExpr).getStringValue() | + s.regexpMatch(getSensitiveCookieNameRegex()) and not s.regexpMatch(getCsrfCookieNameRegex()) + ) + or + isSensitiveCookieNameExpr(expr.(AddExpr).getAnOperand()) +} + +/** A sensitive cookie name. */ +class SensitiveCookieNameExpr extends Expr { + SensitiveCookieNameExpr() { isSensitiveCookieNameExpr(this) } +} + +/** A method call that sets a `Set-Cookie` header. */ +class SetCookieMethodCall extends MethodCall { + SetCookieMethodCall() { + ( + this.getMethod() instanceof ResponseAddHeaderMethod or + this.getMethod() instanceof ResponseSetHeaderMethod + ) and + this.getArgument(0).(CompileTimeConstantExpr).getStringValue().toLowerCase() = "set-cookie" + } +} + +/** + * A taint configuration tracking flow from the text `httponly` to argument 1 of + * `SetCookieMethodCall`. + */ +module MatchesHttpOnlyConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { + source.asExpr().(CompileTimeConstantExpr).getStringValue().toLowerCase().matches("%httponly%") + } + + predicate isSink(DataFlow::Node sink) { + sink.asExpr() = any(SetCookieMethodCall ma).getArgument(1) + } +} + +module MatchesHttpOnlyFlow = TaintTracking::Global; + +/** A class descended from `javax.servlet.http.Cookie`. */ +class CookieClass extends RefType { + CookieClass() { this.getAnAncestor().hasQualifiedName("javax.servlet.http", "Cookie") } +} + +/** Holds if `expr` is any boolean-typed expression other than literal `false`. */ +// Inlined because this could be a very large result set if computed out of context +pragma[inline] +predicate mayBeBooleanTrue(Expr expr) { + expr.getType() instanceof BooleanType and + not expr.(CompileTimeConstantExpr).getBooleanValue() = false +} + +/** Holds if the method call may set the `HttpOnly` flag. */ +predicate setsCookieHttpOnly(MethodCall ma) { + ma.getMethod().getName() = "setHttpOnly" and + // any use of setHttpOnly(x) where x isn't false is probably safe + mayBeBooleanTrue(ma.getArgument(0)) +} + +/** Holds if `ma` removes a cookie. */ +predicate removesCookie(MethodCall ma) { + ma.getMethod().getName() = "setMaxAge" and + ma.getArgument(0).(IntegerLiteral).getIntValue() = 0 +} + +/** + * Holds if the MethodCall `ma` is a test method call indicated by: + * a) in a test directory such as `src/test/java` + * b) in a test package whose name has the word `test` + * c) in a test class whose name has the word `test` + * d) in a test class implementing a test framework such as JUnit or TestNG + */ +predicate isTestMethod(MethodCall ma) { + exists(Method m | + m = ma.getEnclosingCallable() and + ( + m.getDeclaringType().getName().toLowerCase().matches("%test%") or // Simple check to exclude test classes to reduce FPs + m.getDeclaringType().getPackage().getName().toLowerCase().matches("%test%") or // Simple check to exclude classes in test packages to reduce FPs + exists(m.getLocation().getFile().getAbsolutePath().indexOf("/src/test/java")) or // Match test directory structure of build tools like maven + m instanceof TestMethod // Test method of a test case implementing a test framework such as JUnit or TestNG + ) + ) +} + +/** + * A taint configuration tracking flow of a method that sets the `HttpOnly` flag, + * or one that removes a cookie, to a `ServletResponse.addCookie` call. + */ +module SetHttpOnlyOrRemovesCookieConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { + source.asExpr() = + any(MethodCall ma | setsCookieHttpOnly(ma) or removesCookie(ma)).getQualifier() + } + + predicate isSink(DataFlow::Node sink) { + sink.asExpr() = + any(MethodCall ma | ma.getMethod() instanceof ResponseAddCookieMethod).getArgument(0) + } +} + +module SetHttpOnlyOrRemovesCookieFlow = TaintTracking::Global; + +/** + * A cookie that is added to an HTTP response and which doesn't have `httpOnly` set, used as a sink + * in `MissingHttpOnlyConfiguration`. + */ +class CookieResponseSink extends DataFlow::ExprNode { + CookieResponseSink() { + exists(MethodCall ma | + ( + ma.getMethod() instanceof ResponseAddCookieMethod and + this.getExpr() = ma.getArgument(0) and + not SetHttpOnlyOrRemovesCookieFlow::flowTo(this) + or + ma instanceof SetCookieMethodCall and + this.getExpr() = ma.getArgument(1) and + not MatchesHttpOnlyFlow::flowTo(this) // response.addHeader("Set-Cookie", "token=" +authId + ";HttpOnly;Secure") + ) and + not isTestMethod(ma) // Test class or method + ) + } +} + +/** Holds if `cie` is an invocation of a JAX-RS `NewCookie` constructor that sets `HttpOnly` to true. */ +predicate setsHttpOnlyInNewCookie(ClassInstanceExpr cie) { + cie.getConstructedType().hasQualifiedName(["javax.ws.rs.core", "jakarta.ws.rs.core"], "NewCookie") and + ( + cie.getNumArgument() = 6 and + mayBeBooleanTrue(cie.getArgument(5)) // NewCookie(Cookie cookie, String comment, int maxAge, Date expiry, boolean secure, boolean httpOnly) + or + cie.getNumArgument() = 8 and + cie.getArgument(6).getType() instanceof BooleanType and + mayBeBooleanTrue(cie.getArgument(7)) // NewCookie(String name, String value, String path, String domain, String comment, int maxAge, boolean secure, boolean httpOnly) + or + cie.getNumArgument() = 10 and + mayBeBooleanTrue(cie.getArgument(9)) // NewCookie(String name, String value, String path, String domain, int version, String comment, int maxAge, Date expiry, boolean secure, boolean httpOnly) + ) +} + +/** + * A taint configuration tracking flow from a sensitive cookie without the `HttpOnly` flag + * set to its HTTP response. + */ +module MissingHttpOnlyConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source.asExpr() instanceof SensitiveCookieNameExpr } + + predicate isSink(DataFlow::Node sink) { sink instanceof CookieResponseSink } + + predicate isBarrier(DataFlow::Node node) { + // JAX-RS's `new NewCookie("session-access-key", accessKey, "/", null, null, 0, true, true)` and similar + setsHttpOnlyInNewCookie(node.asExpr()) + } + + predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { + exists( + ConstructorCall cc // new Cookie(...) + | + cc.getConstructedType() instanceof CookieClass and + pred.asExpr() = cc.getAnArgument() and + succ.asExpr() = cc + ) + or + exists( + MethodCall ma // cookie.toString() + | + ma.getMethod().getName() = "toString" and + ma.getQualifier().getType() instanceof CookieClass and + pred.asExpr() = ma.getQualifier() and + succ.asExpr() = ma + ) + } +} + +module MissingHttpOnlyFlow = TaintTracking::Global; + +deprecated query predicate problems( + DataFlow::Node sinkNode, MissingHttpOnlyFlow::PathNode source, MissingHttpOnlyFlow::PathNode sink, + string message1, DataFlow::Node sourceNode, string message2 +) { + MissingHttpOnlyFlow::flowPath(source, sink) and + sinkNode = sink.getNode() and + message1 = "$@ doesn't have the HttpOnly flag set." and + sourceNode = source.getNode() and + message2 = "This sensitive cookie" +} From e1cf3d30d2447b779ec3bc847f9262e1f90d3866 Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Wed, 1 Oct 2025 11:20:11 +0100 Subject: [PATCH 02/10] Update documentation, rename things and add more comments to explain how the implementation works, remove filter for test code (prefer to filter in code scanning ui than in query logic) --- .../CWE-1004/SensitiveCookieNotHttpOnly.qhelp | 7 +- .../CWE-1004/SensitiveCookieNotHttpOnly.ql | 70 +++++++------------ 2 files changed, 29 insertions(+), 48 deletions(-) diff --git a/java/ql/src/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.qhelp b/java/ql/src/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.qhelp index ee3e8a4181a9..71e016510e26 100644 --- a/java/ql/src/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.qhelp +++ b/java/ql/src/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.qhelp @@ -2,11 +2,13 @@ -

    Cross-Site Scripting (XSS) is categorized as one of the OWASP Top 10 Security Vulnerabilities. The HttpOnly flag directs compatible browsers to prevent client-side script from accessing cookies. Including the HttpOnly flag in the Set-Cookie HTTP response header for a sensitive cookie helps mitigate the risk associated with XSS where an attacker's script code attempts to read the contents of a cookie and exfiltrate information obtained.

    +

    Cookies without the HttpOnly flag set are accessible to client-side scripts (such as JavaScript) running in the same origin. +In case of a Cross-Site Scripting (XSS) vulnerability, the cookie can be stolen by a malicious script. +If a sensitive cookie does not need to be accessed directly by client-side scripts, the HttpOnly flag should be set.

     -

    Use the HttpOnly flag when generating a cookie containing sensitive information to help mitigate the risk of client side script accessing the protected cookie.

    +

    Use the HttpOnly flag when generating a cookie containing sensitive information to help mitigate the risk of client-side scripts accessing the protected cookie.

     @@ -23,5 +25,6 @@ OWASP: HttpOnly +
  • MDN: Set-Cookie HttpOnly.
    •  diff --git a/java/ql/src/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql b/java/ql/src/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql index d2d596c23fa3..41b2c95c8704 100644 --- a/java/ql/src/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql +++ b/java/ql/src/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql @@ -14,7 +14,7 @@ * Sketch of the structure of this query: we track cookie names that appear to be sensitive * (e.g. `session` or `token`) to a `ServletResponse.addHeader(...)` or `.addCookie(...)` * method that does not set the `httpOnly` flag. Subsidiary configurations - * `MatchesHttpOnlyConfiguration` and `SetHttpOnlyInCookieConfiguration` are used to establish + * `MatchesHttpOnlyToRawHeaderConfiguration` and `SetHttpOnlyInCookieConfiguration` are used to establish * when the `httpOnly` flag is likely to have been set, before configuration * `MissingHttpOnlyConfiguration` establishes that a non-`httpOnly` cookie has a sensitive-seeming name. */ @@ -49,8 +49,8 @@ class SensitiveCookieNameExpr extends Expr { } /** A method call that sets a `Set-Cookie` header. */ -class SetCookieMethodCall extends MethodCall { - SetCookieMethodCall() { +class SetCookieRawHeaderMethodCall extends MethodCall { + SetCookieRawHeaderMethodCall() { ( this.getMethod() instanceof ResponseAddHeaderMethod or this.getMethod() instanceof ResponseSetHeaderMethod @@ -61,19 +61,19 @@ class SetCookieMethodCall extends MethodCall { /** * A taint configuration tracking flow from the text `httponly` to argument 1 of - * `SetCookieMethodCall`. + * `SetCookieRawHeaderMethodCall`. */ -module MatchesHttpOnlyConfig implements DataFlow::ConfigSig { +module MatchesHttpOnlyToRawHeaderConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { source.asExpr().(CompileTimeConstantExpr).getStringValue().toLowerCase().matches("%httponly%") } predicate isSink(DataFlow::Node sink) { - sink.asExpr() = any(SetCookieMethodCall ma).getArgument(1) + sink.asExpr() = any(SetCookieRawHeaderMethodCall ma).getArgument(1) } } -module MatchesHttpOnlyFlow = TaintTracking::Global; +module MatchesHttpOnlyToRawHeaderFlow = TaintTracking::Global; /** A class descended from `javax.servlet.http.Cookie`. */ class CookieClass extends RefType { @@ -101,30 +101,11 @@ predicate removesCookie(MethodCall ma) { ma.getArgument(0).(IntegerLiteral).getIntValue() = 0 } -/** - * Holds if the MethodCall `ma` is a test method call indicated by: - * a) in a test directory such as `src/test/java` - * b) in a test package whose name has the word `test` - * c) in a test class whose name has the word `test` - * d) in a test class implementing a test framework such as JUnit or TestNG - */ -predicate isTestMethod(MethodCall ma) { - exists(Method m | - m = ma.getEnclosingCallable() and - ( - m.getDeclaringType().getName().toLowerCase().matches("%test%") or // Simple check to exclude test classes to reduce FPs - m.getDeclaringType().getPackage().getName().toLowerCase().matches("%test%") or // Simple check to exclude classes in test packages to reduce FPs - exists(m.getLocation().getFile().getAbsolutePath().indexOf("/src/test/java")) or // Match test directory structure of build tools like maven - m instanceof TestMethod // Test method of a test case implementing a test framework such as JUnit or TestNG - ) - ) -} - /** * A taint configuration tracking flow of a method that sets the `HttpOnly` flag, * or one that removes a cookie, to a `ServletResponse.addCookie` call. */ -module SetHttpOnlyOrRemovesCookieConfig implements DataFlow::ConfigSig { +module SetHttpOnlyOrRemovesCookieToAddCookieConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { source.asExpr() = any(MethodCall ma | setsCookieHttpOnly(ma) or removesCookie(ma)).getQualifier() @@ -136,25 +117,25 @@ module SetHttpOnlyOrRemovesCookieConfig implements DataFlow::ConfigSig { } } -module SetHttpOnlyOrRemovesCookieFlow = TaintTracking::Global; +module SetHttpOnlyOrRemovesCookieToAddCookieFlow = + TaintTracking::Global; /** * A cookie that is added to an HTTP response and which doesn't have `httpOnly` set, used as a sink * in `MissingHttpOnlyConfiguration`. */ -class CookieResponseSink extends DataFlow::ExprNode { - CookieResponseSink() { +class CookieResponseWithoutHttpOnlySink extends DataFlow::ExprNode { + CookieResponseWithoutHttpOnlySink() { exists(MethodCall ma | ( ma.getMethod() instanceof ResponseAddCookieMethod and this.getExpr() = ma.getArgument(0) and - not SetHttpOnlyOrRemovesCookieFlow::flowTo(this) + not SetHttpOnlyOrRemovesCookieToAddCookieFlow::flowTo(this) or - ma instanceof SetCookieMethodCall and + ma instanceof SetCookieRawHeaderMethodCall and this.getExpr() = ma.getArgument(1) and - not MatchesHttpOnlyFlow::flowTo(this) // response.addHeader("Set-Cookie", "token=" +authId + ";HttpOnly;Secure") - ) and - not isTestMethod(ma) // Test class or method + not MatchesHttpOnlyToRawHeaderFlow::flowTo(this) // response.addHeader("Set-Cookie", "token=" +authId + ";HttpOnly;Secure") + ) ) } } @@ -178,14 +159,18 @@ predicate setsHttpOnlyInNewCookie(ClassInstanceExpr cie) { /** * A taint configuration tracking flow from a sensitive cookie without the `HttpOnly` flag * set to its HTTP response. + * Tracks string literals containing sensitive names (`SensitiveNameExpr`), to an `addCookie` call (as a `Cookie` object) + * or an `addHeader` call (as a string) (`CookieResponseWithoutHttpOnly`). + * Passes through `Cookie` constructors and `toString` calls. */ module MissingHttpOnlyConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { source.asExpr() instanceof SensitiveCookieNameExpr } - predicate isSink(DataFlow::Node sink) { sink instanceof CookieResponseSink } + predicate isSink(DataFlow::Node sink) { sink instanceof CookieResponseWithoutHttpOnlySink } predicate isBarrier(DataFlow::Node node) { // JAX-RS's `new NewCookie("session-access-key", accessKey, "/", null, null, 0, true, true)` and similar + // Cookie constructors, but barriers to considering the flow of the sensitive name, as httponly flag is set. setsHttpOnlyInNewCookie(node.asExpr()) } @@ -211,13 +196,6 @@ module MissingHttpOnlyConfig implements DataFlow::ConfigSig { module MissingHttpOnlyFlow = TaintTracking::Global; -deprecated query predicate problems( - DataFlow::Node sinkNode, MissingHttpOnlyFlow::PathNode source, MissingHttpOnlyFlow::PathNode sink, - string message1, DataFlow::Node sourceNode, string message2 -) { - MissingHttpOnlyFlow::flowPath(source, sink) and - sinkNode = sink.getNode() and - message1 = "$@ doesn't have the HttpOnly flag set." and - sourceNode = source.getNode() and - message2 = "This sensitive cookie" -} +from MissingHttpOnlyFlow::PathNode source, MissingHttpOnlyFlow::PathNode sink +where MissingHttpOnlyFlow::flowPath(source, sink) +select sink, source, sink, "$@ doesn't have the HttpOnly flag set.", source, "This sensitive cookie" From c799f9381140d28844a1654c323a1455c9e689e2 Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Wed, 1 Oct 2025 16:48:09 +0100 Subject: [PATCH 03/10] Update tests and add inline expectations --- .../CWE-1004/SensitiveCookieNotHttpOnly.ql | 3 +- .../SensitiveCookieNotHttpOnly.expected | 60 +++++++ .../CWE-1004/SensitiveCookieNotHttpOnly.java | 164 ++++++++++++++++++ .../CWE-1004/SensitiveCookieNotHttpOnly.qlref | 2 + .../query-tests/security/CWE-1004/options | 1 + 5 files changed, 229 insertions(+), 1 deletion(-) create mode 100644 java/ql/test/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.expected create mode 100644 java/ql/test/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.java create mode 100644 java/ql/test/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.qlref create mode 100644 java/ql/test/query-tests/security/CWE-1004/options diff --git a/java/ql/src/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql b/java/ql/src/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql index 41b2c95c8704..4eae5f1ee4eb 100644 --- a/java/ql/src/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql +++ b/java/ql/src/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql @@ -23,7 +23,6 @@ import java import semmle.code.java.dataflow.FlowSteps import semmle.code.java.frameworks.Servlets import semmle.code.java.dataflow.TaintTracking -import MissingHttpOnlyFlow::PathGraph /** Gets a regular expression for matching common names of sensitive cookies. */ string getSensitiveCookieNameRegex() { result = "(?i).*(auth|session|token|key|credential).*" } @@ -196,6 +195,8 @@ module MissingHttpOnlyConfig implements DataFlow::ConfigSig { module MissingHttpOnlyFlow = TaintTracking::Global; +import MissingHttpOnlyFlow::PathGraph + from MissingHttpOnlyFlow::PathNode source, MissingHttpOnlyFlow::PathNode sink where MissingHttpOnlyFlow::flowPath(source, sink) select sink, source, sink, "$@ doesn't have the HttpOnly flag set.", source, "This sensitive cookie" diff --git a/java/ql/test/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.expected b/java/ql/test/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.expected new file mode 100644 index 000000000000..71c73f3921e9 --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.expected @@ -0,0 +1,60 @@ +#select +| SensitiveCookieNotHttpOnly.java:31:28:31:36 | jwtCookie | SensitiveCookieNotHttpOnly.java:24:33:24:43 | "jwt_token" : String | SensitiveCookieNotHttpOnly.java:31:28:31:36 | jwtCookie | $@ doesn't have the HttpOnly flag set. | SensitiveCookieNotHttpOnly.java:24:33:24:43 | "jwt_token" : String | This sensitive cookie | +| SensitiveCookieNotHttpOnly.java:42:42:42:69 | ... + ... | SensitiveCookieNotHttpOnly.java:42:42:42:49 | "token=" : String | SensitiveCookieNotHttpOnly.java:42:42:42:69 | ... + ... | $@ doesn't have the HttpOnly flag set. | SensitiveCookieNotHttpOnly.java:42:42:42:49 | "token=" : String | This sensitive cookie | +| SensitiveCookieNotHttpOnly.java:42:42:42:69 | ... + ... | SensitiveCookieNotHttpOnly.java:42:42:42:57 | ... + ... : String | SensitiveCookieNotHttpOnly.java:42:42:42:69 | ... + ... | $@ doesn't have the HttpOnly flag set. | SensitiveCookieNotHttpOnly.java:42:42:42:57 | ... + ... : String | This sensitive cookie | +| SensitiveCookieNotHttpOnly.java:42:42:42:69 | ... + ... | SensitiveCookieNotHttpOnly.java:42:42:42:69 | ... + ... | SensitiveCookieNotHttpOnly.java:42:42:42:69 | ... + ... | $@ doesn't have the HttpOnly flag set. | SensitiveCookieNotHttpOnly.java:42:42:42:69 | ... + ... | This sensitive cookie | +| SensitiveCookieNotHttpOnly.java:52:42:52:124 | toString(...) | SensitiveCookieNotHttpOnly.java:52:56:52:75 | "session-access-key" : String | SensitiveCookieNotHttpOnly.java:52:42:52:124 | toString(...) | $@ doesn't have the HttpOnly flag set. | SensitiveCookieNotHttpOnly.java:52:56:52:75 | "session-access-key" : String | This sensitive cookie | +| SensitiveCookieNotHttpOnly.java:65:42:65:47 | keyStr | SensitiveCookieNotHttpOnly.java:63:51:63:70 | "session-access-key" : String | SensitiveCookieNotHttpOnly.java:65:42:65:47 | keyStr | $@ doesn't have the HttpOnly flag set. | SensitiveCookieNotHttpOnly.java:63:51:63:70 | "session-access-key" : String | This sensitive cookie | +| SensitiveCookieNotHttpOnly.java:71:42:71:50 | secString | SensitiveCookieNotHttpOnly.java:70:28:70:35 | "token=" : String | SensitiveCookieNotHttpOnly.java:71:42:71:50 | secString | $@ doesn't have the HttpOnly flag set. | SensitiveCookieNotHttpOnly.java:70:28:70:35 | "token=" : String | This sensitive cookie | +| SensitiveCookieNotHttpOnly.java:71:42:71:50 | secString | SensitiveCookieNotHttpOnly.java:70:28:70:43 | ... + ... : String | SensitiveCookieNotHttpOnly.java:71:42:71:50 | secString | $@ doesn't have the HttpOnly flag set. | SensitiveCookieNotHttpOnly.java:70:28:70:43 | ... + ... : String | This sensitive cookie | +| SensitiveCookieNotHttpOnly.java:71:42:71:50 | secString | SensitiveCookieNotHttpOnly.java:70:28:70:55 | ... + ... : String | SensitiveCookieNotHttpOnly.java:71:42:71:50 | secString | $@ doesn't have the HttpOnly flag set. | SensitiveCookieNotHttpOnly.java:70:28:70:55 | ... + ... : String | This sensitive cookie | +| SensitiveCookieNotHttpOnly.java:111:28:111:33 | cookie | SensitiveCookieNotHttpOnly.java:88:35:88:51 | "Presto-UI-Token" : String | SensitiveCookieNotHttpOnly.java:111:28:111:33 | cookie | $@ doesn't have the HttpOnly flag set. | SensitiveCookieNotHttpOnly.java:88:35:88:51 | "Presto-UI-Token" : String | This sensitive cookie | +edges +| SensitiveCookieNotHttpOnly.java:24:33:24:43 | "jwt_token" : String | SensitiveCookieNotHttpOnly.java:25:39:25:52 | tokenCookieStr : String | provenance | | +| SensitiveCookieNotHttpOnly.java:25:28:25:64 | new Cookie(...) : Cookie | SensitiveCookieNotHttpOnly.java:31:28:31:36 | jwtCookie | provenance | Sink:MaD:46211 | +| SensitiveCookieNotHttpOnly.java:25:39:25:52 | tokenCookieStr : String | SensitiveCookieNotHttpOnly.java:25:28:25:64 | new Cookie(...) : Cookie | provenance | Config | +| SensitiveCookieNotHttpOnly.java:25:39:25:52 | tokenCookieStr : String | SensitiveCookieNotHttpOnly.java:25:28:25:64 | new Cookie(...) : Cookie | provenance | MaD:46217 | +| SensitiveCookieNotHttpOnly.java:42:42:42:49 | "token=" : String | SensitiveCookieNotHttpOnly.java:42:42:42:69 | ... + ... | provenance | Sink:MaD:46212 | +| SensitiveCookieNotHttpOnly.java:42:42:42:57 | ... + ... : String | SensitiveCookieNotHttpOnly.java:42:42:42:69 | ... + ... | provenance | Sink:MaD:46212 | +| SensitiveCookieNotHttpOnly.java:52:42:52:113 | new NewCookie(...) : NewCookie | SensitiveCookieNotHttpOnly.java:52:42:52:124 | toString(...) | provenance | MaD:46260 Sink:MaD:46214 | +| SensitiveCookieNotHttpOnly.java:52:56:52:75 | "session-access-key" : String | SensitiveCookieNotHttpOnly.java:52:42:52:113 | new NewCookie(...) : NewCookie | provenance | MaD:46298 | +| SensitiveCookieNotHttpOnly.java:63:37:63:115 | new NewCookie(...) : NewCookie | SensitiveCookieNotHttpOnly.java:64:25:64:39 | accessKeyCookie : NewCookie | provenance | | +| SensitiveCookieNotHttpOnly.java:63:51:63:70 | "session-access-key" : String | SensitiveCookieNotHttpOnly.java:63:37:63:115 | new NewCookie(...) : NewCookie | provenance | MaD:46298 | +| SensitiveCookieNotHttpOnly.java:64:25:64:39 | accessKeyCookie : NewCookie | SensitiveCookieNotHttpOnly.java:64:25:64:50 | toString(...) : String | provenance | MaD:46260 | +| SensitiveCookieNotHttpOnly.java:64:25:64:50 | toString(...) : String | SensitiveCookieNotHttpOnly.java:65:42:65:47 | keyStr | provenance | Sink:MaD:46214 | +| SensitiveCookieNotHttpOnly.java:70:28:70:35 | "token=" : String | SensitiveCookieNotHttpOnly.java:71:42:71:50 | secString | provenance | Sink:MaD:46212 | +| SensitiveCookieNotHttpOnly.java:70:28:70:43 | ... + ... : String | SensitiveCookieNotHttpOnly.java:71:42:71:50 | secString | provenance | Sink:MaD:46212 | +| SensitiveCookieNotHttpOnly.java:70:28:70:55 | ... + ... : String | SensitiveCookieNotHttpOnly.java:71:42:71:50 | secString | provenance | Sink:MaD:46212 | +| SensitiveCookieNotHttpOnly.java:88:35:88:51 | "Presto-UI-Token" : String | SensitiveCookieNotHttpOnly.java:89:36:89:51 | PRESTO_UI_COOKIE : String | provenance | | +| SensitiveCookieNotHttpOnly.java:89:25:89:57 | new Cookie(...) : Cookie | SensitiveCookieNotHttpOnly.java:91:16:91:21 | cookie : Cookie | provenance | | +| SensitiveCookieNotHttpOnly.java:89:36:89:51 | PRESTO_UI_COOKIE : String | SensitiveCookieNotHttpOnly.java:89:25:89:57 | new Cookie(...) : Cookie | provenance | Config | +| SensitiveCookieNotHttpOnly.java:89:36:89:51 | PRESTO_UI_COOKIE : String | SensitiveCookieNotHttpOnly.java:89:25:89:57 | new Cookie(...) : Cookie | provenance | MaD:46217 | +| SensitiveCookieNotHttpOnly.java:91:16:91:21 | cookie : Cookie | SensitiveCookieNotHttpOnly.java:110:25:110:64 | createAuthenticationCookie(...) : Cookie | provenance | | +| SensitiveCookieNotHttpOnly.java:110:25:110:64 | createAuthenticationCookie(...) : Cookie | SensitiveCookieNotHttpOnly.java:111:28:111:33 | cookie | provenance | Sink:MaD:46211 | +nodes +| SensitiveCookieNotHttpOnly.java:24:33:24:43 | "jwt_token" : String | semmle.label | "jwt_token" : String | +| SensitiveCookieNotHttpOnly.java:25:28:25:64 | new Cookie(...) : Cookie | semmle.label | new Cookie(...) : Cookie | +| SensitiveCookieNotHttpOnly.java:25:39:25:52 | tokenCookieStr : String | semmle.label | tokenCookieStr : String | +| SensitiveCookieNotHttpOnly.java:31:28:31:36 | jwtCookie | semmle.label | jwtCookie | +| SensitiveCookieNotHttpOnly.java:42:42:42:49 | "token=" : String | semmle.label | "token=" : String | +| SensitiveCookieNotHttpOnly.java:42:42:42:57 | ... + ... : String | semmle.label | ... + ... : String | +| SensitiveCookieNotHttpOnly.java:42:42:42:69 | ... + ... | semmle.label | ... + ... | +| SensitiveCookieNotHttpOnly.java:52:42:52:113 | new NewCookie(...) : NewCookie | semmle.label | new NewCookie(...) : NewCookie | +| SensitiveCookieNotHttpOnly.java:52:42:52:124 | toString(...) | semmle.label | toString(...) | +| SensitiveCookieNotHttpOnly.java:52:56:52:75 | "session-access-key" : String | semmle.label | "session-access-key" : String | +| SensitiveCookieNotHttpOnly.java:63:37:63:115 | new NewCookie(...) : NewCookie | semmle.label | new NewCookie(...) : NewCookie | +| SensitiveCookieNotHttpOnly.java:63:51:63:70 | "session-access-key" : String | semmle.label | "session-access-key" : String | +| SensitiveCookieNotHttpOnly.java:64:25:64:39 | accessKeyCookie : NewCookie | semmle.label | accessKeyCookie : NewCookie | +| SensitiveCookieNotHttpOnly.java:64:25:64:50 | toString(...) : String | semmle.label | toString(...) : String | +| SensitiveCookieNotHttpOnly.java:65:42:65:47 | keyStr | semmle.label | keyStr | +| SensitiveCookieNotHttpOnly.java:70:28:70:35 | "token=" : String | semmle.label | "token=" : String | +| SensitiveCookieNotHttpOnly.java:70:28:70:43 | ... + ... : String | semmle.label | ... + ... : String | +| SensitiveCookieNotHttpOnly.java:70:28:70:55 | ... + ... : String | semmle.label | ... + ... : String | +| SensitiveCookieNotHttpOnly.java:71:42:71:50 | secString | semmle.label | secString | +| SensitiveCookieNotHttpOnly.java:88:35:88:51 | "Presto-UI-Token" : String | semmle.label | "Presto-UI-Token" : String | +| SensitiveCookieNotHttpOnly.java:89:25:89:57 | new Cookie(...) : Cookie | semmle.label | new Cookie(...) : Cookie | +| SensitiveCookieNotHttpOnly.java:89:36:89:51 | PRESTO_UI_COOKIE : String | semmle.label | PRESTO_UI_COOKIE : String | +| SensitiveCookieNotHttpOnly.java:91:16:91:21 | cookie : Cookie | semmle.label | cookie : Cookie | +| SensitiveCookieNotHttpOnly.java:110:25:110:64 | createAuthenticationCookie(...) : Cookie | semmle.label | createAuthenticationCookie(...) : Cookie | +| SensitiveCookieNotHttpOnly.java:111:28:111:33 | cookie | semmle.label | cookie | +subpaths diff --git a/java/ql/test/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.java b/java/ql/test/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.java new file mode 100644 index 000000000000..a57a502336f8 --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.java @@ -0,0 +1,164 @@ +import java.io.IOException; + +import javax.servlet.http.Cookie; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import javax.servlet.ServletException; + +import javax.ws.rs.core.NewCookie; + +import org.springframework.security.web.csrf.CsrfToken; + +class SensitiveCookieNotHttpOnly { + // GOOD - Tests adding a sensitive cookie with the `HttpOnly` flag set. + public void addCookie(String jwt_token, HttpServletRequest request, HttpServletResponse response) { + Cookie jwtCookie = new Cookie("jwt_token", jwt_token); + jwtCookie.setPath("/"); + jwtCookie.setMaxAge(3600*24*7); + jwtCookie.setHttpOnly(true); + response.addCookie(jwtCookie); + } + + // BAD - Tests adding a sensitive cookie without the `HttpOnly` flag set. + public void addCookie2(String jwt_token, String userId, HttpServletRequest request, HttpServletResponse response) { + String tokenCookieStr = "jwt_token"; // $Source + Cookie jwtCookie = new Cookie(tokenCookieStr, jwt_token); + Cookie userIdCookie = new Cookie("user_id", userId); + jwtCookie.setPath("/"); + userIdCookie.setPath("/"); + jwtCookie.setMaxAge(3600*24*7); + userIdCookie.setMaxAge(3600*24*7); + response.addCookie(jwtCookie); // $Alert + response.addCookie(userIdCookie); + } + + // GOOD - Tests set a sensitive cookie header with the `HttpOnly` flag set. + public void addCookie3(String authId, HttpServletRequest request, HttpServletResponse response) { + response.addHeader("Set-Cookie", "token=" +authId + ";HttpOnly;Secure"); + } + + // BAD - Tests set a sensitive cookie header without the `HttpOnly` flag set. + public void addCookie4(String authId, HttpServletRequest request, HttpServletResponse response) { + response.addHeader("Set-Cookie", "token=" +authId + ";Secure"); // $Alert + } + + // GOOD - Tests set a sensitive cookie header using the class `javax.ws.rs.core.Cookie` with the `HttpOnly` flag set through string concatenation. + public void addCookie5(String accessKey, HttpServletRequest request, HttpServletResponse response) { + response.setHeader("Set-Cookie", new NewCookie("session-access-key", accessKey, "/", null, null, 0, true) + ";HttpOnly"); + } + + // BAD - Tests set a sensitive cookie header using the class `javax.ws.rs.core.Cookie` without the `HttpOnly` flag set. + public void addCookie6(String accessKey, HttpServletRequest request, HttpServletResponse response) { + response.setHeader("Set-Cookie", new NewCookie("session-access-key", accessKey, "/", null, null, 0, true).toString()); // $Alert + } + + // GOOD - Tests set a sensitive cookie header using the class `javax.ws.rs.core.Cookie` with the `HttpOnly` flag set through the constructor. + public void addCookie7(String accessKey, HttpServletRequest request, HttpServletResponse response) { + NewCookie accessKeyCookie = new NewCookie("session-access-key", accessKey, "/", null, null, 0, true, true); + response.setHeader("Set-Cookie", accessKeyCookie.toString()); + } + + // BAD - Tests set a sensitive cookie header using the class `javax.ws.rs.core.Cookie` without the `HttpOnly` flag set. + public void addCookie8(String accessKey, HttpServletRequest request, HttpServletResponse response) { + NewCookie accessKeyCookie = new NewCookie("session-access-key", accessKey, "/", null, 0, null, 86400, true); // $Source + String keyStr = accessKeyCookie.toString(); + response.setHeader("Set-Cookie", keyStr); // $Alert + } + + // BAD - Tests set a sensitive cookie header using a variable without the `HttpOnly` flag set. + public void addCookie9(String authId, HttpServletRequest request, HttpServletResponse response) { + String secString = "token=" +authId + ";Secure"; // $Source + response.addHeader("Set-Cookie", secString); // $Alert + } + + // GOOD - Tests set a sensitive cookie header with the `HttpOnly` flag set using `String.format(...)`. + public void addCookie10(HttpServletRequest request, HttpServletResponse response) { + response.addHeader("SET-COOKIE", String.format("%s=%s;HttpOnly", "sessionkey", request.getSession().getAttribute("sessionkey"))); + } + + public Cookie createHttpOnlyAuthenticationCookie(HttpServletRequest request, String jwt) { + String PRESTO_UI_COOKIE = "Presto-UI-Token"; + Cookie cookie = new Cookie(PRESTO_UI_COOKIE, jwt); + cookie.setHttpOnly(true); + cookie.setPath("/ui"); + return cookie; + } + + public Cookie createAuthenticationCookie(HttpServletRequest request, String jwt) { + String PRESTO_UI_COOKIE = "Presto-UI-Token"; // $Source + Cookie cookie = new Cookie(PRESTO_UI_COOKIE, jwt); + cookie.setPath("/ui"); + return cookie; + } + + public Cookie removeAuthenticationCookie(HttpServletRequest request, String jwt) { + String PRESTO_UI_COOKIE = "Presto-UI-Token"; + Cookie cookie = new Cookie(PRESTO_UI_COOKIE, jwt); + cookie.setPath("/ui"); + cookie.setMaxAge(0); + return cookie; + } + + // GOOD - Tests set a sensitive cookie header with the `HttpOnly` flag set using a wrapper method. + public void addCookie11(HttpServletRequest request, HttpServletResponse response, String jwt) { + Cookie cookie = createHttpOnlyAuthenticationCookie(request, jwt); + response.addCookie(cookie); + } + + // BAD - Tests set a sensitive cookie header without the `HttpOnly` flag set using a wrapper method. + public void addCookie12(HttpServletRequest request, HttpServletResponse response, String jwt) { + Cookie cookie = createAuthenticationCookie(request, jwt); + response.addCookie(cookie); // $Alert + } + + // GOOD - Tests remove a sensitive cookie header without the `HttpOnly` flag set using a wrapper method. + public void addCookie13(HttpServletRequest request, HttpServletResponse response, String jwt) { + Cookie cookie = removeAuthenticationCookie(request, jwt); + response.addCookie(cookie); + } + + private Cookie createCookie(String name, String value, Boolean httpOnly){ + Cookie cookie = null; + cookie = new Cookie(name, value); + cookie.setDomain("/"); + cookie.setHttpOnly(httpOnly); + + //for production https + cookie.setSecure(true); + + cookie.setMaxAge(60*60*24*30); + cookie.setPath("/"); + + return cookie; + } + + // GOOD - Tests set a sensitive cookie header with the `HttpOnly` flag set through a boolean variable using a wrapper method. + public void addCookie14(HttpServletRequest request, HttpServletResponse response, String refreshToken) { + response.addCookie(createCookie("refresh_token", refreshToken, true)); + } + + // BAD (but not detected) - Tests set a sensitive cookie header with the `HttpOnly` flag not set through a boolean variable using a wrapper method. + // This example is missed because the `cookie.setHttpOnly` call in `createCookie` is thought to maybe set the HTTP-only flag, and the `cookie` + // object flows to this `addCookie` call. + public void addCookie15(HttpServletRequest request, HttpServletResponse response, String refreshToken) { + response.addCookie(createCookie("refresh_token", refreshToken, false)); // $MISSING:Alert + } + + // GOOD - CSRF token doesn't need to have the `HttpOnly` flag set. + public void addCsrfCookie(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { + // Spring put the CSRF token in session attribute "_csrf" + CsrfToken csrfToken = (CsrfToken) request.getAttribute("_csrf"); + + // Send the cookie only if the token has changed + String actualToken = request.getHeader("X-CSRF-TOKEN"); + if (actualToken == null || !actualToken.equals(csrfToken.getToken())) { + // Session cookie that can be used by AngularJS + String pCookieName = "CSRF-TOKEN"; + Cookie cookie = new Cookie(pCookieName, csrfToken.getToken()); + cookie.setMaxAge(-1); + cookie.setHttpOnly(false); + cookie.setPath("/"); + response.addCookie(cookie); + } + } +} diff --git a/java/ql/test/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.qlref b/java/ql/test/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.qlref new file mode 100644 index 000000000000..e3b99f6e6855 --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.qlref @@ -0,0 +1,2 @@ +query: Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql +postprocess: utils/test/InlineExpectationsTestQuery.ql \ No newline at end of file diff --git a/java/ql/test/query-tests/security/CWE-1004/options b/java/ql/test/query-tests/security/CWE-1004/options new file mode 100644 index 000000000000..0db0b6e72425 --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-1004/options @@ -0,0 +1 @@ +// semmle-extractor-options: --javac-args -cp ${testdir}/../../../stubs/servlet-api-2.4:${testdir}/../../../stubs/jsr311-api-1.1.1:${testdir}/../../../stubs/springframework-5.8.x From c4781146c0f7497ded8090b532f9f84411159391 Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Thu, 2 Oct 2025 09:59:44 +0100 Subject: [PATCH 04/10] Remove experimental query and tests --- .../CWE-1004/SensitiveCookieNotHttpOnly.java | 44 ---- .../CWE-1004/SensitiveCookieNotHttpOnly.qhelp | 27 --- .../CWE-1004/SensitiveCookieNotHttpOnly.ql | 224 ------------------ .../SensitiveCookieNotHttpOnly.expected | 67 ------ .../CWE-1004/SensitiveCookieNotHttpOnly.java | 164 ------------- .../CWE-1004/SensitiveCookieNotHttpOnly.qlref | 2 - .../query-tests/security/CWE-1004/options | 1 - 7 files changed, 529 deletions(-) delete mode 100644 java/ql/src/experimental/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.java delete mode 100644 java/ql/src/experimental/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.qhelp delete mode 100644 java/ql/src/experimental/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql delete mode 100644 java/ql/test/experimental/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.expected delete mode 100644 java/ql/test/experimental/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.java delete mode 100644 java/ql/test/experimental/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.qlref delete mode 100644 java/ql/test/experimental/query-tests/security/CWE-1004/options diff --git a/java/ql/src/experimental/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.java b/java/ql/src/experimental/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.java deleted file mode 100644 index 48d80707ff83..000000000000 --- a/java/ql/src/experimental/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.java +++ /dev/null @@ -1,44 +0,0 @@ -class SensitiveCookieNotHttpOnly { - // GOOD - Create a sensitive cookie with the `HttpOnly` flag set. - public void addCookie(String jwt_token, HttpServletRequest request, HttpServletResponse response) { - Cookie jwtCookie =new Cookie("jwt_token", jwt_token); - jwtCookie.setPath("/"); - jwtCookie.setMaxAge(3600*24*7); - jwtCookie.setHttpOnly(true); - response.addCookie(jwtCookie); - } - - // BAD - Create a sensitive cookie without the `HttpOnly` flag set. - public void addCookie2(String jwt_token, String userId, HttpServletRequest request, HttpServletResponse response) { - Cookie jwtCookie =new Cookie("jwt_token", jwt_token); - jwtCookie.setPath("/"); - jwtCookie.setMaxAge(3600*24*7); - response.addCookie(jwtCookie); - } - - // GOOD - Set a sensitive cookie header with the `HttpOnly` flag set. - public void addCookie3(String authId, HttpServletRequest request, HttpServletResponse response) { - response.addHeader("Set-Cookie", "token=" +authId + ";HttpOnly;Secure"); - } - - // BAD - Set a sensitive cookie header without the `HttpOnly` flag set. - public void addCookie4(String authId, HttpServletRequest request, HttpServletResponse response) { - response.addHeader("Set-Cookie", "token=" +authId + ";Secure"); - } - - // GOOD - Set a sensitive cookie header using the class `javax.ws.rs.core.Cookie` with the `HttpOnly` flag set through string concatenation. - public void addCookie5(String accessKey, HttpServletRequest request, HttpServletResponse response) { - response.setHeader("Set-Cookie", new NewCookie("session-access-key", accessKey, "/", null, null, 0, true) + ";HttpOnly"); - } - - // BAD - Set a sensitive cookie header using the class `javax.ws.rs.core.Cookie` without the `HttpOnly` flag set. - public void addCookie6(String accessKey, HttpServletRequest request, HttpServletResponse response) { - response.setHeader("Set-Cookie", new NewCookie("session-access-key", accessKey, "/", null, null, 0, true).toString()); - } - - // GOOD - Set a sensitive cookie header using the class `javax.ws.rs.core.Cookie` with the `HttpOnly` flag set through the constructor. - public void addCookie7(String accessKey, HttpServletRequest request, HttpServletResponse response) { - NewCookie accessKeyCookie = new NewCookie("session-access-key", accessKey, "/", null, null, 0, true, true); - response.setHeader("Set-Cookie", accessKeyCookie.toString()); - } -} diff --git a/java/ql/src/experimental/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.qhelp b/java/ql/src/experimental/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.qhelp deleted file mode 100644 index ee3e8a4181a9..000000000000 --- a/java/ql/src/experimental/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.qhelp +++ /dev/null @@ -1,27 +0,0 @@ - - - - -

    Cross-Site Scripting (XSS) is categorized as one of the OWASP Top 10 Security Vulnerabilities. The HttpOnly flag directs compatible browsers to prevent client-side script from accessing cookies. Including the HttpOnly flag in the Set-Cookie HTTP response header for a sensitive cookie helps mitigate the risk associated with XSS where an attacker's script code attempts to read the contents of a cookie and exfiltrate information obtained.

    -     - - -

    Use the HttpOnly flag when generating a cookie containing sensitive information to help mitigate the risk of client side script accessing the protected cookie.

    -     - - -

    The following example shows two ways of generating sensitive cookies. In the 'BAD' cases, the HttpOnly flag is not set. In the 'GOOD' cases, the HttpOnly flag is set.

    - -     - - -
  • - PortSwigger: - Cookie without HttpOnly flag set -
    • -
  • - OWASP: - HttpOnly -
    • -     -     diff --git a/java/ql/src/experimental/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql b/java/ql/src/experimental/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql deleted file mode 100644 index fa5237d32bb9..000000000000 --- a/java/ql/src/experimental/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql +++ /dev/null @@ -1,224 +0,0 @@ -/** - * @name Sensitive cookies without the HttpOnly response header set - * @description Sensitive cookies without the 'HttpOnly' flag set leaves session cookies vulnerable to - * an XSS attack. - * @kind path-problem - * @problem.severity warning - * @precision medium - * @id java/sensitive-cookie-not-httponly - * @tags security - * experimental - * external/cwe/cwe-1004 - */ - -/* - * Sketch of the structure of this query: we track cookie names that appear to be sensitive - * (e.g. `session` or `token`) to a `ServletResponse.addHeader(...)` or `.addCookie(...)` - * method that does not set the `httpOnly` flag. Subsidiary configurations - * `MatchesHttpOnlyConfiguration` and `SetHttpOnlyInCookieConfiguration` are used to establish - * when the `httpOnly` flag is likely to have been set, before configuration - * `MissingHttpOnlyConfiguration` establishes that a non-`httpOnly` cookie has a sensitive-seeming name. - */ - -import java -import semmle.code.java.dataflow.FlowSteps -import semmle.code.java.frameworks.Servlets -import semmle.code.java.dataflow.TaintTracking -import MissingHttpOnlyFlow::PathGraph - -/** Gets a regular expression for matching common names of sensitive cookies. */ -string getSensitiveCookieNameRegex() { result = "(?i).*(auth|session|token|key|credential).*" } - -/** Gets a regular expression for matching CSRF cookies. */ -string getCsrfCookieNameRegex() { result = "(?i).*(csrf).*" } - -/** - * Holds if a string is concatenated with the name of a sensitive cookie. Excludes CSRF cookies since - * they are special cookies implementing the Synchronizer Token Pattern that can be used in JavaScript. - */ -predicate isSensitiveCookieNameExpr(Expr expr) { - exists(string s | s = expr.(CompileTimeConstantExpr).getStringValue() | - s.regexpMatch(getSensitiveCookieNameRegex()) and not s.regexpMatch(getCsrfCookieNameRegex()) - ) - or - isSensitiveCookieNameExpr(expr.(AddExpr).getAnOperand()) -} - -/** A sensitive cookie name. */ -class SensitiveCookieNameExpr extends Expr { - SensitiveCookieNameExpr() { isSensitiveCookieNameExpr(this) } -} - -/** A method call that sets a `Set-Cookie` header. */ -class SetCookieMethodCall extends MethodCall { - SetCookieMethodCall() { - ( - this.getMethod() instanceof ResponseAddHeaderMethod or - this.getMethod() instanceof ResponseSetHeaderMethod - ) and - this.getArgument(0).(CompileTimeConstantExpr).getStringValue().toLowerCase() = "set-cookie" - } -} - -/** - * A taint configuration tracking flow from the text `httponly` to argument 1 of - * `SetCookieMethodCall`. - */ -module MatchesHttpOnlyConfig implements DataFlow::ConfigSig { - predicate isSource(DataFlow::Node source) { - source.asExpr().(CompileTimeConstantExpr).getStringValue().toLowerCase().matches("%httponly%") - } - - predicate isSink(DataFlow::Node sink) { - sink.asExpr() = any(SetCookieMethodCall ma).getArgument(1) - } -} - -module MatchesHttpOnlyFlow = TaintTracking::Global; - -/** A class descended from `javax.servlet.http.Cookie`. */ -class CookieClass extends RefType { - CookieClass() { this.getAnAncestor().hasQualifiedName("javax.servlet.http", "Cookie") } -} - -/** Holds if `expr` is any boolean-typed expression other than literal `false`. */ -// Inlined because this could be a very large result set if computed out of context -pragma[inline] -predicate mayBeBooleanTrue(Expr expr) { - expr.getType() instanceof BooleanType and - not expr.(CompileTimeConstantExpr).getBooleanValue() = false -} - -/** Holds if the method call may set the `HttpOnly` flag. */ -predicate setsCookieHttpOnly(MethodCall ma) { - ma.getMethod().getName() = "setHttpOnly" and - // any use of setHttpOnly(x) where x isn't false is probably safe - mayBeBooleanTrue(ma.getArgument(0)) -} - -/** Holds if `ma` removes a cookie. */ -predicate removesCookie(MethodCall ma) { - ma.getMethod().getName() = "setMaxAge" and - ma.getArgument(0).(IntegerLiteral).getIntValue() = 0 -} - -/** - * Holds if the MethodCall `ma` is a test method call indicated by: - * a) in a test directory such as `src/test/java` - * b) in a test package whose name has the word `test` - * c) in a test class whose name has the word `test` - * d) in a test class implementing a test framework such as JUnit or TestNG - */ -predicate isTestMethod(MethodCall ma) { - exists(Method m | - m = ma.getEnclosingCallable() and - ( - m.getDeclaringType().getName().toLowerCase().matches("%test%") or // Simple check to exclude test classes to reduce FPs - m.getDeclaringType().getPackage().getName().toLowerCase().matches("%test%") or // Simple check to exclude classes in test packages to reduce FPs - exists(m.getLocation().getFile().getAbsolutePath().indexOf("/src/test/java")) or // Match test directory structure of build tools like maven - m instanceof TestMethod // Test method of a test case implementing a test framework such as JUnit or TestNG - ) - ) -} - -/** - * A taint configuration tracking flow of a method that sets the `HttpOnly` flag, - * or one that removes a cookie, to a `ServletResponse.addCookie` call. - */ -module SetHttpOnlyOrRemovesCookieConfig implements DataFlow::ConfigSig { - predicate isSource(DataFlow::Node source) { - source.asExpr() = - any(MethodCall ma | setsCookieHttpOnly(ma) or removesCookie(ma)).getQualifier() - } - - predicate isSink(DataFlow::Node sink) { - sink.asExpr() = - any(MethodCall ma | ma.getMethod() instanceof ResponseAddCookieMethod).getArgument(0) - } -} - -module SetHttpOnlyOrRemovesCookieFlow = TaintTracking::Global; - -/** - * A cookie that is added to an HTTP response and which doesn't have `httpOnly` set, used as a sink - * in `MissingHttpOnlyConfiguration`. - */ -class CookieResponseSink extends DataFlow::ExprNode { - CookieResponseSink() { - exists(MethodCall ma | - ( - ma.getMethod() instanceof ResponseAddCookieMethod and - this.getExpr() = ma.getArgument(0) and - not SetHttpOnlyOrRemovesCookieFlow::flowTo(this) - or - ma instanceof SetCookieMethodCall and - this.getExpr() = ma.getArgument(1) and - not MatchesHttpOnlyFlow::flowTo(this) // response.addHeader("Set-Cookie", "token=" +authId + ";HttpOnly;Secure") - ) and - not isTestMethod(ma) // Test class or method - ) - } -} - -/** Holds if `cie` is an invocation of a JAX-RS `NewCookie` constructor that sets `HttpOnly` to true. */ -predicate setsHttpOnlyInNewCookie(ClassInstanceExpr cie) { - cie.getConstructedType().hasQualifiedName(["javax.ws.rs.core", "jakarta.ws.rs.core"], "NewCookie") and - ( - cie.getNumArgument() = 6 and - mayBeBooleanTrue(cie.getArgument(5)) // NewCookie(Cookie cookie, String comment, int maxAge, Date expiry, boolean secure, boolean httpOnly) - or - cie.getNumArgument() = 8 and - cie.getArgument(6).getType() instanceof BooleanType and - mayBeBooleanTrue(cie.getArgument(7)) // NewCookie(String name, String value, String path, String domain, String comment, int maxAge, boolean secure, boolean httpOnly) - or - cie.getNumArgument() = 10 and - mayBeBooleanTrue(cie.getArgument(9)) // NewCookie(String name, String value, String path, String domain, int version, String comment, int maxAge, Date expiry, boolean secure, boolean httpOnly) - ) -} - -/** - * A taint configuration tracking flow from a sensitive cookie without the `HttpOnly` flag - * set to its HTTP response. - */ -module MissingHttpOnlyConfig implements DataFlow::ConfigSig { - predicate isSource(DataFlow::Node source) { source.asExpr() instanceof SensitiveCookieNameExpr } - - predicate isSink(DataFlow::Node sink) { sink instanceof CookieResponseSink } - - predicate isBarrier(DataFlow::Node node) { - // JAX-RS's `new NewCookie("session-access-key", accessKey, "/", null, null, 0, true, true)` and similar - setsHttpOnlyInNewCookie(node.asExpr()) - } - - predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { - exists( - ConstructorCall cc // new Cookie(...) - | - cc.getConstructedType() instanceof CookieClass and - pred.asExpr() = cc.getAnArgument() and - succ.asExpr() = cc - ) - or - exists( - MethodCall ma // cookie.toString() - | - ma.getMethod().getName() = "toString" and - ma.getQualifier().getType() instanceof CookieClass and - pred.asExpr() = ma.getQualifier() and - succ.asExpr() = ma - ) - } -} - -module MissingHttpOnlyFlow = TaintTracking::Global; - -deprecated query predicate problems( - DataFlow::Node sinkNode, MissingHttpOnlyFlow::PathNode source, MissingHttpOnlyFlow::PathNode sink, - string message1, DataFlow::Node sourceNode, string message2 -) { - MissingHttpOnlyFlow::flowPath(source, sink) and - sinkNode = sink.getNode() and - message1 = "$@ doesn't have the HttpOnly flag set." and - sourceNode = source.getNode() and - message2 = "This sensitive cookie" -} diff --git a/java/ql/test/experimental/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.expected b/java/ql/test/experimental/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.expected deleted file mode 100644 index caecb52fe454..000000000000 --- a/java/ql/test/experimental/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.expected +++ /dev/null @@ -1,67 +0,0 @@ -edges -| SensitiveCookieNotHttpOnly.java:24:33:24:43 | "jwt_token" : String | SensitiveCookieNotHttpOnly.java:25:39:25:52 | tokenCookieStr : String | provenance | | -| SensitiveCookieNotHttpOnly.java:25:28:25:64 | new Cookie(...) : Cookie | SensitiveCookieNotHttpOnly.java:31:28:31:36 | jwtCookie | provenance | Sink:MaD:1 | -| SensitiveCookieNotHttpOnly.java:25:39:25:52 | tokenCookieStr : String | SensitiveCookieNotHttpOnly.java:25:28:25:64 | new Cookie(...) : Cookie | provenance | Config | -| SensitiveCookieNotHttpOnly.java:25:39:25:52 | tokenCookieStr : String | SensitiveCookieNotHttpOnly.java:25:28:25:64 | new Cookie(...) : Cookie | provenance | MaD:4 | -| SensitiveCookieNotHttpOnly.java:42:42:42:49 | "token=" : String | SensitiveCookieNotHttpOnly.java:42:42:42:69 | ... + ... | provenance | Sink:MaD:2 | -| SensitiveCookieNotHttpOnly.java:42:42:42:57 | ... + ... : String | SensitiveCookieNotHttpOnly.java:42:42:42:69 | ... + ... | provenance | Sink:MaD:2 | -| SensitiveCookieNotHttpOnly.java:52:42:52:113 | new NewCookie(...) : NewCookie | SensitiveCookieNotHttpOnly.java:52:42:52:124 | toString(...) | provenance | MaD:5 Sink:MaD:3 | -| SensitiveCookieNotHttpOnly.java:52:56:52:75 | "session-access-key" : String | SensitiveCookieNotHttpOnly.java:52:42:52:113 | new NewCookie(...) : NewCookie | provenance | MaD:6 | -| SensitiveCookieNotHttpOnly.java:63:37:63:115 | new NewCookie(...) : NewCookie | SensitiveCookieNotHttpOnly.java:64:25:64:39 | accessKeyCookie : NewCookie | provenance | | -| SensitiveCookieNotHttpOnly.java:63:51:63:70 | "session-access-key" : String | SensitiveCookieNotHttpOnly.java:63:37:63:115 | new NewCookie(...) : NewCookie | provenance | MaD:6 | -| SensitiveCookieNotHttpOnly.java:64:25:64:39 | accessKeyCookie : NewCookie | SensitiveCookieNotHttpOnly.java:64:25:64:50 | toString(...) : String | provenance | MaD:5 | -| SensitiveCookieNotHttpOnly.java:64:25:64:50 | toString(...) : String | SensitiveCookieNotHttpOnly.java:65:42:65:47 | keyStr | provenance | Sink:MaD:3 | -| SensitiveCookieNotHttpOnly.java:70:28:70:35 | "token=" : String | SensitiveCookieNotHttpOnly.java:71:42:71:50 | secString | provenance | Sink:MaD:2 | -| SensitiveCookieNotHttpOnly.java:70:28:70:43 | ... + ... : String | SensitiveCookieNotHttpOnly.java:71:42:71:50 | secString | provenance | Sink:MaD:2 | -| SensitiveCookieNotHttpOnly.java:70:28:70:55 | ... + ... : String | SensitiveCookieNotHttpOnly.java:71:42:71:50 | secString | provenance | Sink:MaD:2 | -| SensitiveCookieNotHttpOnly.java:88:35:88:51 | "Presto-UI-Token" : String | SensitiveCookieNotHttpOnly.java:89:36:89:51 | PRESTO_UI_COOKIE : String | provenance | | -| SensitiveCookieNotHttpOnly.java:89:25:89:57 | new Cookie(...) : Cookie | SensitiveCookieNotHttpOnly.java:91:16:91:21 | cookie : Cookie | provenance | | -| SensitiveCookieNotHttpOnly.java:89:36:89:51 | PRESTO_UI_COOKIE : String | SensitiveCookieNotHttpOnly.java:89:25:89:57 | new Cookie(...) : Cookie | provenance | Config | -| SensitiveCookieNotHttpOnly.java:89:36:89:51 | PRESTO_UI_COOKIE : String | SensitiveCookieNotHttpOnly.java:89:25:89:57 | new Cookie(...) : Cookie | provenance | MaD:4 | -| SensitiveCookieNotHttpOnly.java:91:16:91:21 | cookie : Cookie | SensitiveCookieNotHttpOnly.java:110:25:110:64 | createAuthenticationCookie(...) : Cookie | provenance | | -| SensitiveCookieNotHttpOnly.java:110:25:110:64 | createAuthenticationCookie(...) : Cookie | SensitiveCookieNotHttpOnly.java:111:28:111:33 | cookie | provenance | Sink:MaD:1 | -models -| 1 | Sink: javax.servlet.http; HttpServletResponse; false; addCookie; ; ; Argument[0]; response-splitting; manual | -| 2 | Sink: javax.servlet.http; HttpServletResponse; false; addHeader; ; ; Argument[0..1]; response-splitting; manual | -| 3 | Sink: javax.servlet.http; HttpServletResponse; false; setHeader; ; ; Argument[0..1]; response-splitting; manual | -| 4 | Summary: javax.servlet.http; Cookie; false; Cookie; ; ; Argument[0]; Argument[this]; taint; manual | -| 5 | Summary: javax.ws.rs.core; Cookie; true; toString; ; ; Argument[this]; ReturnValue; taint; manual | -| 6 | Summary: javax.ws.rs.core; NewCookie; false; NewCookie; ; ; Argument[0..9]; Argument[this]; taint; manual | -nodes -| SensitiveCookieNotHttpOnly.java:24:33:24:43 | "jwt_token" : String | semmle.label | "jwt_token" : String | -| SensitiveCookieNotHttpOnly.java:25:28:25:64 | new Cookie(...) : Cookie | semmle.label | new Cookie(...) : Cookie | -| SensitiveCookieNotHttpOnly.java:25:39:25:52 | tokenCookieStr : String | semmle.label | tokenCookieStr : String | -| SensitiveCookieNotHttpOnly.java:31:28:31:36 | jwtCookie | semmle.label | jwtCookie | -| SensitiveCookieNotHttpOnly.java:42:42:42:49 | "token=" : String | semmle.label | "token=" : String | -| SensitiveCookieNotHttpOnly.java:42:42:42:57 | ... + ... : String | semmle.label | ... + ... : String | -| SensitiveCookieNotHttpOnly.java:42:42:42:69 | ... + ... | semmle.label | ... + ... | -| SensitiveCookieNotHttpOnly.java:52:42:52:113 | new NewCookie(...) : NewCookie | semmle.label | new NewCookie(...) : NewCookie | -| SensitiveCookieNotHttpOnly.java:52:42:52:124 | toString(...) | semmle.label | toString(...) | -| SensitiveCookieNotHttpOnly.java:52:56:52:75 | "session-access-key" : String | semmle.label | "session-access-key" : String | -| SensitiveCookieNotHttpOnly.java:63:37:63:115 | new NewCookie(...) : NewCookie | semmle.label | new NewCookie(...) : NewCookie | -| SensitiveCookieNotHttpOnly.java:63:51:63:70 | "session-access-key" : String | semmle.label | "session-access-key" : String | -| SensitiveCookieNotHttpOnly.java:64:25:64:39 | accessKeyCookie : NewCookie | semmle.label | accessKeyCookie : NewCookie | -| SensitiveCookieNotHttpOnly.java:64:25:64:50 | toString(...) : String | semmle.label | toString(...) : String | -| SensitiveCookieNotHttpOnly.java:65:42:65:47 | keyStr | semmle.label | keyStr | -| SensitiveCookieNotHttpOnly.java:70:28:70:35 | "token=" : String | semmle.label | "token=" : String | -| SensitiveCookieNotHttpOnly.java:70:28:70:43 | ... + ... : String | semmle.label | ... + ... : String | -| SensitiveCookieNotHttpOnly.java:70:28:70:55 | ... + ... : String | semmle.label | ... + ... : String | -| SensitiveCookieNotHttpOnly.java:71:42:71:50 | secString | semmle.label | secString | -| SensitiveCookieNotHttpOnly.java:88:35:88:51 | "Presto-UI-Token" : String | semmle.label | "Presto-UI-Token" : String | -| SensitiveCookieNotHttpOnly.java:89:25:89:57 | new Cookie(...) : Cookie | semmle.label | new Cookie(...) : Cookie | -| SensitiveCookieNotHttpOnly.java:89:36:89:51 | PRESTO_UI_COOKIE : String | semmle.label | PRESTO_UI_COOKIE : String | -| SensitiveCookieNotHttpOnly.java:91:16:91:21 | cookie : Cookie | semmle.label | cookie : Cookie | -| SensitiveCookieNotHttpOnly.java:110:25:110:64 | createAuthenticationCookie(...) : Cookie | semmle.label | createAuthenticationCookie(...) : Cookie | -| SensitiveCookieNotHttpOnly.java:111:28:111:33 | cookie | semmle.label | cookie | -problems -| SensitiveCookieNotHttpOnly.java:31:28:31:36 | jwtCookie | SensitiveCookieNotHttpOnly.java:24:33:24:43 | "jwt_token" : String | SensitiveCookieNotHttpOnly.java:31:28:31:36 | jwtCookie | $@ doesn't have the HttpOnly flag set. | SensitiveCookieNotHttpOnly.java:24:33:24:43 | "jwt_token" | This sensitive cookie | -| SensitiveCookieNotHttpOnly.java:42:42:42:69 | ... + ... | SensitiveCookieNotHttpOnly.java:42:42:42:49 | "token=" : String | SensitiveCookieNotHttpOnly.java:42:42:42:69 | ... + ... | $@ doesn't have the HttpOnly flag set. | SensitiveCookieNotHttpOnly.java:42:42:42:49 | "token=" | This sensitive cookie | -| SensitiveCookieNotHttpOnly.java:42:42:42:69 | ... + ... | SensitiveCookieNotHttpOnly.java:42:42:42:57 | ... + ... : String | SensitiveCookieNotHttpOnly.java:42:42:42:69 | ... + ... | $@ doesn't have the HttpOnly flag set. | SensitiveCookieNotHttpOnly.java:42:42:42:57 | ... + ... | This sensitive cookie | -| SensitiveCookieNotHttpOnly.java:42:42:42:69 | ... + ... | SensitiveCookieNotHttpOnly.java:42:42:42:69 | ... + ... | SensitiveCookieNotHttpOnly.java:42:42:42:69 | ... + ... | $@ doesn't have the HttpOnly flag set. | SensitiveCookieNotHttpOnly.java:42:42:42:69 | ... + ... | This sensitive cookie | -| SensitiveCookieNotHttpOnly.java:52:42:52:124 | toString(...) | SensitiveCookieNotHttpOnly.java:52:56:52:75 | "session-access-key" : String | SensitiveCookieNotHttpOnly.java:52:42:52:124 | toString(...) | $@ doesn't have the HttpOnly flag set. | SensitiveCookieNotHttpOnly.java:52:56:52:75 | "session-access-key" | This sensitive cookie | -| SensitiveCookieNotHttpOnly.java:65:42:65:47 | keyStr | SensitiveCookieNotHttpOnly.java:63:51:63:70 | "session-access-key" : String | SensitiveCookieNotHttpOnly.java:65:42:65:47 | keyStr | $@ doesn't have the HttpOnly flag set. | SensitiveCookieNotHttpOnly.java:63:51:63:70 | "session-access-key" | This sensitive cookie | -| SensitiveCookieNotHttpOnly.java:71:42:71:50 | secString | SensitiveCookieNotHttpOnly.java:70:28:70:35 | "token=" : String | SensitiveCookieNotHttpOnly.java:71:42:71:50 | secString | $@ doesn't have the HttpOnly flag set. | SensitiveCookieNotHttpOnly.java:70:28:70:35 | "token=" | This sensitive cookie | -| SensitiveCookieNotHttpOnly.java:71:42:71:50 | secString | SensitiveCookieNotHttpOnly.java:70:28:70:43 | ... + ... : String | SensitiveCookieNotHttpOnly.java:71:42:71:50 | secString | $@ doesn't have the HttpOnly flag set. | SensitiveCookieNotHttpOnly.java:70:28:70:43 | ... + ... | This sensitive cookie | -| SensitiveCookieNotHttpOnly.java:71:42:71:50 | secString | SensitiveCookieNotHttpOnly.java:70:28:70:55 | ... + ... : String | SensitiveCookieNotHttpOnly.java:71:42:71:50 | secString | $@ doesn't have the HttpOnly flag set. | SensitiveCookieNotHttpOnly.java:70:28:70:55 | ... + ... | This sensitive cookie | -| SensitiveCookieNotHttpOnly.java:111:28:111:33 | cookie | SensitiveCookieNotHttpOnly.java:88:35:88:51 | "Presto-UI-Token" : String | SensitiveCookieNotHttpOnly.java:111:28:111:33 | cookie | $@ doesn't have the HttpOnly flag set. | SensitiveCookieNotHttpOnly.java:88:35:88:51 | "Presto-UI-Token" | This sensitive cookie | -subpaths diff --git a/java/ql/test/experimental/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.java b/java/ql/test/experimental/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.java deleted file mode 100644 index 627575c84034..000000000000 --- a/java/ql/test/experimental/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.java +++ /dev/null @@ -1,164 +0,0 @@ -import java.io.IOException; - -import javax.servlet.http.Cookie; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; -import javax.servlet.ServletException; - -import javax.ws.rs.core.NewCookie; - -import org.springframework.security.web.csrf.CsrfToken; - -class SensitiveCookieNotHttpOnly { - // GOOD - Tests adding a sensitive cookie with the `HttpOnly` flag set. - public void addCookie(String jwt_token, HttpServletRequest request, HttpServletResponse response) { - Cookie jwtCookie = new Cookie("jwt_token", jwt_token); - jwtCookie.setPath("/"); - jwtCookie.setMaxAge(3600*24*7); - jwtCookie.setHttpOnly(true); - response.addCookie(jwtCookie); - } - - // BAD - Tests adding a sensitive cookie without the `HttpOnly` flag set. - public void addCookie2(String jwt_token, String userId, HttpServletRequest request, HttpServletResponse response) { - String tokenCookieStr = "jwt_token"; - Cookie jwtCookie = new Cookie(tokenCookieStr, jwt_token); - Cookie userIdCookie = new Cookie("user_id", userId); - jwtCookie.setPath("/"); - userIdCookie.setPath("/"); - jwtCookie.setMaxAge(3600*24*7); - userIdCookie.setMaxAge(3600*24*7); - response.addCookie(jwtCookie); - response.addCookie(userIdCookie); - } - - // GOOD - Tests set a sensitive cookie header with the `HttpOnly` flag set. - public void addCookie3(String authId, HttpServletRequest request, HttpServletResponse response) { - response.addHeader("Set-Cookie", "token=" +authId + ";HttpOnly;Secure"); - } - - // BAD - Tests set a sensitive cookie header without the `HttpOnly` flag set. - public void addCookie4(String authId, HttpServletRequest request, HttpServletResponse response) { - response.addHeader("Set-Cookie", "token=" +authId + ";Secure"); - } - - // GOOD - Tests set a sensitive cookie header using the class `javax.ws.rs.core.Cookie` with the `HttpOnly` flag set through string concatenation. - public void addCookie5(String accessKey, HttpServletRequest request, HttpServletResponse response) { - response.setHeader("Set-Cookie", new NewCookie("session-access-key", accessKey, "/", null, null, 0, true) + ";HttpOnly"); - } - - // BAD - Tests set a sensitive cookie header using the class `javax.ws.rs.core.Cookie` without the `HttpOnly` flag set. - public void addCookie6(String accessKey, HttpServletRequest request, HttpServletResponse response) { - response.setHeader("Set-Cookie", new NewCookie("session-access-key", accessKey, "/", null, null, 0, true).toString()); - } - - // GOOD - Tests set a sensitive cookie header using the class `javax.ws.rs.core.Cookie` with the `HttpOnly` flag set through the constructor. - public void addCookie7(String accessKey, HttpServletRequest request, HttpServletResponse response) { - NewCookie accessKeyCookie = new NewCookie("session-access-key", accessKey, "/", null, null, 0, true, true); - response.setHeader("Set-Cookie", accessKeyCookie.toString()); - } - - // BAD - Tests set a sensitive cookie header using the class `javax.ws.rs.core.Cookie` without the `HttpOnly` flag set. - public void addCookie8(String accessKey, HttpServletRequest request, HttpServletResponse response) { - NewCookie accessKeyCookie = new NewCookie("session-access-key", accessKey, "/", null, 0, null, 86400, true); - String keyStr = accessKeyCookie.toString(); - response.setHeader("Set-Cookie", keyStr); - } - - // BAD - Tests set a sensitive cookie header using a variable without the `HttpOnly` flag set. - public void addCookie9(String authId, HttpServletRequest request, HttpServletResponse response) { - String secString = "token=" +authId + ";Secure"; - response.addHeader("Set-Cookie", secString); - } - - // GOOD - Tests set a sensitive cookie header with the `HttpOnly` flag set using `String.format(...)`. - public void addCookie10(HttpServletRequest request, HttpServletResponse response) { - response.addHeader("SET-COOKIE", String.format("%s=%s;HttpOnly", "sessionkey", request.getSession().getAttribute("sessionkey"))); - } - - public Cookie createHttpOnlyAuthenticationCookie(HttpServletRequest request, String jwt) { - String PRESTO_UI_COOKIE = "Presto-UI-Token"; - Cookie cookie = new Cookie(PRESTO_UI_COOKIE, jwt); - cookie.setHttpOnly(true); - cookie.setPath("/ui"); - return cookie; - } - - public Cookie createAuthenticationCookie(HttpServletRequest request, String jwt) { - String PRESTO_UI_COOKIE = "Presto-UI-Token"; - Cookie cookie = new Cookie(PRESTO_UI_COOKIE, jwt); - cookie.setPath("/ui"); - return cookie; - } - - public Cookie removeAuthenticationCookie(HttpServletRequest request, String jwt) { - String PRESTO_UI_COOKIE = "Presto-UI-Token"; - Cookie cookie = new Cookie(PRESTO_UI_COOKIE, jwt); - cookie.setPath("/ui"); - cookie.setMaxAge(0); - return cookie; - } - - // GOOD - Tests set a sensitive cookie header with the `HttpOnly` flag set using a wrapper method. - public void addCookie11(HttpServletRequest request, HttpServletResponse response, String jwt) { - Cookie cookie = createHttpOnlyAuthenticationCookie(request, jwt); - response.addCookie(cookie); - } - - // BAD - Tests set a sensitive cookie header without the `HttpOnly` flag set using a wrapper method. - public void addCookie12(HttpServletRequest request, HttpServletResponse response, String jwt) { - Cookie cookie = createAuthenticationCookie(request, jwt); - response.addCookie(cookie); - } - - // GOOD - Tests remove a sensitive cookie header without the `HttpOnly` flag set using a wrapper method. - public void addCookie13(HttpServletRequest request, HttpServletResponse response, String jwt) { - Cookie cookie = removeAuthenticationCookie(request, jwt); - response.addCookie(cookie); - } - - private Cookie createCookie(String name, String value, Boolean httpOnly){ - Cookie cookie = null; - cookie = new Cookie(name, value); - cookie.setDomain("/"); - cookie.setHttpOnly(httpOnly); - - //for production https - cookie.setSecure(true); - - cookie.setMaxAge(60*60*24*30); - cookie.setPath("/"); - - return cookie; - } - - // GOOD - Tests set a sensitive cookie header with the `HttpOnly` flag set through a boolean variable using a wrapper method. - public void addCookie14(HttpServletRequest request, HttpServletResponse response, String refreshToken) { - response.addCookie(createCookie("refresh_token", refreshToken, true)); - } - - // BAD (but not detected) - Tests set a sensitive cookie header with the `HttpOnly` flag not set through a boolean variable using a wrapper method. - // This example is missed because the `cookie.setHttpOnly` call in `createCookie` is thought to maybe set the HTTP-only flag, and the `cookie` - // object flows to this `addCookie` call. - public void addCookie15(HttpServletRequest request, HttpServletResponse response, String refreshToken) { - response.addCookie(createCookie("refresh_token", refreshToken, false)); - } - - // GOOD - CSRF token doesn't need to have the `HttpOnly` flag set. - public void addCsrfCookie(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { - // Spring put the CSRF token in session attribute "_csrf" - CsrfToken csrfToken = (CsrfToken) request.getAttribute("_csrf"); - - // Send the cookie only if the token has changed - String actualToken = request.getHeader("X-CSRF-TOKEN"); - if (actualToken == null || !actualToken.equals(csrfToken.getToken())) { - // Session cookie that can be used by AngularJS - String pCookieName = "CSRF-TOKEN"; - Cookie cookie = new Cookie(pCookieName, csrfToken.getToken()); - cookie.setMaxAge(-1); - cookie.setHttpOnly(false); - cookie.setPath("/"); - response.addCookie(cookie); - } - } -} diff --git a/java/ql/test/experimental/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.qlref b/java/ql/test/experimental/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.qlref deleted file mode 100644 index 9c7ce3d63299..000000000000 --- a/java/ql/test/experimental/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.qlref +++ /dev/null @@ -1,2 +0,0 @@ -query: experimental/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql -postprocess: utils/test/PrettyPrintModels.ql diff --git a/java/ql/test/experimental/query-tests/security/CWE-1004/options b/java/ql/test/experimental/query-tests/security/CWE-1004/options deleted file mode 100644 index 00e92689af58..000000000000 --- a/java/ql/test/experimental/query-tests/security/CWE-1004/options +++ /dev/null @@ -1 +0,0 @@ -// semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/servlet-api-2.4:${testdir}/../../../../stubs/jsr311-api-1.1.1:${testdir}/../../../../stubs/springframework-5.8.x From 1c542965459e1860a612a08e7ccac935571cb1d5 Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Thu, 2 Oct 2025 10:42:23 +0100 Subject: [PATCH 05/10] Add change note --- .../src/change-notes/2025-10-02-http-only-cookie-promote.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 java/ql/src/change-notes/2025-10-02-http-only-cookie-promote.md diff --git a/java/ql/src/change-notes/2025-10-02-http-only-cookie-promote.md b/java/ql/src/change-notes/2025-10-02-http-only-cookie-promote.md new file mode 100644 index 000000000000..ee9fe7527bd5 --- /dev/null +++ b/java/ql/src/change-notes/2025-10-02-http-only-cookie-promote.md @@ -0,0 +1,4 @@ +--- +category: newQuery +--- +* The `java/sensitive-cookie-not-httponly` query has been promoted from experimental to the main query pack. \ No newline at end of file From 696ec29dae5170c00e8dc3dfc928223e475b9a55 Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Thu, 2 Oct 2025 14:37:02 +0100 Subject: [PATCH 06/10] Upgrade integration tests --- .../java/query-suite/java-security-and-quality.qls.expected | 1 + .../java/query-suite/java-security-extended.qls.expected | 1 + 2 files changed, 2 insertions(+) diff --git a/java/ql/integration-tests/java/query-suite/java-security-and-quality.qls.expected b/java/ql/integration-tests/java/query-suite/java-security-and-quality.qls.expected index f5470c463c30..7e4401bcce9d 100644 --- a/java/ql/integration-tests/java/query-suite/java-security-and-quality.qls.expected +++ b/java/ql/integration-tests/java/query-suite/java-security-and-quality.qls.expected @@ -127,6 +127,7 @@ ql/java/ql/src/Security/CWE/CWE-094/JexlInjection.ql ql/java/ql/src/Security/CWE/CWE-094/MvelInjection.ql ql/java/ql/src/Security/CWE/CWE-094/SpelInjection.ql ql/java/ql/src/Security/CWE/CWE-094/TemplateInjection.ql +ql/java/ql/src/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql ql/java/ql/src/Security/CWE/CWE-1104/MavenPomDependsOnBintray.ql ql/java/ql/src/Security/CWE/CWE-113/NettyResponseSplitting.ql ql/java/ql/src/Security/CWE/CWE-113/ResponseSplitting.ql diff --git a/java/ql/integration-tests/java/query-suite/java-security-extended.qls.expected b/java/ql/integration-tests/java/query-suite/java-security-extended.qls.expected index a3ebc029d287..6ebf204a1a80 100644 --- a/java/ql/integration-tests/java/query-suite/java-security-extended.qls.expected +++ b/java/ql/integration-tests/java/query-suite/java-security-extended.qls.expected @@ -113,6 +113,7 @@ ql/java/ql/src/Security/CWE/CWE-927/ImplicitPendingIntents.ql ql/java/ql/src/Security/CWE/CWE-927/SensitiveCommunication.ql ql/java/ql/src/Security/CWE/CWE-927/SensitiveResultReceiver.ql ql/java/ql/src/Security/CWE/CWE-940/AndroidIntentRedirection.ql +ql/java/ql/src/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql ql/java/ql/src/Telemetry/DatabaseQualityDiagnostics.ql ql/java/ql/src/Telemetry/ExternalLibraryUsage.ql ql/java/ql/src/Telemetry/ExtractorInformation.ql From 093b04f79f342ec0ea0e2d16597d270296d455ad Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Thu, 2 Oct 2025 15:31:26 +0100 Subject: [PATCH 07/10] Update comments --- .../Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/java/ql/src/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql b/java/ql/src/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql index 4eae5f1ee4eb..d301adbcb996 100644 --- a/java/ql/src/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql +++ b/java/ql/src/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql @@ -14,9 +14,9 @@ * Sketch of the structure of this query: we track cookie names that appear to be sensitive * (e.g. `session` or `token`) to a `ServletResponse.addHeader(...)` or `.addCookie(...)` * method that does not set the `httpOnly` flag. Subsidiary configurations - * `MatchesHttpOnlyToRawHeaderConfiguration` and `SetHttpOnlyInCookieConfiguration` are used to establish + * `MatchesHttpOnlyToRawHeaderConfig` and `SetHttpOnlyInCookieConfig` are used to establish * when the `httpOnly` flag is likely to have been set, before configuration - * `MissingHttpOnlyConfiguration` establishes that a non-`httpOnly` cookie has a sensitive-seeming name. + * `MissingHttpOnlyConfig` establishes that a non-`httpOnly` cookie has a sensitive-seeming name. */ import java @@ -158,8 +158,8 @@ predicate setsHttpOnlyInNewCookie(ClassInstanceExpr cie) { /** * A taint configuration tracking flow from a sensitive cookie without the `HttpOnly` flag * set to its HTTP response. - * Tracks string literals containing sensitive names (`SensitiveNameExpr`), to an `addCookie` call (as a `Cookie` object) - * or an `addHeader` call (as a string) (`CookieResponseWithoutHttpOnly`). + * Tracks string literals containing sensitive names (`SensitiveCookieNameExpr`), to an `addCookie` call (as a `Cookie` object) + * or an `addHeader` call (as a string) (`CookieResponseWithoutHttpOnlySink`). * Passes through `Cookie` constructors and `toString` calls. */ module MissingHttpOnlyConfig implements DataFlow::ConfigSig { From 9cb593b020ef82b81f879b7bca2c1ca670cd5890 Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Thu, 9 Oct 2025 16:01:20 +0100 Subject: [PATCH 08/10] Update tests --- .../java-security-extended.qls.expected | 2 +- .../query-suite/not_included_in_qls.expected | 1 - .../SensitiveCookieNotHttpOnly.expected | 35 +++++++++++-------- .../CWE-1004/SensitiveCookieNotHttpOnly.qlref | 4 ++- 4 files changed, 25 insertions(+), 17 deletions(-) diff --git a/java/ql/integration-tests/java/query-suite/java-security-extended.qls.expected b/java/ql/integration-tests/java/query-suite/java-security-extended.qls.expected index 6ebf204a1a80..b2981db13c2b 100644 --- a/java/ql/integration-tests/java/query-suite/java-security-extended.qls.expected +++ b/java/ql/integration-tests/java/query-suite/java-security-extended.qls.expected @@ -30,6 +30,7 @@ ql/java/ql/src/Security/CWE/CWE-094/JexlInjection.ql ql/java/ql/src/Security/CWE/CWE-094/MvelInjection.ql ql/java/ql/src/Security/CWE/CWE-094/SpelInjection.ql ql/java/ql/src/Security/CWE/CWE-094/TemplateInjection.ql +ql/java/ql/src/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql ql/java/ql/src/Security/CWE/CWE-1104/MavenPomDependsOnBintray.ql ql/java/ql/src/Security/CWE/CWE-113/NettyResponseSplitting.ql ql/java/ql/src/Security/CWE/CWE-113/ResponseSplitting.ql @@ -113,7 +114,6 @@ ql/java/ql/src/Security/CWE/CWE-927/ImplicitPendingIntents.ql ql/java/ql/src/Security/CWE/CWE-927/SensitiveCommunication.ql ql/java/ql/src/Security/CWE/CWE-927/SensitiveResultReceiver.ql ql/java/ql/src/Security/CWE/CWE-940/AndroidIntentRedirection.ql -ql/java/ql/src/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql ql/java/ql/src/Telemetry/DatabaseQualityDiagnostics.ql ql/java/ql/src/Telemetry/ExternalLibraryUsage.ql ql/java/ql/src/Telemetry/ExtractorInformation.ql diff --git a/java/ql/integration-tests/java/query-suite/not_included_in_qls.expected b/java/ql/integration-tests/java/query-suite/not_included_in_qls.expected index d1b6428ae227..1aa63644947a 100644 --- a/java/ql/integration-tests/java/query-suite/not_included_in_qls.expected +++ b/java/ql/integration-tests/java/query-suite/not_included_in_qls.expected @@ -190,7 +190,6 @@ ql/java/ql/src/experimental/Security/CWE/CWE-094/ScriptInjection.ql ql/java/ql/src/experimental/Security/CWE/CWE-094/SpringImplicitViewManipulation.ql ql/java/ql/src/experimental/Security/CWE/CWE-094/SpringViewManipulation.ql ql/java/ql/src/experimental/Security/CWE/CWE-1004/InsecureTomcatConfig.ql -ql/java/ql/src/experimental/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql ql/java/ql/src/experimental/Security/CWE/CWE-200/InsecureWebResourceResponse.ql ql/java/ql/src/experimental/Security/CWE/CWE-200/SensitiveAndroidFileLeak.ql ql/java/ql/src/experimental/Security/CWE/CWE-208/PossibleTimingAttackAgainstSignature.ql diff --git a/java/ql/test/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.expected b/java/ql/test/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.expected index 71c73f3921e9..f00a00c72586 100644 --- a/java/ql/test/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.expected +++ b/java/ql/test/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.expected @@ -11,26 +11,33 @@ | SensitiveCookieNotHttpOnly.java:111:28:111:33 | cookie | SensitiveCookieNotHttpOnly.java:88:35:88:51 | "Presto-UI-Token" : String | SensitiveCookieNotHttpOnly.java:111:28:111:33 | cookie | $@ doesn't have the HttpOnly flag set. | SensitiveCookieNotHttpOnly.java:88:35:88:51 | "Presto-UI-Token" : String | This sensitive cookie | edges | SensitiveCookieNotHttpOnly.java:24:33:24:43 | "jwt_token" : String | SensitiveCookieNotHttpOnly.java:25:39:25:52 | tokenCookieStr : String | provenance | | -| SensitiveCookieNotHttpOnly.java:25:28:25:64 | new Cookie(...) : Cookie | SensitiveCookieNotHttpOnly.java:31:28:31:36 | jwtCookie | provenance | Sink:MaD:46211 | +| SensitiveCookieNotHttpOnly.java:25:28:25:64 | new Cookie(...) : Cookie | SensitiveCookieNotHttpOnly.java:31:28:31:36 | jwtCookie | provenance | Sink:MaD:1 | | SensitiveCookieNotHttpOnly.java:25:39:25:52 | tokenCookieStr : String | SensitiveCookieNotHttpOnly.java:25:28:25:64 | new Cookie(...) : Cookie | provenance | Config | -| SensitiveCookieNotHttpOnly.java:25:39:25:52 | tokenCookieStr : String | SensitiveCookieNotHttpOnly.java:25:28:25:64 | new Cookie(...) : Cookie | provenance | MaD:46217 | -| SensitiveCookieNotHttpOnly.java:42:42:42:49 | "token=" : String | SensitiveCookieNotHttpOnly.java:42:42:42:69 | ... + ... | provenance | Sink:MaD:46212 | -| SensitiveCookieNotHttpOnly.java:42:42:42:57 | ... + ... : String | SensitiveCookieNotHttpOnly.java:42:42:42:69 | ... + ... | provenance | Sink:MaD:46212 | -| SensitiveCookieNotHttpOnly.java:52:42:52:113 | new NewCookie(...) : NewCookie | SensitiveCookieNotHttpOnly.java:52:42:52:124 | toString(...) | provenance | MaD:46260 Sink:MaD:46214 | -| SensitiveCookieNotHttpOnly.java:52:56:52:75 | "session-access-key" : String | SensitiveCookieNotHttpOnly.java:52:42:52:113 | new NewCookie(...) : NewCookie | provenance | MaD:46298 | +| SensitiveCookieNotHttpOnly.java:25:39:25:52 | tokenCookieStr : String | SensitiveCookieNotHttpOnly.java:25:28:25:64 | new Cookie(...) : Cookie | provenance | MaD:4 | +| SensitiveCookieNotHttpOnly.java:42:42:42:49 | "token=" : String | SensitiveCookieNotHttpOnly.java:42:42:42:69 | ... + ... | provenance | Sink:MaD:2 | +| SensitiveCookieNotHttpOnly.java:42:42:42:57 | ... + ... : String | SensitiveCookieNotHttpOnly.java:42:42:42:69 | ... + ... | provenance | Sink:MaD:2 | +| SensitiveCookieNotHttpOnly.java:52:42:52:113 | new NewCookie(...) : NewCookie | SensitiveCookieNotHttpOnly.java:52:42:52:124 | toString(...) | provenance | MaD:5 Sink:MaD:3 | +| SensitiveCookieNotHttpOnly.java:52:56:52:75 | "session-access-key" : String | SensitiveCookieNotHttpOnly.java:52:42:52:113 | new NewCookie(...) : NewCookie | provenance | MaD:6 | | SensitiveCookieNotHttpOnly.java:63:37:63:115 | new NewCookie(...) : NewCookie | SensitiveCookieNotHttpOnly.java:64:25:64:39 | accessKeyCookie : NewCookie | provenance | | -| SensitiveCookieNotHttpOnly.java:63:51:63:70 | "session-access-key" : String | SensitiveCookieNotHttpOnly.java:63:37:63:115 | new NewCookie(...) : NewCookie | provenance | MaD:46298 | -| SensitiveCookieNotHttpOnly.java:64:25:64:39 | accessKeyCookie : NewCookie | SensitiveCookieNotHttpOnly.java:64:25:64:50 | toString(...) : String | provenance | MaD:46260 | -| SensitiveCookieNotHttpOnly.java:64:25:64:50 | toString(...) : String | SensitiveCookieNotHttpOnly.java:65:42:65:47 | keyStr | provenance | Sink:MaD:46214 | -| SensitiveCookieNotHttpOnly.java:70:28:70:35 | "token=" : String | SensitiveCookieNotHttpOnly.java:71:42:71:50 | secString | provenance | Sink:MaD:46212 | -| SensitiveCookieNotHttpOnly.java:70:28:70:43 | ... + ... : String | SensitiveCookieNotHttpOnly.java:71:42:71:50 | secString | provenance | Sink:MaD:46212 | -| SensitiveCookieNotHttpOnly.java:70:28:70:55 | ... + ... : String | SensitiveCookieNotHttpOnly.java:71:42:71:50 | secString | provenance | Sink:MaD:46212 | +| SensitiveCookieNotHttpOnly.java:63:51:63:70 | "session-access-key" : String | SensitiveCookieNotHttpOnly.java:63:37:63:115 | new NewCookie(...) : NewCookie | provenance | MaD:6 | +| SensitiveCookieNotHttpOnly.java:64:25:64:39 | accessKeyCookie : NewCookie | SensitiveCookieNotHttpOnly.java:64:25:64:50 | toString(...) : String | provenance | MaD:5 | +| SensitiveCookieNotHttpOnly.java:64:25:64:50 | toString(...) : String | SensitiveCookieNotHttpOnly.java:65:42:65:47 | keyStr | provenance | Sink:MaD:3 | +| SensitiveCookieNotHttpOnly.java:70:28:70:35 | "token=" : String | SensitiveCookieNotHttpOnly.java:71:42:71:50 | secString | provenance | Sink:MaD:2 | +| SensitiveCookieNotHttpOnly.java:70:28:70:43 | ... + ... : String | SensitiveCookieNotHttpOnly.java:71:42:71:50 | secString | provenance | Sink:MaD:2 | +| SensitiveCookieNotHttpOnly.java:70:28:70:55 | ... + ... : String | SensitiveCookieNotHttpOnly.java:71:42:71:50 | secString | provenance | Sink:MaD:2 | | SensitiveCookieNotHttpOnly.java:88:35:88:51 | "Presto-UI-Token" : String | SensitiveCookieNotHttpOnly.java:89:36:89:51 | PRESTO_UI_COOKIE : String | provenance | | | SensitiveCookieNotHttpOnly.java:89:25:89:57 | new Cookie(...) : Cookie | SensitiveCookieNotHttpOnly.java:91:16:91:21 | cookie : Cookie | provenance | | | SensitiveCookieNotHttpOnly.java:89:36:89:51 | PRESTO_UI_COOKIE : String | SensitiveCookieNotHttpOnly.java:89:25:89:57 | new Cookie(...) : Cookie | provenance | Config | -| SensitiveCookieNotHttpOnly.java:89:36:89:51 | PRESTO_UI_COOKIE : String | SensitiveCookieNotHttpOnly.java:89:25:89:57 | new Cookie(...) : Cookie | provenance | MaD:46217 | +| SensitiveCookieNotHttpOnly.java:89:36:89:51 | PRESTO_UI_COOKIE : String | SensitiveCookieNotHttpOnly.java:89:25:89:57 | new Cookie(...) : Cookie | provenance | MaD:4 | | SensitiveCookieNotHttpOnly.java:91:16:91:21 | cookie : Cookie | SensitiveCookieNotHttpOnly.java:110:25:110:64 | createAuthenticationCookie(...) : Cookie | provenance | | -| SensitiveCookieNotHttpOnly.java:110:25:110:64 | createAuthenticationCookie(...) : Cookie | SensitiveCookieNotHttpOnly.java:111:28:111:33 | cookie | provenance | Sink:MaD:46211 | +| SensitiveCookieNotHttpOnly.java:110:25:110:64 | createAuthenticationCookie(...) : Cookie | SensitiveCookieNotHttpOnly.java:111:28:111:33 | cookie | provenance | Sink:MaD:1 | +models +| 1 | Sink: javax.servlet.http; HttpServletResponse; false; addCookie; ; ; Argument[0]; response-splitting; manual | +| 2 | Sink: javax.servlet.http; HttpServletResponse; false; addHeader; ; ; Argument[0..1]; response-splitting; manual | +| 3 | Sink: javax.servlet.http; HttpServletResponse; false; setHeader; ; ; Argument[0..1]; response-splitting; manual | +| 4 | Summary: javax.servlet.http; Cookie; false; Cookie; ; ; Argument[0]; Argument[this]; taint; manual | +| 5 | Summary: javax.ws.rs.core; Cookie; true; toString; ; ; Argument[this]; ReturnValue; taint; manual | +| 6 | Summary: javax.ws.rs.core; NewCookie; false; NewCookie; ; ; Argument[0..9]; Argument[this]; taint; manual | nodes | SensitiveCookieNotHttpOnly.java:24:33:24:43 | "jwt_token" : String | semmle.label | "jwt_token" : String | | SensitiveCookieNotHttpOnly.java:25:28:25:64 | new Cookie(...) : Cookie | semmle.label | new Cookie(...) : Cookie | diff --git a/java/ql/test/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.qlref b/java/ql/test/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.qlref index e3b99f6e6855..fd347f0adf80 100644 --- a/java/ql/test/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.qlref +++ b/java/ql/test/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.qlref @@ -1,2 +1,4 @@ query: Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql -postprocess: utils/test/InlineExpectationsTestQuery.ql \ No newline at end of file +postprocess: +- utils/test/InlineExpectationsTestQuery.ql +- utils/test/PrettyPrintModels.ql \ No newline at end of file From d8b37d0cde0ef12833b9641c929bbee1a81e92b5 Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Tue, 14 Oct 2025 16:03:40 +0100 Subject: [PATCH 09/10] Review suggestions - update comments and description --- .../CWE-1004/SensitiveCookieNotHttpOnly.ql | 20 +++++++++++-------- .../CWE-1004/SensitiveCookieNotHttpOnly.java | 4 ++-- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/java/ql/src/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql b/java/ql/src/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql index d301adbcb996..494e851a5333 100644 --- a/java/ql/src/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql +++ b/java/ql/src/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql @@ -1,10 +1,11 @@ /** * @name Sensitive cookies without the HttpOnly response header set - * @description Sensitive cookies without the 'HttpOnly' flag set leaves session cookies vulnerable to + * @description A sensitive cookie without the 'HttpOnly' flag set may be vulnerable to * an XSS attack. * @kind path-problem * @problem.severity warning - * @precision medium + * @precision high + * @security-severity 5.0 * @id java/sensitive-cookie-not-httponly * @tags security * external/cwe/cwe-1004 @@ -101,8 +102,9 @@ predicate removesCookie(MethodCall ma) { } /** - * A taint configuration tracking flow of a method that sets the `HttpOnly` flag, - * or one that removes a cookie, to a `ServletResponse.addCookie` call. + * A taint configuration tracking the flow of a cookie that has had the + * `HttpOnly` flag set, or has been removed, to a `ServletResponse.addCookie` + * call. */ module SetHttpOnlyOrRemovesCookieToAddCookieConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { @@ -120,8 +122,8 @@ module SetHttpOnlyOrRemovesCookieToAddCookieFlow = TaintTracking::Global; /** - * A cookie that is added to an HTTP response and which doesn't have `httpOnly` set, used as a sink - * in `MissingHttpOnlyConfiguration`. + * A cookie that is added to an HTTP response and which doesn't have `HttpOnly` set, used as a sink + * in `MissingHttpOnlyConfig`. */ class CookieResponseWithoutHttpOnlySink extends DataFlow::ExprNode { CookieResponseWithoutHttpOnlySink() { @@ -157,9 +159,11 @@ predicate setsHttpOnlyInNewCookie(ClassInstanceExpr cie) { /** * A taint configuration tracking flow from a sensitive cookie without the `HttpOnly` flag - * set to its HTTP response. + * set to an HTTP response. + * * Tracks string literals containing sensitive names (`SensitiveCookieNameExpr`), to an `addCookie` call (as a `Cookie` object) * or an `addHeader` call (as a string) (`CookieResponseWithoutHttpOnlySink`). + * * Passes through `Cookie` constructors and `toString` calls. */ module MissingHttpOnlyConfig implements DataFlow::ConfigSig { @@ -169,7 +173,7 @@ module MissingHttpOnlyConfig implements DataFlow::ConfigSig { predicate isBarrier(DataFlow::Node node) { // JAX-RS's `new NewCookie("session-access-key", accessKey, "/", null, null, 0, true, true)` and similar - // Cookie constructors, but barriers to considering the flow of the sensitive name, as httponly flag is set. + // Cookie constructors that set the `HttpOnly` flag are considered barriers to the flow of sensitive names. setsHttpOnlyInNewCookie(node.asExpr()) } diff --git a/java/ql/test/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.java b/java/ql/test/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.java index a57a502336f8..91f8f3ad4cee 100644 --- a/java/ql/test/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.java +++ b/java/ql/test/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.java @@ -16,7 +16,7 @@ public void addCookie(String jwt_token, HttpServletRequest request, HttpServletR jwtCookie.setPath("/"); jwtCookie.setMaxAge(3600*24*7); jwtCookie.setHttpOnly(true); - response.addCookie(jwtCookie); + response.addCookie(jwtCookie); } // BAD - Tests adding a sensitive cookie without the `HttpOnly` flag set. @@ -29,7 +29,7 @@ public void addCookie2(String jwt_token, String userId, HttpServletRequest reque jwtCookie.setMaxAge(3600*24*7); userIdCookie.setMaxAge(3600*24*7); response.addCookie(jwtCookie); // $Alert - response.addCookie(userIdCookie); + response.addCookie(userIdCookie); } // GOOD - Tests set a sensitive cookie header with the `HttpOnly` flag set. From e95e1a0386a85d7bb1d2777c147e1036c6766953 Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Tue, 14 Oct 2025 16:27:28 +0100 Subject: [PATCH 10/10] Update integration test --- .../java/query-suite/java-code-scanning.qls.expected | 1 + 1 file changed, 1 insertion(+) diff --git a/java/ql/integration-tests/java/query-suite/java-code-scanning.qls.expected b/java/ql/integration-tests/java/query-suite/java-code-scanning.qls.expected index afa6cebba311..dd67b7df413b 100644 --- a/java/ql/integration-tests/java/query-suite/java-code-scanning.qls.expected +++ b/java/ql/integration-tests/java/query-suite/java-code-scanning.qls.expected @@ -21,6 +21,7 @@ ql/java/ql/src/Security/CWE/CWE-094/JexlInjection.ql ql/java/ql/src/Security/CWE/CWE-094/MvelInjection.ql ql/java/ql/src/Security/CWE/CWE-094/SpelInjection.ql ql/java/ql/src/Security/CWE/CWE-094/TemplateInjection.ql +ql/java/ql/src/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql ql/java/ql/src/Security/CWE/CWE-1104/MavenPomDependsOnBintray.ql ql/java/ql/src/Security/CWE/CWE-113/NettyResponseSplitting.ql ql/java/ql/src/Security/CWE/CWE-113/ResponseSplitting.ql