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..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 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/src/experimental/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.java b/java/ql/src/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.java similarity index 100% rename from java/ql/src/experimental/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.java rename to java/ql/src/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.java diff --git a/java/ql/src/experimental/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.qhelp b/java/ql/src/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.qhelp similarity index 58% rename from java/ql/src/experimental/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.qhelp rename to java/ql/src/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.qhelp index ee3e8a4181a9..71e016510e26 100644 --- a/java/ql/src/experimental/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/experimental/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql b/java/ql/src/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql similarity index 72% rename from java/ql/src/experimental/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql rename to java/ql/src/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql index fa5237d32bb9..d301adbcb996 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql +++ b/java/ql/src/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql @@ -7,7 +7,6 @@ * @precision medium * @id java/sensitive-cookie-not-httponly * @tags security - * experimental * external/cwe/cwe-1004 */ @@ -15,16 +14,15 @@ * 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 + * `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 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).*" } @@ -50,8 +48,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 @@ -62,19 +60,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 { @@ -102,30 +100,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() @@ -137,25 +116,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") + ) ) } } @@ -179,14 +158,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 (`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 { 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()) } @@ -212,13 +195,8 @@ 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" -} +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/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 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 diff --git a/java/ql/test/experimental/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.expected b/java/ql/test/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.expected similarity index 93% rename from java/ql/test/experimental/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.expected rename to java/ql/test/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.expected index caecb52fe454..f00a00c72586 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.expected +++ b/java/ql/test/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.expected @@ -1,3 +1,14 @@ +#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:1 | @@ -53,15 +64,4 @@ nodes | 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/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.java similarity index 91% rename from java/ql/test/experimental/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.java rename to java/ql/test/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.java index 627575c84034..a57a502336f8 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.java +++ b/java/ql/test/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.java @@ -16,20 +16,20 @@ 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. public void addCookie2(String jwt_token, String userId, HttpServletRequest request, HttpServletResponse response) { - String tokenCookieStr = "jwt_token"; + 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); - response.addCookie(userIdCookie); + response.addCookie(jwtCookie); // $Alert + response.addCookie(userIdCookie); } // GOOD - Tests set a sensitive cookie header with the `HttpOnly` flag set. @@ -39,7 +39,7 @@ public void addCookie3(String authId, HttpServletRequest request, HttpServletRes // 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"); + 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. @@ -49,7 +49,7 @@ public void addCookie5(String accessKey, HttpServletRequest request, HttpServlet // 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()); + 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. @@ -60,15 +60,15 @@ public void addCookie7(String accessKey, HttpServletRequest request, HttpServlet // 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); + NewCookie accessKeyCookie = new NewCookie("session-access-key", accessKey, "/", null, 0, null, 86400, true); // $Source String keyStr = accessKeyCookie.toString(); - response.setHeader("Set-Cookie", keyStr); + 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"; - response.addHeader("Set-Cookie", secString); + 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(...)`. @@ -85,7 +85,7 @@ public Cookie createHttpOnlyAuthenticationCookie(HttpServletRequest request, Str } public Cookie createAuthenticationCookie(HttpServletRequest request, String jwt) { - String PRESTO_UI_COOKIE = "Presto-UI-Token"; + String PRESTO_UI_COOKIE = "Presto-UI-Token"; // $Source Cookie cookie = new Cookie(PRESTO_UI_COOKIE, jwt); cookie.setPath("/ui"); return cookie; @@ -108,7 +108,7 @@ public void addCookie11(HttpServletRequest request, HttpServletResponse response // 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); + response.addCookie(cookie); // $Alert } // GOOD - Tests remove a sensitive cookie header without the `HttpOnly` flag set using a wrapper method. @@ -141,7 +141,7 @@ public void addCookie14(HttpServletRequest request, HttpServletResponse response // 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)); + response.addCookie(createCookie("refresh_token", refreshToken, false)); // $MISSING:Alert } // GOOD - CSRF token doesn't need to have the `HttpOnly` flag set. 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..fd347f0adf80 --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.qlref @@ -0,0 +1,4 @@ +query: Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql +postprocess: +- utils/test/InlineExpectationsTestQuery.ql +- utils/test/PrettyPrintModels.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