hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-30 19:26:02 +02:00
Code Issues Packages Projects Releases Wiki Activity

Update the query to accommodate more cases

Browse Source
This commit is contained in:
luchua-bc
2021-03-16 03:18:26 +00:00
parent 0a35feef76
commit 08c3bf26d5
3 changed files with 130 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

73
java/ql/src/experimental/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql
View File

@@ -33,7 +33,23 @@ predicate isSensitiveCookieNameExpr(Expr expr) {
/** Holds if a string is concatenated with the `HttpOnly` flag. */
predicate hasHttpOnlyExpr(Expr expr) {
expr.(CompileTimeConstantExpr).getStringValue().toLowerCase().matches("%httponly%") or
(
expr.(CompileTimeConstantExpr).getStringValue().toLowerCase().matches("%httponly%")
or
exists(
StaticMethodAccess ma // String.format("%s=%s;HttpOnly", "sessionkey", sessionKey)
|
ma.getType().getName() = "String" and
ma.getMethod().getName() = "format" and
ma.getArgument(0)
.(CompileTimeConstantExpr)
.getStringValue()
.toLowerCase()
.matches("%httponly%") and
expr = ma
)
)
or
hasHttpOnlyExpr(expr.(AddExpr).getAnOperand())
}
@@ -56,6 +72,40 @@ class CookieClass extends RefType {
}
}
/** Holds if the `Expr` expr is evaluated to boolean true. */
predicate isBooleanTrue(Expr expr) {
expr.(CompileTimeConstantExpr).getBooleanValue() = true or
expr.(VarAccess).getVariable().getAnAssignedValue().(CompileTimeConstantExpr).getBooleanValue() =
true
}
/** Holds if the method or a wrapper method sets the `HttpOnly` flag. */
predicate setHttpOnlyInCookie(MethodAccess ma) {
ma.getMethod().getName() = "setHttpOnly" and
(
isBooleanTrue(ma.getArgument(0)) // boolean literal true
or
exists(
MethodAccess mpa, int i // runtime assignment of boolean value true
|
TaintTracking::localTaint(DataFlow::parameterNode(mpa.getMethod().getParameter(i)),
DataFlow::exprNode(ma.getArgument(0))) and
isBooleanTrue(mpa.getArgument(i))
)
)
or
exists(MethodAccess mca |
ma.getMethod().calls(mca.getMethod()) and
setHttpOnlyInCookie(mca)
)
}
/** Holds if the method or a wrapper method removes a cookie. */
predicate removeCookie(MethodAccess ma) {
ma.getMethod().getName() = "setMaxAge" and
ma.getArgument(0).(IntegerLiteral).getIntValue() = 0
}
/** Sensitive cookie name used in a `Cookie` constructor or a `Set-Cookie` call. */
class SensitiveCookieNameExpr extends Expr {
SensitiveCookieNameExpr() { isSensitiveCookieNameExpr(this) }
@@ -69,11 +119,16 @@ class CookieResponseSink extends DataFlow::ExprNode {
ma.getMethod() instanceof ResponseAddCookieMethod and
this.getExpr() = ma.getArgument(0) and
not exists(
MethodAccess ma2 // cookie.setHttpOnly(true)
MethodAccess ma2 // a method or wrapper method that invokes cookie.setHttpOnly(true)
|
ma2.getMethod().getName() = "setHttpOnly" and
ma2.getArgument(0).(BooleanLiteral).getBooleanValue() = true and
DataFlow::localExprFlow(ma2.getQualifier(), this.getExpr())
(
setHttpOnlyInCookie(ma2) or
removeCookie(ma2)
) and
(
DataFlow::localExprFlow(ma2.getQualifier(), this.getExpr()) or
DataFlow::localExprFlow(ma2, this.getExpr())
)
)
or
ma instanceof SetCookieMethodAccess and
@@ -92,13 +147,15 @@ class CookieResponseSink extends DataFlow::ExprNode {
predicate setHttpOnlyInNewCookie(ClassInstanceExpr cie) {
cie.getConstructedType().hasQualifiedName(["javax.ws.rs.core", "jakarta.ws.rs.core"], "NewCookie") and
(
cie.getNumArgument() = 6 and cie.getArgument(5).(BooleanLiteral).getBooleanValue() = true // NewCookie(Cookie cookie, String comment, int maxAge, Date expiry, boolean secure, boolean httpOnly)
cie.getNumArgument() = 6 and
isBooleanTrue(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
cie.getArgument(7).(BooleanLiteral).getBooleanValue() = true // NewCookie(String name, String value, String path, String domain, String comment, int maxAge, boolean secure, boolean httpOnly)
isBooleanTrue(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 cie.getArgument(9).(BooleanLiteral).getBooleanValue() = true // NewCookie(String name, String value, String path, String domain, int version, String comment, int maxAge, Date expiry, boolean secure, boolean httpOnly)
cie.getNumArgument() = 10 and
isBooleanTrue(cie.getArgument(9)) // NewCookie(String name, String value, String path, String domain, int version, String comment, int maxAge, Date expiry, boolean secure, boolean httpOnly)
)
}

8
java/ql/test/experimental/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.expected
View File

@@ -7,6 +7,9 @@ edges
| SensitiveCookieNotHttpOnly.java:70:28:70:35 | "token=" : String | SensitiveCookieNotHttpOnly.java:71:42:71:50 | secString |
| SensitiveCookieNotHttpOnly.java:70:28:70:43 | ... + ... : String | SensitiveCookieNotHttpOnly.java:71:42:71:50 | secString |
| SensitiveCookieNotHttpOnly.java:70:28:70:55 | ... + ... : String | SensitiveCookieNotHttpOnly.java:71:42:71:50 | secString |
| SensitiveCookieNotHttpOnly.java:88:35:88:51 | "Presto-UI-Token" : String | SensitiveCookieNotHttpOnly.java:91:16:91:21 | cookie : Cookie |
| SensitiveCookieNotHttpOnly.java:91:16:91:21 | cookie : Cookie | SensitiveCookieNotHttpOnly.java:102:25:102:64 | createAuthenticationCookie(...) : Cookie |
| SensitiveCookieNotHttpOnly.java:102:25:102:64 | createAuthenticationCookie(...) : Cookie | SensitiveCookieNotHttpOnly.java:103:28:103:33 | cookie |
nodes
| SensitiveCookieNotHttpOnly.java:24:33:24:43 | "jwt_token" : String | semmle.label | "jwt_token" : String |
| SensitiveCookieNotHttpOnly.java:31:28:31:36 | jwtCookie | semmle.label | jwtCookie |
@@ -21,6 +24,10 @@ nodes
| 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:91:16:91:21 | cookie : Cookie | semmle.label | cookie : Cookie |
| SensitiveCookieNotHttpOnly.java:102:25:102:64 | createAuthenticationCookie(...) : Cookie | semmle.label | createAuthenticationCookie(...) : Cookie |
| SensitiveCookieNotHttpOnly.java:103:28:103:33 | cookie | semmle.label | cookie |
#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" | 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 |
@@ -31,3 +38,4 @@ nodes
| 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:103:28:103:33 | cookie | SensitiveCookieNotHttpOnly.java:88:35:88:51 | "Presto-UI-Token" : String | SensitiveCookieNotHttpOnly.java:103:28:103:33 | cookie | $@ doesn't have the HttpOnly flag set. | SensitiveCookieNotHttpOnly.java:88:35:88:51 | "Presto-UI-Token" | This sensitive cookie |

57
java/ql/test/experimental/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.java
View File

@@ -71,6 +71,63 @@ class SensitiveCookieNotHttpOnly {
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;
}
// 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);
}
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 addCookie13(HttpServletRequest request, HttpServletResponse response, String refreshToken) {
response.addCookie(createCookie("refresh_token", refreshToken, true));
}
// BAD - Tests set a sensitive cookie header with the `HttpOnly` flag not set through a boolean variable using a wrapper method.
public void addCookie14(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"