mirror of
https://github.com/github/codeql.git
synced 2025-12-16 16:53:25 +01:00
Copy experimental query to main
This commit is contained in:
Joe Farebrother
@@ -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());
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,27 @@
|
||||
<!DOCTYPE qhelp SYSTEM "qhelp.dtd">
|
||||
<qhelp>
|
||||
|
||||
<overview>
|
||||
<p>Cross-Site Scripting (XSS) is categorized as one of the OWASP Top 10 Security Vulnerabilities. The <code>HttpOnly</code> flag directs compatible browsers to prevent client-side script from accessing cookies. Including the <code>HttpOnly</code> 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.</p>
|
||||
</overview>
|
||||
|
||||
<recommendation>
|
||||
<p>Use the <code>HttpOnly</code> flag when generating a cookie containing sensitive information to help mitigate the risk of client side script accessing the protected cookie.</p>
|
||||
</recommendation>
|
||||
|
||||
<example>
|
||||
<p>The following example shows two ways of generating sensitive cookies. In the 'BAD' cases, the <code>HttpOnly</code> flag is not set. In the 'GOOD' cases, the <code>HttpOnly</code> flag is set.</p>
|
||||
<sample src="SensitiveCookieNotHttpOnly.java" />
|
||||
</example>
|
||||
|
||||
<references>
|
||||
<li>
|
||||
PortSwigger:
|
||||
<a href="https://portswigger.net/kb/issues/00500600_cookie-without-httponly-flag-set">Cookie without HttpOnly flag set</a>
|
||||
</li>
|
||||
<li>
|
||||
OWASP:
|
||||
<a href="https://owasp.org/www-community/HttpOnly">HttpOnly</a>
|
||||
</li>
|
||||
</references>
|
||||
</qhelp>
|
||||
223
java/ql/src/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql
Normal file
223
java/ql/src/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql
Normal file
@@ -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<MatchesHttpOnlyConfig>;
|
||||
|
||||
/** 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<SetHttpOnlyOrRemovesCookieConfig>;
|
||||
|
||||
/**
|
||||
* 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<MissingHttpOnlyConfig>;
|
||||
|
||||
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"
|
||||
}
|
||||
Reference in New Issue
Block a user