Skip to content

Commit 378db7d

Browse files
committed
Remove local user input and use fluent model
1 parent 5264936 commit 378db7d

File tree

4 files changed

+5
-37
lines changed

4 files changed

+5
-37
lines changed

java/ql/src/experimental/Security/CWE/CWE-400/ThreadPauseSink.qll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ private class MathCompDataModel extends SummaryModelCsv {
99
override predicate row(string row) {
1010
row =
1111
[
12-
"java.lang;Math;false;min;;;Argument[0..1];ReturnValue;taint",
13-
"java.lang;Math;false;max;;;Argument[0..1];ReturnValue;taint"
12+
"java.lang;Math;false;min;;;Argument[0..1];ReturnValue;value",
13+
"java.lang;Math;false;max;;;Argument[0..1];ReturnValue;value"
1414
]
1515
}
1616
}

java/ql/src/experimental/Security/CWE/CWE-400/ThreadResourceAbuse.ql

Lines changed: 2 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
* or even resource exhaustion.
55
* @kind path-problem
66
* @id java/thread-resource-abuse
7+
* @problem.severity warning
78
* @tags security
89
* external/cwe/cwe-400
910
*/
@@ -13,32 +14,6 @@ import ThreadPauseSink
1314
import semmle.code.java.dataflow.FlowSources
1415
import DataFlow::PathGraph
1516

16-
/** The `getInitParameter` method of servlet or JSF. */
17-
class GetInitParameter extends Method {
18-
GetInitParameter() {
19-
(
20-
this.getDeclaringType()
21-
.getASupertype*()
22-
.hasQualifiedName(["javax.servlet", "jakarta.servlet"],
23-
["FilterConfig", "Registration", "ServletConfig", "ServletContext"]) or
24-
this.getDeclaringType()
25-
.getASupertype*()
26-
.hasQualifiedName(["javax.faces.context", "jakarta.faces.context"], "ExternalContext")
27-
) and
28-
this.getName() = "getInitParameter"
29-
}
30-
}
31-
32-
/** An access to the `getInitParameter` method. */
33-
class GetInitParameterAccess extends MethodAccess {
34-
GetInitParameterAccess() { this.getMethod() instanceof GetInitParameter }
35-
}
36-
37-
/* Init parameter input of a Java EE web application. */
38-
class InitParameterInput extends LocalUserInput {
39-
InitParameterInput() { this.asExpr() instanceof GetInitParameterAccess }
40-
}
41-
4217
private class LessThanSanitizer extends DataFlow::BarrierGuard {
4318
LessThanSanitizer() { this instanceof ComparisonExpr }
4419

@@ -55,9 +30,7 @@ private class LessThanSanitizer extends DataFlow::BarrierGuard {
5530
class ThreadResourceAbuse extends TaintTracking::Configuration {
5631
ThreadResourceAbuse() { this = "ThreadResourceAbuse" }
5732

58-
override predicate isSource(DataFlow::Node source) {
59-
source instanceof RemoteFlowSource or source instanceof LocalUserInput
60-
}
33+
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
6134

6235
override predicate isSink(DataFlow::Node sink) { sink instanceof PauseThreadSink }
6336

java/ql/test/experimental/query-tests/security/CWE-400/ThreadResourceAbuse.expected

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,13 @@ edges
33
| ThreadResourceAbuse.java:21:28:21:36 | delayTime : Number | ThreadResourceAbuse.java:74:18:74:25 | waitTime |
44
| ThreadResourceAbuse.java:29:82:29:114 | getParameter(...) : String | ThreadResourceAbuse.java:30:28:30:36 | delayTime : Number |
55
| ThreadResourceAbuse.java:30:28:30:36 | delayTime : Number | ThreadResourceAbuse.java:74:18:74:25 | waitTime |
6-
| ThreadResourceAbuse.java:37:25:37:73 | getInitParameter(...) : String | ThreadResourceAbuse.java:40:28:40:36 | delayTime : Number |
7-
| ThreadResourceAbuse.java:40:28:40:36 | delayTime : Number | ThreadResourceAbuse.java:74:18:74:25 | waitTime |
86
| ThreadResourceAbuse.java:141:27:141:43 | getValue(...) : String | ThreadResourceAbuse.java:144:34:144:42 | delayTime |
97
| ThreadResourceAbuse.java:172:19:172:50 | getHeader(...) : String | ThreadResourceAbuse.java:177:17:177:26 | retryAfter |
108
nodes
119
| ThreadResourceAbuse.java:18:25:18:57 | getParameter(...) : String | semmle.label | getParameter(...) : String |
1210
| ThreadResourceAbuse.java:21:28:21:36 | delayTime : Number | semmle.label | delayTime : Number |
1311
| ThreadResourceAbuse.java:29:82:29:114 | getParameter(...) : String | semmle.label | getParameter(...) : String |
1412
| ThreadResourceAbuse.java:30:28:30:36 | delayTime : Number | semmle.label | delayTime : Number |
15-
| ThreadResourceAbuse.java:37:25:37:73 | getInitParameter(...) : String | semmle.label | getInitParameter(...) : String |
16-
| ThreadResourceAbuse.java:40:28:40:36 | delayTime : Number | semmle.label | delayTime : Number |
1713
| ThreadResourceAbuse.java:74:18:74:25 | waitTime | semmle.label | waitTime |
1814
| ThreadResourceAbuse.java:141:27:141:43 | getValue(...) : String | semmle.label | getValue(...) : String |
1915
| ThreadResourceAbuse.java:144:34:144:42 | delayTime | semmle.label | delayTime |
@@ -22,6 +18,5 @@ nodes
2218
#select
2319
| ThreadResourceAbuse.java:74:18:74:25 | waitTime | ThreadResourceAbuse.java:18:25:18:57 | getParameter(...) : String | ThreadResourceAbuse.java:74:18:74:25 | waitTime | Vulnerability of uncontrolled resource consumption due to $@. | ThreadResourceAbuse.java:18:25:18:57 | getParameter(...) | user-provided value |
2420
| ThreadResourceAbuse.java:74:18:74:25 | waitTime | ThreadResourceAbuse.java:29:82:29:114 | getParameter(...) : String | ThreadResourceAbuse.java:74:18:74:25 | waitTime | Vulnerability of uncontrolled resource consumption due to $@. | ThreadResourceAbuse.java:29:82:29:114 | getParameter(...) | user-provided value |
25-
| ThreadResourceAbuse.java:74:18:74:25 | waitTime | ThreadResourceAbuse.java:37:25:37:73 | getInitParameter(...) : String | ThreadResourceAbuse.java:74:18:74:25 | waitTime | Vulnerability of uncontrolled resource consumption due to $@. | ThreadResourceAbuse.java:37:25:37:73 | getInitParameter(...) | user-provided value |
2621
| ThreadResourceAbuse.java:144:34:144:42 | delayTime | ThreadResourceAbuse.java:141:27:141:43 | getValue(...) : String | ThreadResourceAbuse.java:144:34:144:42 | delayTime | Vulnerability of uncontrolled resource consumption due to $@. | ThreadResourceAbuse.java:141:27:141:43 | getValue(...) | user-provided value |
2722
| ThreadResourceAbuse.java:177:17:177:26 | retryAfter | ThreadResourceAbuse.java:172:19:172:50 | getHeader(...) : String | ThreadResourceAbuse.java:177:17:177:26 | retryAfter | Vulnerability of uncontrolled resource consumption due to $@. | ThreadResourceAbuse.java:172:19:172:50 | getHeader(...) | user-provided value |

java/ql/test/experimental/query-tests/security/CWE-400/ThreadResourceAbuse.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ protected void doGet2(HttpServletRequest request, HttpServletResponse response)
3333
}
3434

3535
protected void doPost(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
36-
// Get thread pause time from init container parameter
36+
// Get thread pause time from init container parameter (not detected because LocalUserInput tends to add a lot of FP)
3737
String delayTimeStr = getServletContext().getInitParameter("DelayTime");
3838
try {
3939
int delayTime = Integer.valueOf(delayTimeStr);

0 commit comments

Comments
 (0)