Skip to content

Commit a83f9ce

Browse files
luchua-bcluchua-bc
committed
Change the query to only catch the common exception rethrown case
1 parent 3f0cdb6 commit a83f9ce

File tree

4 files changed

+12
-63
lines changed

4 files changed

+12
-63
lines changed

‎java/ql/src/experimental/Security/CWE/CWE-600/UncaughtServletException.ql‎

Lines changed: 1 addition & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -9,48 +9,11 @@
99

1010
import java
1111
import semmle.code.java.dataflow.FlowSources
12-
import semmle.code.java.dataflow.DataFlow2
1312
import semmle.code.java.dataflow.TaintTracking
1413
import semmle.code.java.frameworks.Servlets
1514
import semmle.code.xml.WebXML
1615
import DataFlow::PathGraph
1716

18-
/** DataFlow configuration for detecting the case of rethrowing unprocessed exceptions */
19-
class ThrowExConfiguration extends DataFlow2::Configuration {
20-
ThrowExConfiguration() { this = "Throwing Unprocessed Exception Configuration" }
21-
22-
/** Source of `LocalVariableDeclExpr` */
23-
override predicate isSource(DataFlow::Node source) {
24-
exists(LocalVariableDeclExpr v | source.asExpr() = v.getAnAccess())
25-
}
26-
27-
/** Sink of `ThrowStmt` */
28-
override predicate isSink(DataFlow::Node sink) {
29-
exists(ThrowStmt ts | sink.asExpr() = ts.getExpr()) // e.g. the uhex exception thrown in `catch (UnknownHostException uhex) {throw uhex;}`
30-
}
31-
32-
/**
33-
* Holds if there are additional flow steps below:
34-
* new IOException(uhex);
35-
* IOException ioException = new IOException(); ioException.initCause(e); throw ioException;
36-
* IOException ioException = new IOException(); ioException.addSuppressed(e); throw ioException;
37-
*/
38-
override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
39-
exists(ClassInstanceExpr cie | node1.asExpr() = cie.getAnArgument() and node2.asExpr() = cie) // catch (UnknownHostException uhex) {throw new IOException(uhex);}
40-
or
41-
exists(
42-
MethodAccess ma // e.g. IOException ioException = new IOException(); ioException.addSuppressed(e); throw ioException;
43-
|
44-
ma.getMethod().getName() in ["initCause", "addSuppressed"] and
45-
node1.asExpr() = ma.getAnArgument() and
46-
(
47-
node2.asExpr() = ma.getQualifier() or
48-
node2.asExpr() = ma
49-
)
50-
)
51-
}
52-
}
53-
5417
/** Holds if a given exception type is caught. */
5518
private predicate exceptionIsCaught(TryStmt t, RefType exType) {
5619
exists(CatchClause cc, LocalVariableDeclExpr v |
@@ -61,9 +24,7 @@ private predicate exceptionIsCaught(TryStmt t, RefType exType) {
6124
ThrowStmt ts // Detect the edge case that exception is caught then rethrown without processing in a catch clause
6225
|
6326
ts.getEnclosingStmt() = cc.getBlock() and
64-
exists(ThrowExConfiguration tec |
65-
tec.hasFlow(DataFlow::exprNode(v.getAnAccess()), DataFlow::exprNode(ts.getExpr()))
66-
)
27+
ts.getExpr() = v.getAnAccess()
6728
)
6829
)
6930
}

‎java/ql/test/experimental/query-tests/security/CWE-600/UncaughtServletException.expected‎

Lines changed: 4 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,35 +1,19 @@
11
edges
2-
| UncaughtServletException2.java:14:16:14:44 | getParameter(...) : String | UncaughtServletException2.java:15:45:15:46 | ip |
3-
| UncaughtServletException2.java:26:16:26:44 | getParameter(...) : String | UncaughtServletException2.java:27:45:27:46 | ip |
4-
| UncaughtServletException2.java:37:16:37:44 | getParameter(...) : String | UncaughtServletException2.java:38:45:38:46 | ip |
5-
| UncaughtServletException2.java:49:16:49:44 | getParameter(...) : String | UncaughtServletException2.java:50:45:50:46 | ip |
62
| UncaughtServletException.java:13:15:13:43 | getParameter(...) : String | UncaughtServletException.java:14:44:14:45 | ip |
73
| UncaughtServletException.java:16:19:16:41 | getRemoteUser(...) : String | UncaughtServletException.java:17:20:17:25 | userId |
84
| UncaughtServletException.java:54:16:54:44 | getParameter(...) : String | UncaughtServletException.java:55:45:55:46 | ip |
9-
| UncaughtServletException.java:74:21:74:43 | getRemoteUser(...) : String | UncaughtServletException.java:75:22:75:27 | userId |
5+
| UncaughtServletException.java:75:21:75:43 | getRemoteUser(...) : String | UncaughtServletException.java:76:22:76:27 | userId |
106
nodes
11-
| UncaughtServletException2.java:14:16:14:44 | getParameter(...) : String | semmle.label | getParameter(...) : String |
12-
| UncaughtServletException2.java:15:45:15:46 | ip | semmle.label | ip |
13-
| UncaughtServletException2.java:26:16:26:44 | getParameter(...) : String | semmle.label | getParameter(...) : String |
14-
| UncaughtServletException2.java:27:45:27:46 | ip | semmle.label | ip |
15-
| UncaughtServletException2.java:37:16:37:44 | getParameter(...) : String | semmle.label | getParameter(...) : String |
16-
| UncaughtServletException2.java:38:45:38:46 | ip | semmle.label | ip |
17-
| UncaughtServletException2.java:49:16:49:44 | getParameter(...) : String | semmle.label | getParameter(...) : String |
18-
| UncaughtServletException2.java:50:45:50:46 | ip | semmle.label | ip |
197
| UncaughtServletException.java:13:15:13:43 | getParameter(...) : String | semmle.label | getParameter(...) : String |
208
| UncaughtServletException.java:14:44:14:45 | ip | semmle.label | ip |
219
| UncaughtServletException.java:16:19:16:41 | getRemoteUser(...) : String | semmle.label | getRemoteUser(...) : String |
2210
| UncaughtServletException.java:17:20:17:25 | userId | semmle.label | userId |
2311
| UncaughtServletException.java:54:16:54:44 | getParameter(...) : String | semmle.label | getParameter(...) : String |
2412
| UncaughtServletException.java:55:45:55:46 | ip | semmle.label | ip |
25-
| UncaughtServletException.java:74:21:74:43 | getRemoteUser(...) : String | semmle.label | getRemoteUser(...) : String |
26-
| UncaughtServletException.java:75:22:75:27 | userId | semmle.label | userId |
13+
| UncaughtServletException.java:75:21:75:43 | getRemoteUser(...) : String | semmle.label | getRemoteUser(...) : String |
14+
| UncaughtServletException.java:76:22:76:27 | userId | semmle.label | userId |
2715
#select
28-
| UncaughtServletException2.java:15:45:15:46 | ip | UncaughtServletException2.java:14:16:14:44 | getParameter(...) : String | UncaughtServletException2.java:15:45:15:46 | ip | $@ flows to here and can throw uncaught exception. | UncaughtServletException2.java:14:16:14:44 | getParameter(...) | User-provided value |
29-
| UncaughtServletException2.java:27:45:27:46 | ip | UncaughtServletException2.java:26:16:26:44 | getParameter(...) : String | UncaughtServletException2.java:27:45:27:46 | ip | $@ flows to here and can throw uncaught exception. | UncaughtServletException2.java:26:16:26:44 | getParameter(...) | User-provided value |
30-
| UncaughtServletException2.java:38:45:38:46 | ip | UncaughtServletException2.java:37:16:37:44 | getParameter(...) : String | UncaughtServletException2.java:38:45:38:46 | ip | $@ flows to here and can throw uncaught exception. | UncaughtServletException2.java:37:16:37:44 | getParameter(...) | User-provided value |
31-
| UncaughtServletException2.java:50:45:50:46 | ip | UncaughtServletException2.java:49:16:49:44 | getParameter(...) : String | UncaughtServletException2.java:50:45:50:46 | ip | $@ flows to here and can throw uncaught exception. | UncaughtServletException2.java:49:16:49:44 | getParameter(...) | User-provided value |
3216
| UncaughtServletException.java:14:44:14:45 | ip | UncaughtServletException.java:13:15:13:43 | getParameter(...) : String | UncaughtServletException.java:14:44:14:45 | ip | $@ flows to here and can throw uncaught exception. | UncaughtServletException.java:13:15:13:43 | getParameter(...) | User-provided value |
3317
| UncaughtServletException.java:17:20:17:25 | userId | UncaughtServletException.java:16:19:16:41 | getRemoteUser(...) : String | UncaughtServletException.java:17:20:17:25 | userId | $@ flows to here and can throw uncaught exception. | UncaughtServletException.java:16:19:16:41 | getRemoteUser(...) | User-provided value |
3418
| UncaughtServletException.java:55:45:55:46 | ip | UncaughtServletException.java:54:16:54:44 | getParameter(...) : String | UncaughtServletException.java:55:45:55:46 | ip | $@ flows to here and can throw uncaught exception. | UncaughtServletException.java:54:16:54:44 | getParameter(...) | User-provided value |
35-
| UncaughtServletException.java:75:22:75:27 | userId | UncaughtServletException.java:74:21:74:43 | getRemoteUser(...) : String | UncaughtServletException.java:75:22:75:27 | userId | $@ flows to here and can throw uncaught exception. | UncaughtServletException.java:74:21:74:43 | getRemoteUser(...) | User-provided value |
19+
| UncaughtServletException.java:76:22:76:27 | userId | UncaughtServletException.java:75:21:75:43 | getRemoteUser(...) : String | UncaughtServletException.java:76:22:76:27 | userId | $@ flows to here and can throw uncaught exception. | UncaughtServletException.java:75:21:75:43 | getRemoteUser(...) | User-provided value |

‎java/ql/test/experimental/query-tests/security/CWE-600/UncaughtServletException.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,8 @@ public void doOptions(HttpServletRequest request, HttpServletResponse response)
5454
String ip = request.getParameter("srcIP");
5555
InetAddress addr = InetAddress.getByName(ip);
5656
} catch (UnknownHostException uhex) {
57-
throw new IOException(uhex);
57+
uhex.printStackTrace();
58+
throw uhex;
5859
}
5960
}
6061

‎java/ql/test/experimental/query-tests/security/CWE-600/UncaughtServletException2.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
class UncaughtServletException2 extends HttpServlet {
1111
// BAD - Tests rethrowing caught exceptions with stack trace using `initCause(...)`
12+
// Note this special case is not being handled by the query since in 99% of cases we're looking for `catch(Exception e) { ... throw e; }`
1213
public void doGet(HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException {
1314
try {
1415
String ip = request.getParameter("srcIP");
@@ -21,17 +22,18 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) thro
2122
}
2223

2324
// BAD - Tests rethrowing caught exceptions with stack trace using the same exception variable.
25+
// Note this special case is not being handled by the query since in 99% of cases we're looking for `catch(Exception e) { ... throw e; }`
2426
public void doHead(HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException {
2527
try {
2628
String ip = request.getParameter("srcIP");
2729
InetAddress addr = InetAddress.getByName(ip);
2830
} catch (UnknownHostException uhex) {
29-
uhex.printStackTrace();
30-
throw uhex;
31+
throw new IOException(uhex);
3132
}
3233
}
3334

3435
// BAD - Tests rethrowing caught exceptions with stack trace using `addSuppressed(...)`.
36+
// Note this special case is not being handled by the query since in 99% of cases we're looking for `catch(Exception e) { ... throw e; }`
3537
public void doTrace(HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException {
3638
try {
3739
String ip = request.getParameter("srcIP");
@@ -44,6 +46,7 @@ public void doTrace(HttpServletRequest request, HttpServletResponse response) th
4446
}
4547

4648
// BAD - Tests rethrowing caught exceptions with stack trace using `initCause(...)`
49+
// Note this special case is not being handled by the query since in 99% of cases we're looking for `catch(Exception e) { ... throw e; }`
4750
public void doPost(HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException {
4851
try {
4952
String ip = request.getParameter("srcIP");

0 commit comments

Comments
 (0)