Skip to content

Commit 367ff99

Browse files
luchua-bcluchua-bc
committed
Change the source to be the request variable
1 parent 195755d commit 367ff99

4 files changed

Lines changed: 89 additions & 35 deletions

File tree

‎java/ql/src/experimental/Security/CWE/CWE-598/SensitiveGetQuery.ql‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
import java
1111
import semmle.code.java.dataflow.FlowSources
1212
import semmle.code.java.dataflow.TaintTracking
13-
import semmle.code.java.frameworks.Servlets
1413
import semmle.code.java.security.SensitiveActions
1514
import DataFlow::PathGraph
1615

@@ -23,15 +22,15 @@ class SensitiveInfoExpr extends Expr {
2322
}
2423
}
2524

26-
/** Holds if `ma` is a method access to some override of `HttpServlet.doGet`. */
27-
private predicate isGetServletMethod(MethodAccess ma) { isServletMethod(ma, "doGet") }
25+
/** Holds if `m` is a method of some override of `HttpServlet.doGet`. */
26+
private predicate isGetServletMethod(Method m) { isServletMethod(m) and m.getName() = "doGet" }
2827

2928
/** Source of GET servlet requests. */
30-
class GetServletMethodSource extends DataFlow::ExprNode {
31-
GetServletMethodSource() {
32-
exists(MethodAccess ma |
33-
isGetServletMethod(ma) and
34-
ma = this.getExpr()
29+
class GetHttpRequestSource extends DataFlow::ExprNode {
30+
GetHttpRequestSource() {
31+
exists(Method m |
32+
isGetServletMethod(m) and
33+
m.getParameter(0).getAnAccess() = this.asExpr()
3534
)
3635
}
3736
}
@@ -40,9 +39,15 @@ class GetServletMethodSource extends DataFlow::ExprNode {
4039
class SensitiveGetQueryConfiguration extends TaintTracking::Configuration {
4140
SensitiveGetQueryConfiguration() { this = "SensitiveGetQueryConfiguration" }
4241

43-
override predicate isSource(DataFlow::Node source) { source instanceof GetServletMethodSource }
42+
override predicate isSource(DataFlow::Node source) { source instanceof GetHttpRequestSource }
4443

4544
override predicate isSink(DataFlow::Node sink) { sink.asExpr() instanceof SensitiveExpr }
45+
46+
override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
47+
exists(MethodAccess ma |
48+
isRequestGetParamMethod(ma) and pred.asExpr() = ma.getQualifier() and succ.asExpr() = ma
49+
)
50+
}
4651
}
4752

4853
from DataFlow::PathNode source, DataFlow::PathNode sink, SensitiveGetQueryConfiguration c

‎java/ql/src/semmle/code/java/frameworks/Servlets.qll‎

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -323,20 +323,17 @@ class ServletWebXMLListenerType extends RefType {
323323
}
324324
}
325325

326-
/** Holds if `ma` is a method access to some override of methods of `HttpServlet`, for example `doGet` or `doPost`. */
327-
predicate isServletMethod(MethodAccess ma, string methodName) {
328-
exists(Method m |
329-
m = ma.getEnclosingCallable() and
330-
m.getDeclaringType() instanceof ServletClass and
331-
m.getNumberOfParameters() = 2 and
332-
m.getParameter(0).getType() instanceof ServletRequest and
333-
m.getParameter(1).getType() instanceof ServletResponse and
334-
m.getName() = methodName and
335-
ma.getQualifier() = m.getParameter(0).getAnAccess() and
336-
(
337-
ma.getMethod() instanceof ServletRequestGetParameterMethod or
338-
ma.getMethod() instanceof ServletRequestGetParameterMapMethod or
339-
ma.getMethod() instanceof HttpServletRequestGetQueryStringMethod
340-
)
341-
)
326+
/** Holds if `m` is a request handler method (for example `doGet` or `doPost`). */
327+
predicate isServletMethod(Method m) {
328+
m.getDeclaringType() instanceof ServletClass and
329+
m.getNumberOfParameters() = 2 and
330+
m.getParameter(0).getType() instanceof ServletRequest and
331+
m.getParameter(1).getType() instanceof ServletResponse
332+
}
333+
334+
/** Holds if `ma` is a call that gets a request parameter. */
335+
predicate isRequestGetParamMethod(MethodAccess ma) {
336+
ma.getMethod() instanceof ServletRequestGetParameterMethod or
337+
ma.getMethod() instanceof ServletRequestGetParameterMapMethod or
338+
ma.getMethod() instanceof HttpServletRequestGetQueryStringMethod
342339
}

‎java/ql/test/experimental/query-tests/security/CWE-598/SensitiveGetQuery.expected‎

Lines changed: 36 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,35 +1,61 @@
11
edges
2-
| SensitiveGetQuery2.java:12:13:12:37 | getParameterMap(...) : Map | SensitiveGetQuery2.java:14:21:14:48 | (...)... : Object |
3-
| SensitiveGetQuery2.java:12:13:12:37 | getParameterMap(...) : Map | SensitiveGetQuery2.java:14:30:14:48 | get(...) |
4-
| SensitiveGetQuery2.java:12:13:12:37 | getParameterMap(...) : Map | SensitiveGetQuery2.java:14:30:14:48 | get(...) : Object |
2+
| SensitiveGetQuery2.java:12:13:12:19 | request : HttpServletRequest | SensitiveGetQuery2.java:14:21:14:48 | (...)... : Object |
3+
| SensitiveGetQuery2.java:12:13:12:19 | request : HttpServletRequest | SensitiveGetQuery2.java:14:30:14:48 | get(...) |
4+
| SensitiveGetQuery2.java:12:13:12:19 | request : HttpServletRequest | SensitiveGetQuery2.java:14:30:14:48 | get(...) : Object |
55
| SensitiveGetQuery2.java:14:21:14:48 | (...)... : Object | SensitiveGetQuery2.java:15:29:15:36 | password |
66
| SensitiveGetQuery2.java:14:21:14:48 | (...)... : Object | SensitiveGetQuery2.java:15:29:15:36 | password : Object |
77
| SensitiveGetQuery2.java:14:30:14:48 | get(...) : Object | SensitiveGetQuery2.java:14:21:14:48 | (...)... : Object |
88
| SensitiveGetQuery2.java:15:29:15:36 | password : Object | SensitiveGetQuery2.java:18:40:18:54 | password : Object |
99
| SensitiveGetQuery2.java:18:40:18:54 | password : Object | SensitiveGetQuery2.java:19:61:19:68 | password |
10+
| SensitiveGetQuery3.java:11:41:11:47 | request : HttpServletRequest | SensitiveGetQuery3.java:12:41:12:47 | request : HttpServletRequest |
11+
| SensitiveGetQuery3.java:12:21:12:60 | getRequestParameter(...) : String | SensitiveGetQuery3.java:13:57:13:64 | password |
12+
| SensitiveGetQuery3.java:12:41:12:47 | request : HttpServletRequest | SensitiveGetQuery3.java:12:21:12:60 | getRequestParameter(...) |
13+
| SensitiveGetQuery3.java:12:41:12:47 | request : HttpServletRequest | SensitiveGetQuery3.java:12:21:12:60 | getRequestParameter(...) : String |
14+
| SensitiveGetQuery.java:11:21:11:27 | request : HttpServletRequest | SensitiveGetQuery.java:12:21:12:52 | getParameter(...) |
15+
| SensitiveGetQuery.java:11:21:11:27 | request : HttpServletRequest | SensitiveGetQuery.java:12:21:12:52 | getParameter(...) : String |
16+
| SensitiveGetQuery.java:11:21:11:27 | request : HttpServletRequest | SensitiveGetQuery.java:14:29:14:36 | password |
17+
| SensitiveGetQuery.java:11:21:11:27 | request : HttpServletRequest | SensitiveGetQuery.java:14:29:14:36 | password : String |
18+
| SensitiveGetQuery.java:12:21:12:27 | request : HttpServletRequest | SensitiveGetQuery.java:12:21:12:52 | getParameter(...) |
19+
| SensitiveGetQuery.java:12:21:12:27 | request : HttpServletRequest | SensitiveGetQuery.java:12:21:12:52 | getParameter(...) : String |
20+
| SensitiveGetQuery.java:12:21:12:27 | request : HttpServletRequest | SensitiveGetQuery.java:14:29:14:36 | password |
21+
| SensitiveGetQuery.java:12:21:12:27 | request : HttpServletRequest | SensitiveGetQuery.java:14:29:14:36 | password : String |
1022
| SensitiveGetQuery.java:12:21:12:52 | getParameter(...) : String | SensitiveGetQuery.java:14:29:14:36 | password |
1123
| SensitiveGetQuery.java:12:21:12:52 | getParameter(...) : String | SensitiveGetQuery.java:14:29:14:36 | password : String |
1224
| SensitiveGetQuery.java:14:29:14:36 | password : String | SensitiveGetQuery.java:17:40:17:54 | password : String |
1325
| SensitiveGetQuery.java:17:40:17:54 | password : String | SensitiveGetQuery.java:18:61:18:68 | password |
1426
nodes
15-
| SensitiveGetQuery2.java:12:13:12:37 | getParameterMap(...) : Map | semmle.label | getParameterMap(...) : Map |
27+
| SensitiveGetQuery2.java:12:13:12:19 | request : HttpServletRequest | semmle.label | request : HttpServletRequest |
1628
| SensitiveGetQuery2.java:14:21:14:48 | (...)... : Object | semmle.label | (...)... : Object |
1729
| SensitiveGetQuery2.java:14:30:14:48 | get(...) | semmle.label | get(...) |
1830
| SensitiveGetQuery2.java:14:30:14:48 | get(...) : Object | semmle.label | get(...) : Object |
1931
| SensitiveGetQuery2.java:15:29:15:36 | password | semmle.label | password |
2032
| SensitiveGetQuery2.java:15:29:15:36 | password : Object | semmle.label | password : Object |
2133
| SensitiveGetQuery2.java:18:40:18:54 | password : Object | semmle.label | password : Object |
2234
| SensitiveGetQuery2.java:19:61:19:68 | password | semmle.label | password |
35+
| SensitiveGetQuery3.java:11:41:11:47 | request : HttpServletRequest | semmle.label | request : HttpServletRequest |
36+
| SensitiveGetQuery3.java:12:21:12:60 | getRequestParameter(...) | semmle.label | getRequestParameter(...) |
37+
| SensitiveGetQuery3.java:12:21:12:60 | getRequestParameter(...) : String | semmle.label | getRequestParameter(...) : String |
38+
| SensitiveGetQuery3.java:12:41:12:47 | request : HttpServletRequest | semmle.label | request : HttpServletRequest |
39+
| SensitiveGetQuery3.java:13:57:13:64 | password | semmle.label | password |
40+
| SensitiveGetQuery.java:11:21:11:27 | request : HttpServletRequest | semmle.label | request : HttpServletRequest |
41+
| SensitiveGetQuery.java:12:21:12:27 | request : HttpServletRequest | semmle.label | request : HttpServletRequest |
2342
| SensitiveGetQuery.java:12:21:12:52 | getParameter(...) | semmle.label | getParameter(...) |
2443
| SensitiveGetQuery.java:12:21:12:52 | getParameter(...) : String | semmle.label | getParameter(...) : String |
2544
| SensitiveGetQuery.java:14:29:14:36 | password | semmle.label | password |
2645
| SensitiveGetQuery.java:14:29:14:36 | password : String | semmle.label | password : String |
2746
| SensitiveGetQuery.java:17:40:17:54 | password : String | semmle.label | password : String |
2847
| SensitiveGetQuery.java:18:61:18:68 | password | semmle.label | password |
2948
#select
30-
| SensitiveGetQuery2.java:14:30:14:48 | get(...) | SensitiveGetQuery2.java:12:13:12:37 | getParameterMap(...) : Map | SensitiveGetQuery2.java:14:30:14:48 | get(...) | $@ uses GET request method with sensitive information. | SensitiveGetQuery2.java:12:13:12:37 | getParameterMap(...) | sensitive query string |
31-
| SensitiveGetQuery2.java:15:29:15:36 | password | SensitiveGetQuery2.java:12:13:12:37 | getParameterMap(...) : Map | SensitiveGetQuery2.java:15:29:15:36 | password | $@ uses GET request method with sensitive information. | SensitiveGetQuery2.java:12:13:12:37 | getParameterMap(...) | sensitive query string |
32-
| SensitiveGetQuery2.java:19:61:19:68 | password | SensitiveGetQuery2.java:12:13:12:37 | getParameterMap(...) : Map | SensitiveGetQuery2.java:19:61:19:68 | password | $@ uses GET request method with sensitive information. | SensitiveGetQuery2.java:12:13:12:37 | getParameterMap(...) | sensitive query string |
33-
| SensitiveGetQuery.java:12:21:12:52 | getParameter(...) | SensitiveGetQuery.java:12:21:12:52 | getParameter(...) | SensitiveGetQuery.java:12:21:12:52 | getParameter(...) | $@ uses GET request method with sensitive information. | SensitiveGetQuery.java:12:21:12:52 | getParameter(...) | sensitive query string |
34-
| SensitiveGetQuery.java:14:29:14:36 | password | SensitiveGetQuery.java:12:21:12:52 | getParameter(...) : String | SensitiveGetQuery.java:14:29:14:36 | password | $@ uses GET request method with sensitive information. | SensitiveGetQuery.java:12:21:12:52 | getParameter(...) | sensitive query string |
35-
| SensitiveGetQuery.java:18:61:18:68 | password | SensitiveGetQuery.java:12:21:12:52 | getParameter(...) : String | SensitiveGetQuery.java:18:61:18:68 | password | $@ uses GET request method with sensitive information. | SensitiveGetQuery.java:12:21:12:52 | getParameter(...) | sensitive query string |
49+
| SensitiveGetQuery2.java:14:30:14:48 | get(...) | SensitiveGetQuery2.java:12:13:12:19 | request : HttpServletRequest | SensitiveGetQuery2.java:14:30:14:48 | get(...) | $@ uses GET request method with sensitive information. | SensitiveGetQuery2.java:12:13:12:19 | request | sensitive query string |
50+
| SensitiveGetQuery2.java:15:29:15:36 | password | SensitiveGetQuery2.java:12:13:12:19 | request : HttpServletRequest | SensitiveGetQuery2.java:15:29:15:36 | password | $@ uses GET request method with sensitive information. | SensitiveGetQuery2.java:12:13:12:19 | request | sensitive query string |
51+
| SensitiveGetQuery2.java:19:61:19:68 | password | SensitiveGetQuery2.java:12:13:12:19 | request : HttpServletRequest | SensitiveGetQuery2.java:19:61:19:68 | password | $@ uses GET request method with sensitive information. | SensitiveGetQuery2.java:12:13:12:19 | request | sensitive query string |
52+
| SensitiveGetQuery3.java:12:21:12:60 | getRequestParameter(...) | SensitiveGetQuery3.java:11:41:11:47 | request : HttpServletRequest | SensitiveGetQuery3.java:12:21:12:60 | getRequestParameter(...) | $@ uses GET request method with sensitive information. | SensitiveGetQuery3.java:11:41:11:47 | request | sensitive query string |
53+
| SensitiveGetQuery3.java:12:21:12:60 | getRequestParameter(...) | SensitiveGetQuery3.java:12:41:12:47 | request : HttpServletRequest | SensitiveGetQuery3.java:12:21:12:60 | getRequestParameter(...) | $@ uses GET request method with sensitive information. | SensitiveGetQuery3.java:12:41:12:47 | request | sensitive query string |
54+
| SensitiveGetQuery3.java:13:57:13:64 | password | SensitiveGetQuery3.java:11:41:11:47 | request : HttpServletRequest | SensitiveGetQuery3.java:13:57:13:64 | password | $@ uses GET request method with sensitive information. | SensitiveGetQuery3.java:11:41:11:47 | request | sensitive query string |
55+
| SensitiveGetQuery3.java:13:57:13:64 | password | SensitiveGetQuery3.java:12:41:12:47 | request : HttpServletRequest | SensitiveGetQuery3.java:13:57:13:64 | password | $@ uses GET request method with sensitive information. | SensitiveGetQuery3.java:12:41:12:47 | request | sensitive query string |
56+
| SensitiveGetQuery.java:12:21:12:52 | getParameter(...) | SensitiveGetQuery.java:11:21:11:27 | request : HttpServletRequest | SensitiveGetQuery.java:12:21:12:52 | getParameter(...) | $@ uses GET request method with sensitive information. | SensitiveGetQuery.java:11:21:11:27 | request | sensitive query string |
57+
| SensitiveGetQuery.java:12:21:12:52 | getParameter(...) | SensitiveGetQuery.java:12:21:12:27 | request : HttpServletRequest | SensitiveGetQuery.java:12:21:12:52 | getParameter(...) | $@ uses GET request method with sensitive information. | SensitiveGetQuery.java:12:21:12:27 | request | sensitive query string |
58+
| SensitiveGetQuery.java:14:29:14:36 | password | SensitiveGetQuery.java:11:21:11:27 | request : HttpServletRequest | SensitiveGetQuery.java:14:29:14:36 | password | $@ uses GET request method with sensitive information. | SensitiveGetQuery.java:11:21:11:27 | request | sensitive query string |
59+
| SensitiveGetQuery.java:14:29:14:36 | password | SensitiveGetQuery.java:12:21:12:27 | request : HttpServletRequest | SensitiveGetQuery.java:14:29:14:36 | password | $@ uses GET request method with sensitive information. | SensitiveGetQuery.java:12:21:12:27 | request | sensitive query string |
60+
| SensitiveGetQuery.java:18:61:18:68 | password | SensitiveGetQuery.java:11:21:11:27 | request : HttpServletRequest | SensitiveGetQuery.java:18:61:18:68 | password | $@ uses GET request method with sensitive information. | SensitiveGetQuery.java:11:21:11:27 | request | sensitive query string |
61+
| SensitiveGetQuery.java:18:61:18:68 | password | SensitiveGetQuery.java:12:21:12:27 | request : HttpServletRequest | SensitiveGetQuery.java:18:61:18:68 | password | $@ uses GET request method with sensitive information. | SensitiveGetQuery.java:12:21:12:27 | request | sensitive query string |

‎java/ql/test/experimental/query-tests/security/CWE-598/SensitiveGetQuery3.java‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import java.io.IOException;
2+
3+
import javax.servlet.http.HttpServlet;
4+
import javax.servlet.http.HttpServletRequest;
5+
import javax.servlet.http.HttpServletResponse;
6+
import javax.servlet.ServletException;
7+
8+
public class SensitiveGetQuery3 extends HttpServlet {
9+
// BAD - Tests sending sensitive information in a GET request.
10+
public void doGet(HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException {
11+
String username = getRequestParameter(request, "username");
12+
String password = getRequestParameter(request, "password");
13+
System.out.println("Username="+username+"; password="+password);
14+
}
15+
16+
String getRequestParameter(HttpServletRequest request, String paramName) {
17+
return request.getParameter(paramName);
18+
}
19+
20+
// GOOD - Tests sending sensitive information in a POST request.
21+
public void doPost(HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException {
22+
String username = getRequestParameter(request, "username");
23+
String password = getRequestParameter(request, "password");
24+
System.out.println("Username="+username+"; password="+password);
25+
}
26+
}

0 commit comments

Comments
 (0)