Skip to content

Commit 919c6b4

Browse files
luchua-bcluchua-bc
committed
Optimize flow steps
1 parent 502cf38 commit 919c6b4

File tree

3 files changed

+37
-22
lines changed

3 files changed

+37
-22
lines changed

‎java/ql/src/experimental/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
*/
99

1010
import java
11+
import semmle.code.java.dataflow.FlowSteps
1112
import semmle.code.java.frameworks.Servlets
1213
import semmle.code.java.dataflow.TaintTracking
1314
import DataFlow::PathGraph
@@ -41,18 +42,31 @@ class SetCookieMethodAccess extends MethodAccess {
4142
}
4243
}
4344

45+
/** The cookie class of Java EE. */
46+
class CookieClass extends RefType {
47+
CookieClass() {
48+
this.getASupertype*()
49+
.hasQualifiedName(["javax.servlet.http", "javax.ws.rs.core", "jakarta.ws.rs.core"], "Cookie")
50+
}
51+
}
52+
53+
/** The method call `toString` to get a stringified cookie representation. */
54+
class CookieInstanceExpr extends TaintPreservingCallable {
55+
CookieInstanceExpr() {
56+
this.getDeclaringType() instanceof CookieClass and
57+
this.hasName("toString")
58+
}
59+
60+
override predicate returnsTaintFrom(int arg) { arg = -1 }
61+
}
62+
4463
/** Sensitive cookie name used in a `Cookie` constructor or a `Set-Cookie` call. */
4564
class SensitiveCookieNameExpr extends Expr {
4665
SensitiveCookieNameExpr() {
4766
exists(
4867
ClassInstanceExpr cie // new Cookie("jwt_token", token)
4968
|
50-
(
51-
cie.getConstructedType().hasQualifiedName("javax.servlet.http", "Cookie") or
52-
cie.getConstructedType()
53-
.getASupertype*()
54-
.hasQualifiedName(["javax.ws.rs.core", "jakarta.ws.rs.core"], "Cookie")
55-
) and
69+
cie.getConstructedType() instanceof CookieClass and
5670
this = cie and
5771
isSensitiveCookieNameExpr(cie.getArgument(0))
5872
)
@@ -169,16 +183,6 @@ class MissingHttpOnlyConfiguration extends TaintTracking::Configuration {
169183
// Test class or method
170184
isTestMethod(node)
171185
}
172-
173-
override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
174-
exists(
175-
MethodAccess ma // `toString` call on a cookie object
176-
|
177-
ma.getQualifier() = pred.asExpr() and
178-
ma.getMethod().hasName("toString") and
179-
ma = succ.asExpr()
180-
)
181-
}
182186
}
183187

184188
from DataFlow::PathNode source, DataFlow::PathNode sink, MissingHttpOnlyConfiguration c

‎java/ql/test/experimental/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.expected

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,17 @@
11
edges
2-
| SensitiveCookieNotHttpOnly.java:22:27:22:60 | new Cookie(...) : Cookie | SensitiveCookieNotHttpOnly.java:28:28:28:36 | jwtCookie |
2+
| SensitiveCookieNotHttpOnly.java:22:28:22:61 | new Cookie(...) : Cookie | SensitiveCookieNotHttpOnly.java:28:28:28:36 | jwtCookie |
33
| SensitiveCookieNotHttpOnly.java:49:42:49:113 | new NewCookie(...) : NewCookie | SensitiveCookieNotHttpOnly.java:49:42:49:124 | toString(...) |
4+
| SensitiveCookieNotHttpOnly.java:60:37:60:115 | new NewCookie(...) : NewCookie | SensitiveCookieNotHttpOnly.java:62:42:62:47 | keyStr |
45
nodes
5-
| SensitiveCookieNotHttpOnly.java:22:27:22:60 | new Cookie(...) : Cookie | semmle.label | new Cookie(...) : Cookie |
6+
| SensitiveCookieNotHttpOnly.java:22:28:22:61 | new Cookie(...) : Cookie | semmle.label | new Cookie(...) : Cookie |
67
| SensitiveCookieNotHttpOnly.java:28:28:28:36 | jwtCookie | semmle.label | jwtCookie |
78
| SensitiveCookieNotHttpOnly.java:39:42:39:69 | ... + ... | semmle.label | ... + ... |
89
| SensitiveCookieNotHttpOnly.java:49:42:49:113 | new NewCookie(...) : NewCookie | semmle.label | new NewCookie(...) : NewCookie |
910
| SensitiveCookieNotHttpOnly.java:49:42:49:124 | toString(...) | semmle.label | toString(...) |
11+
| SensitiveCookieNotHttpOnly.java:60:37:60:115 | new NewCookie(...) : NewCookie | semmle.label | new NewCookie(...) : NewCookie |
12+
| SensitiveCookieNotHttpOnly.java:62:42:62:47 | keyStr | semmle.label | keyStr |
1013
#select
11-
| SensitiveCookieNotHttpOnly.java:28:28:28:36 | jwtCookie | SensitiveCookieNotHttpOnly.java:22:27:22:60 | new Cookie(...) : Cookie | SensitiveCookieNotHttpOnly.java:28:28:28:36 | jwtCookie | $@ doesn't have the HttpOnly flag set. | SensitiveCookieNotHttpOnly.java:22:27:22:60 | new Cookie(...) | This sensitive cookie |
14+
| SensitiveCookieNotHttpOnly.java:28:28:28:36 | jwtCookie | SensitiveCookieNotHttpOnly.java:22:28:22:61 | new Cookie(...) : Cookie | SensitiveCookieNotHttpOnly.java:28:28:28:36 | jwtCookie | $@ doesn't have the HttpOnly flag set. | SensitiveCookieNotHttpOnly.java:22:28:22:61 | new Cookie(...) | This sensitive cookie |
1215
| SensitiveCookieNotHttpOnly.java:39:42:39:69 | ... + ... | SensitiveCookieNotHttpOnly.java:39:42:39:69 | ... + ... | SensitiveCookieNotHttpOnly.java:39:42:39:69 | ... + ... | $@ doesn't have the HttpOnly flag set. | SensitiveCookieNotHttpOnly.java:39:42:39:69 | ... + ... | This sensitive cookie |
1316
| SensitiveCookieNotHttpOnly.java:49:42:49:124 | toString(...) | SensitiveCookieNotHttpOnly.java:49:42:49:113 | new NewCookie(...) : NewCookie | SensitiveCookieNotHttpOnly.java:49:42:49:124 | toString(...) | $@ doesn't have the HttpOnly flag set. | SensitiveCookieNotHttpOnly.java:49:42:49:113 | new NewCookie(...) | This sensitive cookie |
17+
| SensitiveCookieNotHttpOnly.java:62:42:62:47 | keyStr | SensitiveCookieNotHttpOnly.java:60:37:60:115 | new NewCookie(...) : NewCookie | SensitiveCookieNotHttpOnly.java:62:42:62:47 | keyStr | $@ doesn't have the HttpOnly flag set. | SensitiveCookieNotHttpOnly.java:60:37:60:115 | new NewCookie(...) | This sensitive cookie |

‎java/ql/test/experimental/query-tests/security/CWE-1004/SensitiveCookieNotHttpOnly.java

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
class SensitiveCookieNotHttpOnly {
1111
// GOOD - Tests adding a sensitive cookie with the `HttpOnly` flag set.
1212
public void addCookie(String jwt_token, HttpServletRequest request, HttpServletResponse response) {
13-
Cookie jwtCookie =new Cookie("jwt_token", jwt_token);
13+
Cookie jwtCookie = new Cookie("jwt_token", jwt_token);
1414
jwtCookie.setPath("/");
1515
jwtCookie.setMaxAge(3600*24*7);
1616
jwtCookie.setHttpOnly(true);
@@ -19,8 +19,8 @@ public void addCookie(String jwt_token, HttpServletRequest request, HttpServletR
1919

2020
// BAD - Tests adding a sensitive cookie without the `HttpOnly` flag set.
2121
public void addCookie2(String jwt_token, String userId, HttpServletRequest request, HttpServletResponse response) {
22-
Cookie jwtCookie =new Cookie("jwt_token", jwt_token);
23-
Cookie userIdCookie =new Cookie("user_id", userId.toString());
22+
Cookie jwtCookie = new Cookie("jwt_token", jwt_token);
23+
Cookie userIdCookie = new Cookie("user_id", userId);
2424
jwtCookie.setPath("/");
2525
userIdCookie.setPath("/");
2626
jwtCookie.setMaxAge(3600*24*7);
@@ -54,4 +54,11 @@ public void addCookie7(String accessKey, HttpServletRequest request, HttpServlet
5454
NewCookie accessKeyCookie = new NewCookie("session-access-key", accessKey, "/", null, null, 0, true, true);
5555
response.setHeader("Set-Cookie", accessKeyCookie.toString());
5656
}
57+
58+
// BAD - Tests set a sensitive cookie header using the class `javax.ws.rs.core.Cookie` without the `HttpOnly` flag set.
59+
public void addCookie8(String accessKey, HttpServletRequest request, HttpServletResponse response) {
60+
NewCookie accessKeyCookie = new NewCookie("session-access-key", accessKey, "/", null, 0, null, 86400, true);
61+
String keyStr = accessKeyCookie.toString();
62+
response.setHeader("Set-Cookie", keyStr);
63+
}
5764
}

0 commit comments

Comments
 (0)