Skip to content

Commit add960b

Browse files
aschackmullaschackmull
authored
Merge pull request github#4880 from luchua-bc/java/sensitive-query-with-get
Java: Sensitive GET Query
2 parents 8262f03 + 56e3b30 commit add960b

File tree

11 files changed

+290
-0
lines changed

11 files changed

+290
-0
lines changed

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
public class SensitiveGetQuery extends HttpServlet {
2+
// BAD - Tests sending sensitive information in a GET request.
3+
public void doGet(HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException {
4+
String password = request.getParameter("password");
5+
System.out.println("password = " + password);
6+
}
7+
8+
// GOOD - Tests sending sensitive information in a POST request.
9+
public void doPost(HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException {
10+
String password = request.getParameter("password");
11+
System.out.println("password = " + password);
12+
}
13+
}

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
2+
<qhelp>
3+
4+
<overview>
5+
<p>Sensitive information such as user passwords should not be transmitted within the query string of the requested URL. Sensitive information within URLs may be logged in various locations, including the user's browser, the web server, and any forward or reverse proxy servers between the two endpoints. URLs may also be displayed on-screen, bookmarked or emailed around by users. They may be disclosed to third parties via the Referer header when any off-site links are followed. Placing passwords into the URL therefore increases the risk that they will be captured by an attacker.</p>
6+
</overview>
7+
8+
<recommendation>
9+
<p>Use HTTP POST to send sensitive information as part of the request body; for example, as form data.</p>
10+
</recommendation>
11+
12+
<example>
13+
<p>The following example shows two ways of sending sensitive information. In the 'BAD' case, a password is transmitted using the GET method. In the 'GOOD' case, the password is transmitted using the POST method.</p>
14+
<sample src="SensitiveGetQuery.java" />
15+
</example>
16+
17+
<references>
18+
<li>
19+
CWE:
20+
<a href="https://cwe.mitre.org/data/definitions/598.html">CWE-598: Use of GET Request Method with Sensitive Query Strings</a>
21+
</li>
22+
<li>
23+
PortSwigger (Burp):
24+
<a href="https://portswigger.net/kb/issues/00400300_password-submitted-using-get-method">Password Submitted using GET Method</a>
25+
</li>
26+
<li>
27+
OWASP:
28+
<a href="https://owasp.org/www-community/vulnerabilities/Information_exposure_through_query_strings_in_url">Information Exposure through Query Strings in URL</a>
29+
</li>
30+
</references>
31+
</qhelp>

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

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
/**
2+
* @name Sensitive GET Query
3+
* @description Use of GET request method with sensitive query strings.
4+
* @kind path-problem
5+
* @id java/sensitive-query-with-get
6+
* @tags security
7+
* external/cwe-598
8+
*/
9+
10+
import java
11+
import semmle.code.java.dataflow.FlowSources
12+
import semmle.code.java.dataflow.TaintTracking
13+
import semmle.code.java.security.SensitiveActions
14+
import DataFlow::PathGraph
15+
16+
/** A variable that holds sensitive information judging by its name. */
17+
class SensitiveInfoExpr extends Expr {
18+
SensitiveInfoExpr() {
19+
exists(Variable v | this = v.getAnAccess() |
20+
v.getName().regexpMatch(getCommonSensitiveInfoRegex()) and
21+
not v.getName().regexpMatch("token.*") // exclude ^token.* since sensitive tokens are usually in the form of accessToken, authToken, ...
22+
)
23+
}
24+
}
25+
26+
/** Holds if `m` is a method of some override of `HttpServlet.doGet`. */
27+
private predicate isGetServletMethod(Method m) {
28+
isServletRequestMethod(m) and m.getName() = "doGet"
29+
}
30+
31+
/** The `doGet` method of `HttpServlet`. */
32+
class DoGetServletMethod extends Method {
33+
DoGetServletMethod() { isGetServletMethod(this) }
34+
}
35+
36+
/** Holds if `ma` is (perhaps indirectly) called from the `doGet` method of `HttpServlet`. */
37+
predicate isReachableFromServletDoGet(MethodAccess ma) {
38+
ma.getEnclosingCallable() instanceof DoGetServletMethod
39+
or
40+
exists(Method pm, MethodAccess pma |
41+
ma.getEnclosingCallable() = pm and
42+
pma.getMethod() = pm and
43+
isReachableFromServletDoGet(pma)
44+
)
45+
}
46+
47+
/** Source of GET servlet requests. */
48+
class RequestGetParamSource extends DataFlow::ExprNode {
49+
RequestGetParamSource() {
50+
exists(MethodAccess ma |
51+
isRequestGetParamMethod(ma) and
52+
ma = this.asExpr() and
53+
isReachableFromServletDoGet(ma)
54+
)
55+
}
56+
}
57+
58+
/** A taint configuration tracking flow from the `ServletRequest` of a GET request handler to an expression whose name suggests it holds security-sensitive data. */
59+
class SensitiveGetQueryConfiguration extends TaintTracking::Configuration {
60+
SensitiveGetQueryConfiguration() { this = "SensitiveGetQueryConfiguration" }
61+
62+
override predicate isSource(DataFlow::Node source) { source instanceof RequestGetParamSource }
63+
64+
override predicate isSink(DataFlow::Node sink) { sink.asExpr() instanceof SensitiveInfoExpr }
65+
66+
/** Holds if the node is in a servlet method other than `doGet`. */
67+
override predicate isSanitizer(DataFlow::Node node) {
68+
isServletRequestMethod(node.getEnclosingCallable()) and
69+
not isGetServletMethod(node.getEnclosingCallable())
70+
}
71+
}
72+
73+
from DataFlow::PathNode source, DataFlow::PathNode sink, SensitiveGetQueryConfiguration c
74+
where c.hasFlowPath(source, sink)
75+
select sink.getNode(), source, sink,
76+
"$@ uses the GET request method to transmit sensitive information.", source.getNode(),
77+
"This request"

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,3 +322,18 @@ class ServletWebXMLListenerType extends RefType {
322322
// - `HttpSessionBindingListener`
323323
}
324324
}
325+
326+
/** Holds if `m` is a request handler method (for example `doGet` or `doPost`). */
327+
predicate isServletRequestMethod(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
339+
}

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
edges
2+
| SensitiveGetQuery2.java:12:13:12:37 | getParameterMap(...) : Map | SensitiveGetQuery2.java:14:21:14:48 | (...)... : Object |
3+
| SensitiveGetQuery2.java:14:21:14:48 | (...)... : Object | SensitiveGetQuery2.java:15:29:15:36 | password |
4+
| SensitiveGetQuery2.java:14:21:14:48 | (...)... : Object | SensitiveGetQuery2.java:15:29:15:36 | password : Object |
5+
| SensitiveGetQuery2.java:15:29:15:36 | password : Object | SensitiveGetQuery2.java:18:40:18:54 | password : Object |
6+
| SensitiveGetQuery2.java:18:40:18:54 | password : Object | SensitiveGetQuery2.java:19:61:19:68 | password |
7+
| SensitiveGetQuery3.java:12:21:12:60 | getRequestParameter(...) : String | SensitiveGetQuery3.java:13:57:13:64 | password |
8+
| SensitiveGetQuery3.java:17:10:17:40 | getParameter(...) : String | SensitiveGetQuery3.java:12:21:12:60 | getRequestParameter(...) : String |
9+
| SensitiveGetQuery4.java:14:24:14:66 | getRequestParameter(...) : String | SensitiveGetQuery4.java:16:37:16:47 | accessToken |
10+
| SensitiveGetQuery4.java:20:10:20:40 | getParameter(...) : String | SensitiveGetQuery4.java:14:24:14:66 | getRequestParameter(...) : String |
11+
| SensitiveGetQuery.java:12:21:12:52 | getParameter(...) : String | SensitiveGetQuery.java:14:29:14:36 | password |
12+
| SensitiveGetQuery.java:12:21:12:52 | getParameter(...) : String | SensitiveGetQuery.java:14:29:14:36 | password : String |
13+
| SensitiveGetQuery.java:14:29:14:36 | password : String | SensitiveGetQuery.java:17:40:17:54 | password : String |
14+
| SensitiveGetQuery.java:17:40:17:54 | password : String | SensitiveGetQuery.java:18:61:18:68 | password |
15+
nodes
16+
| SensitiveGetQuery2.java:12:13:12:37 | getParameterMap(...) : Map | semmle.label | getParameterMap(...) : Map |
17+
| SensitiveGetQuery2.java:14:21:14:48 | (...)... : Object | semmle.label | (...)... : Object |
18+
| SensitiveGetQuery2.java:15:29:15:36 | password | semmle.label | password |
19+
| SensitiveGetQuery2.java:15:29:15:36 | password : Object | semmle.label | password : Object |
20+
| SensitiveGetQuery2.java:18:40:18:54 | password : Object | semmle.label | password : Object |
21+
| SensitiveGetQuery2.java:19:61:19:68 | password | semmle.label | password |
22+
| SensitiveGetQuery3.java:12:21:12:60 | getRequestParameter(...) : String | semmle.label | getRequestParameter(...) : String |
23+
| SensitiveGetQuery3.java:13:57:13:64 | password | semmle.label | password |
24+
| SensitiveGetQuery3.java:17:10:17:40 | getParameter(...) : String | semmle.label | getParameter(...) : String |
25+
| SensitiveGetQuery4.java:14:24:14:66 | getRequestParameter(...) : String | semmle.label | getRequestParameter(...) : String |
26+
| SensitiveGetQuery4.java:16:37:16:47 | accessToken | semmle.label | accessToken |
27+
| SensitiveGetQuery4.java:20:10:20:40 | getParameter(...) : String | semmle.label | getParameter(...) : String |
28+
| SensitiveGetQuery.java:12:21:12:52 | getParameter(...) : String | semmle.label | getParameter(...) : String |
29+
| SensitiveGetQuery.java:14:29:14:36 | password | semmle.label | password |
30+
| SensitiveGetQuery.java:14:29:14:36 | password : String | semmle.label | password : String |
31+
| SensitiveGetQuery.java:17:40:17:54 | password : String | semmle.label | password : String |
32+
| SensitiveGetQuery.java:18:61:18:68 | password | semmle.label | password |
33+
#select
34+
| SensitiveGetQuery2.java:15:29:15:36 | password | SensitiveGetQuery2.java:12:13:12:37 | getParameterMap(...) : Map | SensitiveGetQuery2.java:15:29:15:36 | password | $@ uses the GET request method to transmit sensitive information. | SensitiveGetQuery2.java:12:13:12:37 | getParameterMap(...) | This request |
35+
| SensitiveGetQuery2.java:19:61:19:68 | password | SensitiveGetQuery2.java:12:13:12:37 | getParameterMap(...) : Map | SensitiveGetQuery2.java:19:61:19:68 | password | $@ uses the GET request method to transmit sensitive information. | SensitiveGetQuery2.java:12:13:12:37 | getParameterMap(...) | This request |
36+
| SensitiveGetQuery3.java:13:57:13:64 | password | SensitiveGetQuery3.java:17:10:17:40 | getParameter(...) : String | SensitiveGetQuery3.java:13:57:13:64 | password | $@ uses the GET request method to transmit sensitive information. | SensitiveGetQuery3.java:17:10:17:40 | getParameter(...) | This request |
37+
| SensitiveGetQuery4.java:16:37:16:47 | accessToken | SensitiveGetQuery4.java:20:10:20:40 | getParameter(...) : String | SensitiveGetQuery4.java:16:37:16:47 | accessToken | $@ uses the GET request method to transmit sensitive information. | SensitiveGetQuery4.java:20:10:20:40 | getParameter(...) | This request |
38+
| SensitiveGetQuery.java:14:29:14:36 | password | SensitiveGetQuery.java:12:21:12:52 | getParameter(...) : String | SensitiveGetQuery.java:14:29:14:36 | password | $@ uses the GET request method to transmit sensitive information. | SensitiveGetQuery.java:12:21:12:52 | getParameter(...) | This request |
39+
| SensitiveGetQuery.java:18:61:18:68 | password | SensitiveGetQuery.java:12:21:12:52 | getParameter(...) : String | SensitiveGetQuery.java:18:61:18:68 | password | $@ uses the GET request method to transmit sensitive information. | SensitiveGetQuery.java:12:21:12:52 | getParameter(...) | This request |

‎java/ql/test/experimental/query-tests/security/CWE-598/SensitiveGetQuery.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 SensitiveGetQuery extends HttpServlet {
9+
// BAD - Tests retrieving sensitive information through `request.getParameter()` in a GET request.
10+
public void doGet(HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException {
11+
String username = request.getParameter("username");
12+
String password = request.getParameter("password");
13+
14+
processUserInfo(username, password);
15+
}
16+
17+
void processUserInfo(String username, String password) {
18+
System.out.println("username = " + username+"; password "+password);
19+
}
20+
21+
// GOOD - Tests retrieving sensitive information through `request.getParameter()` in a POST request.
22+
public void doPost(HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException {
23+
String password = request.getParameter("password");
24+
System.out.println("password = " + password);
25+
}
26+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
experimental/Security/CWE/CWE-598/SensitiveGetQuery.ql

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

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

‎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 retrieving sensitive information through a wrapper call 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 retrieving sensitive information through a wrapper call 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+
}

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

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
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 SensitiveGetQuery4 extends HttpServlet {
9+
// BAD - Tests retrieving non-sensitive tokens and sensitive tokens in a GET request.
10+
public void doGet(HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException {
11+
String username = getRequestParameter(request, "username");
12+
String token = getRequestParameter(request, "token");
13+
String tokenType = getRequestParameter(request, "tokenType");
14+
String accessToken = getRequestParameter(request, "accessToken");
15+
System.out.println("Username="+username+"; token="+token+"; tokenType="+tokenType);
16+
System.out.println("AccessToken="+accessToken);
17+
}
18+
19+
String getRequestParameter(HttpServletRequest request, String paramName) {
20+
return request.getParameter(paramName);
21+
}
22+
23+
// GOOD - Tests retrieving non-sensitive tokens and sensitive tokens in a POST request.
24+
public void doPost(HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException {
25+
String username = getRequestParameter(request, "username");
26+
String token = getRequestParameter(request, "token");
27+
String tokenType = getRequestParameter(request, "tokenType");
28+
String accessToken = getRequestParameter(request, "accessToken");
29+
System.out.println("Username="+username+"; token="+token+"; tokenType="+tokenType);
30+
System.out.println("AccessToken="+accessToken);
31+
}
32+
}

0 commit comments

Comments
 (0)