Skip to content

Commit c799f93

Browse files
Update tests and add inline expectations
1 parent e1cf3d3 commit c799f93

File tree

5 files changed

+229
-1
lines changed

5 files changed

+229
-1
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ import java
2323
import semmle.code.java.dataflow.FlowSteps
2424
import semmle.code.java.frameworks.Servlets
2525
import semmle.code.java.dataflow.TaintTracking
26-
import MissingHttpOnlyFlow::PathGraph
2726

2827
/** Gets a regular expression for matching common names of sensitive cookies. */
2928
string getSensitiveCookieNameRegex() { result = "(?i).*(auth|session|token|key|credential).*" }
@@ -196,6 +195,8 @@ module MissingHttpOnlyConfig implements DataFlow::ConfigSig {
196195

197196
module MissingHttpOnlyFlow = TaintTracking::Global<MissingHttpOnlyConfig>;
198197

198+
import MissingHttpOnlyFlow::PathGraph
199+
199200
from MissingHttpOnlyFlow::PathNode source, MissingHttpOnlyFlow::PathNode sink
200201
where MissingHttpOnlyFlow::flowPath(source, sink)
201202
select sink, source, sink, "$@ doesn't have the HttpOnly flag set.", source, "This sensitive cookie"
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
#select
2+
| 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 |
3+
| 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 |
4+
| 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 |
5+
| 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 |
6+
| 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 |
7+
| 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 |
8+
| 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 |
9+
| 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 |
10+
| 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 |
11+
| 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 |
12+
edges
13+
| SensitiveCookieNotHttpOnly.java:24:33:24:43 | "jwt_token" : String | SensitiveCookieNotHttpOnly.java:25:39:25:52 | tokenCookieStr : String | provenance | |
14+
| SensitiveCookieNotHttpOnly.java:25:28:25:64 | new Cookie(...) : Cookie | SensitiveCookieNotHttpOnly.java:31:28:31:36 | jwtCookie | provenance | Sink:MaD:46211 |
15+
| SensitiveCookieNotHttpOnly.java:25:39:25:52 | tokenCookieStr : String | SensitiveCookieNotHttpOnly.java:25:28:25:64 | new Cookie(...) : Cookie | provenance | Config |
16+
| SensitiveCookieNotHttpOnly.java:25:39:25:52 | tokenCookieStr : String | SensitiveCookieNotHttpOnly.java:25:28:25:64 | new Cookie(...) : Cookie | provenance | MaD:46217 |
17+
| SensitiveCookieNotHttpOnly.java:42:42:42:49 | "token=" : String | SensitiveCookieNotHttpOnly.java:42:42:42:69 | ... + ... | provenance | Sink:MaD:46212 |
18+
| SensitiveCookieNotHttpOnly.java:42:42:42:57 | ... + ... : String | SensitiveCookieNotHttpOnly.java:42:42:42:69 | ... + ... | provenance | Sink:MaD:46212 |
19+
| SensitiveCookieNotHttpOnly.java:52:42:52:113 | new NewCookie(...) : NewCookie | SensitiveCookieNotHttpOnly.java:52:42:52:124 | toString(...) | provenance | MaD:46260 Sink:MaD:46214 |
20+
| SensitiveCookieNotHttpOnly.java:52:56:52:75 | "session-access-key" : String | SensitiveCookieNotHttpOnly.java:52:42:52:113 | new NewCookie(...) : NewCookie | provenance | MaD:46298 |
21+
| SensitiveCookieNotHttpOnly.java:63:37:63:115 | new NewCookie(...) : NewCookie | SensitiveCookieNotHttpOnly.java:64:25:64:39 | accessKeyCookie : NewCookie | provenance | |
22+
| SensitiveCookieNotHttpOnly.java:63:51:63:70 | "session-access-key" : String | SensitiveCookieNotHttpOnly.java:63:37:63:115 | new NewCookie(...) : NewCookie | provenance | MaD:46298 |
23+
| SensitiveCookieNotHttpOnly.java:64:25:64:39 | accessKeyCookie : NewCookie | SensitiveCookieNotHttpOnly.java:64:25:64:50 | toString(...) : String | provenance | MaD:46260 |
24+
| SensitiveCookieNotHttpOnly.java:64:25:64:50 | toString(...) : String | SensitiveCookieNotHttpOnly.java:65:42:65:47 | keyStr | provenance | Sink:MaD:46214 |
25+
| SensitiveCookieNotHttpOnly.java:70:28:70:35 | "token=" : String | SensitiveCookieNotHttpOnly.java:71:42:71:50 | secString | provenance | Sink:MaD:46212 |
26+
| SensitiveCookieNotHttpOnly.java:70:28:70:43 | ... + ... : String | SensitiveCookieNotHttpOnly.java:71:42:71:50 | secString | provenance | Sink:MaD:46212 |
27+
| SensitiveCookieNotHttpOnly.java:70:28:70:55 | ... + ... : String | SensitiveCookieNotHttpOnly.java:71:42:71:50 | secString | provenance | Sink:MaD:46212 |
28+
| SensitiveCookieNotHttpOnly.java:88:35:88:51 | "Presto-UI-Token" : String | SensitiveCookieNotHttpOnly.java:89:36:89:51 | PRESTO_UI_COOKIE : String | provenance | |
29+
| SensitiveCookieNotHttpOnly.java:89:25:89:57 | new Cookie(...) : Cookie | SensitiveCookieNotHttpOnly.java:91:16:91:21 | cookie : Cookie | provenance | |
30+
| SensitiveCookieNotHttpOnly.java:89:36:89:51 | PRESTO_UI_COOKIE : String | SensitiveCookieNotHttpOnly.java:89:25:89:57 | new Cookie(...) : Cookie | provenance | Config |
31+
| SensitiveCookieNotHttpOnly.java:89:36:89:51 | PRESTO_UI_COOKIE : String | SensitiveCookieNotHttpOnly.java:89:25:89:57 | new Cookie(...) : Cookie | provenance | MaD:46217 |
32+
| SensitiveCookieNotHttpOnly.java:91:16:91:21 | cookie : Cookie | SensitiveCookieNotHttpOnly.java:110:25:110:64 | createAuthenticationCookie(...) : Cookie | provenance | |
33+
| SensitiveCookieNotHttpOnly.java:110:25:110:64 | createAuthenticationCookie(...) : Cookie | SensitiveCookieNotHttpOnly.java:111:28:111:33 | cookie | provenance | Sink:MaD:46211 |
34+
nodes
35+
| SensitiveCookieNotHttpOnly.java:24:33:24:43 | "jwt_token" : String | semmle.label | "jwt_token" : String |
36+
| SensitiveCookieNotHttpOnly.java:25:28:25:64 | new Cookie(...) : Cookie | semmle.label | new Cookie(...) : Cookie |
37+
| SensitiveCookieNotHttpOnly.java:25:39:25:52 | tokenCookieStr : String | semmle.label | tokenCookieStr : String |
38+
| SensitiveCookieNotHttpOnly.java:31:28:31:36 | jwtCookie | semmle.label | jwtCookie |
39+
| SensitiveCookieNotHttpOnly.java:42:42:42:49 | "token=" : String | semmle.label | "token=" : String |
40+
| SensitiveCookieNotHttpOnly.java:42:42:42:57 | ... + ... : String | semmle.label | ... + ... : String |
41+
| SensitiveCookieNotHttpOnly.java:42:42:42:69 | ... + ... | semmle.label | ... + ... |
42+
| SensitiveCookieNotHttpOnly.java:52:42:52:113 | new NewCookie(...) : NewCookie | semmle.label | new NewCookie(...) : NewCookie |
43+
| SensitiveCookieNotHttpOnly.java:52:42:52:124 | toString(...) | semmle.label | toString(...) |
44+
| SensitiveCookieNotHttpOnly.java:52:56:52:75 | "session-access-key" : String | semmle.label | "session-access-key" : String |
45+
| SensitiveCookieNotHttpOnly.java:63:37:63:115 | new NewCookie(...) : NewCookie | semmle.label | new NewCookie(...) : NewCookie |
46+
| SensitiveCookieNotHttpOnly.java:63:51:63:70 | "session-access-key" : String | semmle.label | "session-access-key" : String |
47+
| SensitiveCookieNotHttpOnly.java:64:25:64:39 | accessKeyCookie : NewCookie | semmle.label | accessKeyCookie : NewCookie |
48+
| SensitiveCookieNotHttpOnly.java:64:25:64:50 | toString(...) : String | semmle.label | toString(...) : String |
49+
| SensitiveCookieNotHttpOnly.java:65:42:65:47 | keyStr | semmle.label | keyStr |
50+
| SensitiveCookieNotHttpOnly.java:70:28:70:35 | "token=" : String | semmle.label | "token=" : String |
51+
| SensitiveCookieNotHttpOnly.java:70:28:70:43 | ... + ... : String | semmle.label | ... + ... : String |
52+
| SensitiveCookieNotHttpOnly.java:70:28:70:55 | ... + ... : String | semmle.label | ... + ... : String |
53+
| SensitiveCookieNotHttpOnly.java:71:42:71:50 | secString | semmle.label | secString |
54+
| SensitiveCookieNotHttpOnly.java:88:35:88:51 | "Presto-UI-Token" : String | semmle.label | "Presto-UI-Token" : String |
55+
| SensitiveCookieNotHttpOnly.java:89:25:89:57 | new Cookie(...) : Cookie | semmle.label | new Cookie(...) : Cookie |
56+
| SensitiveCookieNotHttpOnly.java:89:36:89:51 | PRESTO_UI_COOKIE : String | semmle.label | PRESTO_UI_COOKIE : String |
57+
| SensitiveCookieNotHttpOnly.java:91:16:91:21 | cookie : Cookie | semmle.label | cookie : Cookie |
58+
| SensitiveCookieNotHttpOnly.java:110:25:110:64 | createAuthenticationCookie(...) : Cookie | semmle.label | createAuthenticationCookie(...) : Cookie |
59+
| SensitiveCookieNotHttpOnly.java:111:28:111:33 | cookie | semmle.label | cookie |
60+
subpaths
Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,164 @@
1+
import java.io.IOException;
2+
3+
import javax.servlet.http.Cookie;
4+
import javax.servlet.http.HttpServletRequest;
5+
import javax.servlet.http.HttpServletResponse;
6+
import javax.servlet.ServletException;
7+
8+
import javax.ws.rs.core.NewCookie;
9+
10+
import org.springframework.security.web.csrf.CsrfToken;
11+
12+
class SensitiveCookieNotHttpOnly {
13+
// GOOD - Tests adding a sensitive cookie with the `HttpOnly` flag set.
14+
public void addCookie(String jwt_token, HttpServletRequest request, HttpServletResponse response) {
15+
Cookie jwtCookie = new Cookie("jwt_token", jwt_token);
16+
jwtCookie.setPath("/");
17+
jwtCookie.setMaxAge(3600*24*7);
18+
jwtCookie.setHttpOnly(true);
19+
response.addCookie(jwtCookie);
20+
}
21+
22+
// BAD - Tests adding a sensitive cookie without the `HttpOnly` flag set.
23+
public void addCookie2(String jwt_token, String userId, HttpServletRequest request, HttpServletResponse response) {
24+
String tokenCookieStr = "jwt_token"; // $Source
25+
Cookie jwtCookie = new Cookie(tokenCookieStr, jwt_token);
26+
Cookie userIdCookie = new Cookie("user_id", userId);
27+
jwtCookie.setPath("/");
28+
userIdCookie.setPath("/");
29+
jwtCookie.setMaxAge(3600*24*7);
30+
userIdCookie.setMaxAge(3600*24*7);
31+
response.addCookie(jwtCookie); // $Alert
32+
response.addCookie(userIdCookie);
33+
}
34+
35+
// GOOD - Tests set a sensitive cookie header with the `HttpOnly` flag set.
36+
public void addCookie3(String authId, HttpServletRequest request, HttpServletResponse response) {
37+
response.addHeader("Set-Cookie", "token=" +authId + ";HttpOnly;Secure");
38+
}
39+
40+
// BAD - Tests set a sensitive cookie header without the `HttpOnly` flag set.
41+
public void addCookie4(String authId, HttpServletRequest request, HttpServletResponse response) {
42+
response.addHeader("Set-Cookie", "token=" +authId + ";Secure"); // $Alert
43+
}
44+
45+
// GOOD - Tests set a sensitive cookie header using the class `javax.ws.rs.core.Cookie` with the `HttpOnly` flag set through string concatenation.
46+
public void addCookie5(String accessKey, HttpServletRequest request, HttpServletResponse response) {
47+
response.setHeader("Set-Cookie", new NewCookie("session-access-key", accessKey, "/", null, null, 0, true) + ";HttpOnly");
48+
}
49+
50+
// BAD - Tests set a sensitive cookie header using the class `javax.ws.rs.core.Cookie` without the `HttpOnly` flag set.
51+
public void addCookie6(String accessKey, HttpServletRequest request, HttpServletResponse response) {
52+
response.setHeader("Set-Cookie", new NewCookie("session-access-key", accessKey, "/", null, null, 0, true).toString()); // $Alert
53+
}
54+
55+
// GOOD - Tests set a sensitive cookie header using the class `javax.ws.rs.core.Cookie` with the `HttpOnly` flag set through the constructor.
56+
public void addCookie7(String accessKey, HttpServletRequest request, HttpServletResponse response) {
57+
NewCookie accessKeyCookie = new NewCookie("session-access-key", accessKey, "/", null, null, 0, true, true);
58+
response.setHeader("Set-Cookie", accessKeyCookie.toString());
59+
}
60+
61+
// BAD - Tests set a sensitive cookie header using the class `javax.ws.rs.core.Cookie` without the `HttpOnly` flag set.
62+
public void addCookie8(String accessKey, HttpServletRequest request, HttpServletResponse response) {
63+
NewCookie accessKeyCookie = new NewCookie("session-access-key", accessKey, "/", null, 0, null, 86400, true); // $Source
64+
String keyStr = accessKeyCookie.toString();
65+
response.setHeader("Set-Cookie", keyStr); // $Alert
66+
}
67+
68+
// BAD - Tests set a sensitive cookie header using a variable without the `HttpOnly` flag set.
69+
public void addCookie9(String authId, HttpServletRequest request, HttpServletResponse response) {
70+
String secString = "token=" +authId + ";Secure"; // $Source
71+
response.addHeader("Set-Cookie", secString); // $Alert
72+
}
73+
74+
// GOOD - Tests set a sensitive cookie header with the `HttpOnly` flag set using `String.format(...)`.
75+
public void addCookie10(HttpServletRequest request, HttpServletResponse response) {
76+
response.addHeader("SET-COOKIE", String.format("%s=%s;HttpOnly", "sessionkey", request.getSession().getAttribute("sessionkey")));
77+
}
78+
79+
public Cookie createHttpOnlyAuthenticationCookie(HttpServletRequest request, String jwt) {
80+
String PRESTO_UI_COOKIE = "Presto-UI-Token";
81+
Cookie cookie = new Cookie(PRESTO_UI_COOKIE, jwt);
82+
cookie.setHttpOnly(true);
83+
cookie.setPath("/ui");
84+
return cookie;
85+
}
86+
87+
public Cookie createAuthenticationCookie(HttpServletRequest request, String jwt) {
88+
String PRESTO_UI_COOKIE = "Presto-UI-Token"; // $Source
89+
Cookie cookie = new Cookie(PRESTO_UI_COOKIE, jwt);
90+
cookie.setPath("/ui");
91+
return cookie;
92+
}
93+
94+
public Cookie removeAuthenticationCookie(HttpServletRequest request, String jwt) {
95+
String PRESTO_UI_COOKIE = "Presto-UI-Token";
96+
Cookie cookie = new Cookie(PRESTO_UI_COOKIE, jwt);
97+
cookie.setPath("/ui");
98+
cookie.setMaxAge(0);
99+
return cookie;
100+
}
101+
102+
// GOOD - Tests set a sensitive cookie header with the `HttpOnly` flag set using a wrapper method.
103+
public void addCookie11(HttpServletRequest request, HttpServletResponse response, String jwt) {
104+
Cookie cookie = createHttpOnlyAuthenticationCookie(request, jwt);
105+
response.addCookie(cookie);
106+
}
107+
108+
// BAD - Tests set a sensitive cookie header without the `HttpOnly` flag set using a wrapper method.
109+
public void addCookie12(HttpServletRequest request, HttpServletResponse response, String jwt) {
110+
Cookie cookie = createAuthenticationCookie(request, jwt);
111+
response.addCookie(cookie); // $Alert
112+
}
113+
114+
// GOOD - Tests remove a sensitive cookie header without the `HttpOnly` flag set using a wrapper method.
115+
public void addCookie13(HttpServletRequest request, HttpServletResponse response, String jwt) {
116+
Cookie cookie = removeAuthenticationCookie(request, jwt);
117+
response.addCookie(cookie);
118+
}
119+
120+
private Cookie createCookie(String name, String value, Boolean httpOnly){
121+
Cookie cookie = null;
122+
cookie = new Cookie(name, value);
123+
cookie.setDomain("/");
124+
cookie.setHttpOnly(httpOnly);
125+
126+
//for production https
127+
cookie.setSecure(true);
128+
129+
cookie.setMaxAge(60*60*24*30);
130+
cookie.setPath("/");
131+
132+
return cookie;
133+
}
134+
135+
// GOOD - Tests set a sensitive cookie header with the `HttpOnly` flag set through a boolean variable using a wrapper method.
136+
public void addCookie14(HttpServletRequest request, HttpServletResponse response, String refreshToken) {
137+
response.addCookie(createCookie("refresh_token", refreshToken, true));
138+
}
139+
140+
// BAD (but not detected) - Tests set a sensitive cookie header with the `HttpOnly` flag not set through a boolean variable using a wrapper method.
141+
// This example is missed because the `cookie.setHttpOnly` call in `createCookie` is thought to maybe set the HTTP-only flag, and the `cookie`
142+
// object flows to this `addCookie` call.
143+
public void addCookie15(HttpServletRequest request, HttpServletResponse response, String refreshToken) {
144+
response.addCookie(createCookie("refresh_token", refreshToken, false)); // $MISSING:Alert
145+
}
146+
147+
// GOOD - CSRF token doesn't need to have the `HttpOnly` flag set.
148+
public void addCsrfCookie(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
149+
// Spring put the CSRF token in session attribute "_csrf"
150+
CsrfToken csrfToken = (CsrfToken) request.getAttribute("_csrf");
151+
152+
// Send the cookie only if the token has changed
153+
String actualToken = request.getHeader("X-CSRF-TOKEN");
154+
if (actualToken == null || !actualToken.equals(csrfToken.getToken())) {
155+
// Session cookie that can be used by AngularJS
156+
String pCookieName = "CSRF-TOKEN";
157+
Cookie cookie = new Cookie(pCookieName, csrfToken.getToken());
158+
cookie.setMaxAge(-1);
159+
cookie.setHttpOnly(false);
160+
cookie.setPath("/");
161+
response.addCookie(cookie);
162+
}
163+
}
164+
}
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
query: Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql
2+
postprocess: utils/test/InlineExpectationsTestQuery.ql
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
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

0 commit comments

Comments
 (0)