hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2025-12-22 03:36:30 +01:00
Code Issues Packages Projects Releases Wiki Activity

Update the query

Browse Source
This commit is contained in:
luchua-bc
2021-09-24 01:48:11 +00:00
parent 2dc38aee54
commit 272e4f6cf9
4 changed files with 122 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
java/ql/src/experimental/Security/CWE/CWE-400/ThreadResourceAbuse.qhelp
View File

@@ -26,7 +26,7 @@ check on maximum allowed sleep time is enforced.</p>
<references> <references>
<li> <li>
snyk: Snyk:
<a href="https://snyk.io/vuln/SNYK-JAVA-COMGOOGLECODEGWTUPLOAD-569506">Denial of Service (DoS) <a href="https://snyk.io/vuln/SNYK-JAVA-COMGOOGLECODEGWTUPLOAD-569506">Denial of Service (DoS)
in com.googlecode.gwtupload:gwtupload</a>. in com.googlecode.gwtupload:gwtupload</a>.
</li> </li>

11
java/ql/src/experimental/Security/CWE/CWE-400/ThreadResourceAbuse.ql
View File

@@ -40,11 +40,14 @@ class InitParameterInput extends LocalUserInput {
} }
private class LessThanSanitizer extends DataFlow::BarrierGuard { private class LessThanSanitizer extends DataFlow::BarrierGuard {
LessThanSanitizer() { this instanceof LTExpr } LessThanSanitizer() { this instanceof ComparisonExpr }
override predicate checks(Expr e, boolean branch) { override predicate checks(Expr e, boolean branch) {
e = this.(LTExpr).getLeftOperand() and e = this.(ComparisonExpr).getLesserOperand() and
branch = true branch = true
or
e = this.(ComparisonExpr).getGreaterOperand() and
branch = false
} }
} }
@@ -59,13 +62,11 @@ class ThreadResourceAbuse extends TaintTracking::Configuration {
override predicate isSink(DataFlow::Node sink) { sink instanceof PauseThreadSink } override predicate isSink(DataFlow::Node sink) { sink instanceof PauseThreadSink }
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) { override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
exists(ConditionalExpr ce | ce.getAChildExpr() = node1.asExpr() and ce = node2.asExpr()) // request.getParameter("nodelay") != null ? 0 : sleepTime
or
exists( exists(
Method rm, ClassInstanceExpr ce, Argument arg, FieldAccess fa // thread.start() invokes the run() method of thread implementation Method rm, ClassInstanceExpr ce, Argument arg, FieldAccess fa // thread.start() invokes the run() method of thread implementation
| |
rm.hasName("run") and rm.hasName("run") and
ce.getConstructedType() = rm.getSourceDeclaration().getDeclaringType() and ce.getConstructedType().getSourceDeclaration() = rm.getSourceDeclaration().getDeclaringType() and
ce.getConstructedType().getASupertype*().hasQualifiedName("java.lang", "Runnable") and ce.getConstructedType().getASupertype*().hasQualifiedName("java.lang", "Runnable") and
ce.getAnArgument() = arg and ce.getAnArgument() = arg and
fa = rm.getAnAccessedField().getAnAccess() and fa = rm.getAnAccessedField().getAnAccess() and

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

@@ -1,18 +1,27 @@
edges edges
| ThreadResourceAbuse.java:15:25:15:57 | getParameter(...) : String | ThreadResourceAbuse.java:18:28:18:36 | delayTime : Number | | ThreadResourceAbuse.java:18:25:18:57 | getParameter(...) : String | ThreadResourceAbuse.java:21:28:21:36 | delayTime : Number |
| ThreadResourceAbuse.java:18:28:18:36 | delayTime : Number | ThreadResourceAbuse.java:62:18:62:25 | waitTime | | ThreadResourceAbuse.java:21:28:21:36 | delayTime : Number | ThreadResourceAbuse.java:74:18:74:25 | waitTime |
| ThreadResourceAbuse.java:25:25:25:73 | getInitParameter(...) : String | ThreadResourceAbuse.java:28:28:28:36 | delayTime : Number | | ThreadResourceAbuse.java:29:82:29:114 | getParameter(...) : String | ThreadResourceAbuse.java:30:28:30:36 | delayTime : Number |
| ThreadResourceAbuse.java:28:28:28:36 | delayTime : Number | ThreadResourceAbuse.java:62:18:62:25 | waitTime | | ThreadResourceAbuse.java:30:28:30:36 | delayTime : Number | ThreadResourceAbuse.java:74:18:74:25 | waitTime |
| ThreadResourceAbuse.java:97:27:97:43 | getValue(...) : String | ThreadResourceAbuse.java:100:34:100:42 | delayTime | | ThreadResourceAbuse.java:37:25:37:73 | getInitParameter(...) : String | ThreadResourceAbuse.java:40:28:40:36 | delayTime : Number |
| ThreadResourceAbuse.java:40:28:40:36 | delayTime : Number | ThreadResourceAbuse.java:74:18:74:25 | waitTime |
| ThreadResourceAbuse.java:141:27:141:43 | getValue(...) : String | ThreadResourceAbuse.java:144:34:144:42 | delayTime |
| ThreadResourceAbuse.java:172:19:172:50 | getHeader(...) : String | ThreadResourceAbuse.java:177:17:177:26 | retryAfter |
nodes nodes
| ThreadResourceAbuse.java:15:25:15:57 | getParameter(...) : String | semmle.label | getParameter(...) : String | | ThreadResourceAbuse.java:18:25:18:57 | getParameter(...) : String | semmle.label | getParameter(...) : String |
| ThreadResourceAbuse.java:18:28:18:36 | delayTime : Number | semmle.label | delayTime : Number | | ThreadResourceAbuse.java:21:28:21:36 | delayTime : Number | semmle.label | delayTime : Number |
| ThreadResourceAbuse.java:25:25:25:73 | getInitParameter(...) : String | semmle.label | getInitParameter(...) : String | | ThreadResourceAbuse.java:29:82:29:114 | getParameter(...) : String | semmle.label | getParameter(...) : String |
| ThreadResourceAbuse.java:28:28:28:36 | delayTime : Number | semmle.label | delayTime : Number | | ThreadResourceAbuse.java:30:28:30:36 | delayTime : Number | semmle.label | delayTime : Number |
| ThreadResourceAbuse.java:62:18:62:25 | waitTime | semmle.label | waitTime | | ThreadResourceAbuse.java:37:25:37:73 | getInitParameter(...) : String | semmle.label | getInitParameter(...) : String |
| ThreadResourceAbuse.java:97:27:97:43 | getValue(...) : String | semmle.label | getValue(...) : String | | ThreadResourceAbuse.java:40:28:40:36 | delayTime : Number | semmle.label | delayTime : Number |
| ThreadResourceAbuse.java:100:34:100:42 | delayTime | semmle.label | delayTime | | ThreadResourceAbuse.java:74:18:74:25 | waitTime | semmle.label | waitTime |
| ThreadResourceAbuse.java:141:27:141:43 | getValue(...) : String | semmle.label | getValue(...) : String |
| ThreadResourceAbuse.java:144:34:144:42 | delayTime | semmle.label | delayTime |
| ThreadResourceAbuse.java:172:19:172:50 | getHeader(...) : String | semmle.label | getHeader(...) : String |
| ThreadResourceAbuse.java:177:17:177:26 | retryAfter | semmle.label | retryAfter |
#select #select
| ThreadResourceAbuse.java:62:18:62:25 | waitTime | ThreadResourceAbuse.java:15:25:15:57 | getParameter(...) : String | ThreadResourceAbuse.java:62:18:62:25 | waitTime | Vulnerability of uncontrolled resource consumption due to $@. | ThreadResourceAbuse.java:15:25:15:57 | getParameter(...) | user-provided value | | 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 |
| ThreadResourceAbuse.java:62:18:62:25 | waitTime | ThreadResourceAbuse.java:25:25:25:73 | getInitParameter(...) : String | ThreadResourceAbuse.java:62:18:62:25 | waitTime | Vulnerability of uncontrolled resource consumption due to $@. | ThreadResourceAbuse.java:25:25:25:73 | getInitParameter(...) | user-provided value | | 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 |
| ThreadResourceAbuse.java:100:34:100:42 | delayTime | ThreadResourceAbuse.java:97:27:97:43 | getValue(...) : String | ThreadResourceAbuse.java:100:34:100:42 | delayTime | Vulnerability of uncontrolled resource consumption due to $@. | ThreadResourceAbuse.java:97:27:97:43 | getValue(...) | user-provided value | | 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 |
| 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 |
| 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 |

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

@@ -10,6 +10,9 @@ import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpServletResponse;
public class ThreadResourceAbuse extends HttpServlet { public class ThreadResourceAbuse extends HttpServlet {
static final int DEFAULT_RETRY_AFTER = 5*1000;
static final int MAX_RETRY_AFTER = 10*1000;
protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
// Get thread pause time from request parameter // Get thread pause time from request parameter
String delayTimeStr = request.getParameter("DelayTime"); String delayTimeStr = request.getParameter("DelayTime");
@@ -20,6 +23,15 @@ public class ThreadResourceAbuse extends HttpServlet {
} }
} }
protected void doGet2(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
// Get thread pause time from request parameter
try {
int delayTime = request.getParameter("nodelay") != null ? 0 : Integer.valueOf(request.getParameter("DelayTime"));
new UncheckedSyncAction(delayTime).start();
} catch (NumberFormatException e) {
}
}
protected void doPost(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { protected void doPost(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
// Get thread pause time from init container parameter // Get thread pause time from init container parameter
String delayTimeStr = getServletContext().getInitParameter("DelayTime"); String delayTimeStr = getServletContext().getInitParameter("DelayTime");
@@ -86,6 +98,38 @@ public class ThreadResourceAbuse extends HttpServlet {
} }
} }
class CheckedSyncAction2 extends Thread {
int waitTime;
public CheckedSyncAction2(int waitTime) {
this.waitTime = waitTime;
}
// GOOD: enforce an upper limit on wait time
@Override
public void run() {
try {
if (waitTime >= 5000) {
// No action
} else {
Thread.sleep(waitTime);
}
// Do other updates
} catch (InterruptedException e) {
}
}
}
protected void doPost2(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
// Get thread pause time from init container parameter
String delayTimeStr = getServletContext().getInitParameter("DelayTime");
try {
int delayTime = Integer.valueOf(delayTimeStr);
new CheckedSyncAction2(delayTime).start();
} catch (NumberFormatException e) {
}
}
protected void doHead(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { protected void doHead(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
// Get thread pause time from request cookie // Get thread pause time from request cookie
Cookie[] cookies = request.getCookies(); Cookie[] cookies = request.getCookies();
@@ -105,4 +149,47 @@ public class ThreadResourceAbuse extends HttpServlet {
} }
} }
} }
int parseReplyAfter(String value) {
if (value == null || value.isEmpty()) {
return DEFAULT_RETRY_AFTER;
}
try {
int n = Integer.parseInt(value);
if (n < 0) {
return DEFAULT_RETRY_AFTER;
}
return Math.min(n, MAX_RETRY_AFTER);
} catch (NumberFormatException e) {
return DEFAULT_RETRY_AFTER;
}
}
protected void doHead2(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
// Get thread pause time from request header
String header = request.getHeader("Retry-After");
int retryAfter = Integer.parseInt(header);
try {
// BAD: wait for retry-after without input validation
Thread.sleep(retryAfter);
} catch (InterruptedException ignore) {
// ignore
}
}
protected void doHead3(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
// Get thread pause time from request header
String header = request.getHeader("Retry-After");
int retryAfter = parseReplyAfter(header);
try {
// GOOD: wait for retry-after with input validation
Thread.sleep(retryAfter);
} catch (InterruptedException ignore) {
// ignore
}
}
} }