Skip to content

Commit 1ce31ec

Browse files
luchua-bcluchua-bc
committed
Add sinks of servlet dispatcher and filter
1 parent 962069c commit 1ce31ec

File tree

4 files changed

+184
-28
lines changed

4 files changed

+184
-28
lines changed

‎java/ql/src/experimental/Security/CWE/CWE-625/PermissiveDotRegex.ql

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,24 @@ private class GetServletUriSource extends SourceModelCsv {
3333
}
3434
}
3535

36+
/** Sink model of servlet dispatcher. */
37+
private class UrlDispatchSink extends SinkModelCsv {
38+
override predicate row(string row) {
39+
row =
40+
[
41+
"javax.servlet;RequestDispatcher;false;forward;;;Argument[-1];url-dispatch;manual",
42+
"javax.servlet;RequestDispatcher;false;include;;;Argument[-1];url-dispatch;manual"
43+
]
44+
}
45+
}
46+
47+
/** Sink model of servlet filter. */
48+
private class UrlFilterSink extends SinkModelCsv {
49+
override predicate row(string row) {
50+
row = ["javax.servlet;FilterChain;true;doFilter;;;Argument[-1];url-filter;manual"]
51+
}
52+
}
53+
3654
/**
3755
* `.` without a `\` prefix, which is likely not a character literal in regex
3856
*/
@@ -119,7 +137,11 @@ class MatchRegexConfiguration extends TaintTracking::Configuration {
119137
DataFlow::localExprFlow(ce, guard.(MethodAccess).getQualifier()) or
120138
DataFlow::localExprFlow(ce, guard.(MethodAccess).getAnArgument())
121139
) and
122-
DataFlow::exprNode(se) instanceof UrlRedirectSink and
140+
(
141+
DataFlow::exprNode(se) instanceof UrlRedirectSink or
142+
sinkNode(DataFlow::exprNode(se), "url-dispatch") or
143+
sinkNode(DataFlow::exprNode(se), "url-filter")
144+
) and
123145
guard.controls(se.getBasicBlock(), true)
124146
)
125147
}

‎java/ql/test/experimental/query-tests/security/CWE-625/DotRegexFilter.java

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
import java.io.IOException;
2+
import java.util.regex.Matcher;
3+
import java.util.regex.Pattern;
4+
5+
import javax.servlet.Filter;
6+
import javax.servlet.FilterChain;
7+
import javax.servlet.FilterConfig;
8+
import javax.servlet.ServletContext;
9+
import javax.servlet.ServletException;
10+
import javax.servlet.ServletRequest;
11+
import javax.servlet.ServletResponse;
12+
import javax.servlet.http.HttpServletRequest;
13+
import javax.servlet.http.HttpServletResponse;
14+
15+
public class DotRegexFilter implements Filter {
16+
private static final String PROTECTED_PATTERN = "/protected/.*";
17+
private static final String CONSTRAINT_PATTERN = "/protected/xyz\\.xml";
18+
19+
private ServletContext context;
20+
21+
public void init(FilterConfig config) throws ServletException {
22+
this.context = config.getServletContext();
23+
}
24+
25+
// BAD: A string with line return e.g. `/protected/%0dxyz` can bypass the path check
26+
public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException {
27+
HttpServletRequest httpRequest = (HttpServletRequest) request;
28+
HttpServletResponse httpResponse = (HttpServletResponse) response;
29+
String source = httpRequest.getPathInfo();
30+
31+
Pattern p = Pattern.compile(PROTECTED_PATTERN);
32+
Matcher m = p.matcher(source);
33+
34+
if (m.matches()) {
35+
// Protected page - check access token and redirect to login page
36+
if (!httpRequest.getSession().getAttribute("secAttr").equals("secValue")) {
37+
// Redirect to the login page
38+
httpResponse.sendRedirect("/login.html");
39+
} else {
40+
// Not protected page - pass the request along the filter chain
41+
chain.doFilter(request, response);
42+
}
43+
}
44+
}
45+
46+
// GOOD: A string with line return e.g. `/protected/%0dxyz` cannot bypass the path check
47+
public void doFilter2(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException {
48+
HttpServletRequest httpRequest = (HttpServletRequest) request;
49+
HttpServletResponse httpResponse = (HttpServletResponse) response;
50+
String source = httpRequest.getPathInfo();
51+
52+
Pattern p = Pattern.compile(CONSTRAINT_PATTERN);
53+
Matcher m = p.matcher(source);
54+
55+
if (m.matches()) {
56+
// Protected page - check access token and redirect to login page
57+
if (!httpRequest.getSession().getAttribute("secAttr").equals("secValue")) {
58+
// Redirect to the login page
59+
httpResponse.sendRedirect("/login.html");
60+
} else {
61+
// Not protected page - pass the request along the filter chain
62+
chain.doFilter(request, response);
63+
}
64+
}
65+
}
66+
67+
public void destroy() {
68+
// Close resources
69+
}
70+
}

‎java/ql/test/experimental/query-tests/security/CWE-625/DotRegexServlet.java

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import javax.servlet.http.HttpServlet;
66
import javax.servlet.http.HttpServletRequest;
77
import javax.servlet.http.HttpServletResponse;
8+
import javax.servlet.RequestDispatcher;
89
import javax.servlet.ServletException;
910

1011
public class DotRegexServlet extends HttpServlet {
@@ -104,4 +105,46 @@ protected void doGet5(HttpServletRequest request, HttpServletResponse response)
104105
}
105106
}
106107
}
108+
109+
// BAD: A string with line return e.g. `/protected/%0dxyz` can bypass the path check
110+
protected void doGet6(HttpServletRequest request, HttpServletResponse response)
111+
throws ServletException, IOException {
112+
String source = request.getPathInfo();
113+
114+
Pattern p = Pattern.compile(PROTECTED_PATTERN);
115+
Matcher m = p.matcher(source);
116+
117+
if (m.matches()) {
118+
// Protected page - check access token and redirect to login page
119+
if (!request.getSession().getAttribute("secAttr").equals("secValue")) {
120+
RequestDispatcher dispatcher = getServletContext().getRequestDispatcher("/login");
121+
dispatcher.forward(request, response);
122+
} else {
123+
// Not protected page - render content
124+
RequestDispatcher dispatcher = getServletContext().getRequestDispatcher(source);
125+
dispatcher.forward(request, response);
126+
}
127+
}
128+
}
129+
130+
// GOOD: A string with line return e.g. `/protected/%0dxyz` cannot bypass the path check
131+
protected void doGet7(HttpServletRequest request, HttpServletResponse response)
132+
throws ServletException, IOException {
133+
String source = request.getPathInfo();
134+
135+
Pattern p = Pattern.compile(PROTECTED_PATTERN, Pattern.DOTALL);
136+
Matcher m = p.matcher(source);
137+
138+
if (m.matches()) {
139+
// Protected page - check access token and redirect to login page
140+
if (!request.getSession().getAttribute("secAttr").equals("secValue")) {
141+
RequestDispatcher dispatcher = getServletContext().getRequestDispatcher("/login");
142+
dispatcher.forward(request, response);
143+
} else {
144+
// Not protected page - render content
145+
RequestDispatcher dispatcher = getServletContext().getRequestDispatcher(source);
146+
dispatcher.include(request, response);
147+
}
148+
}
149+
}
107150
}

‎java/ql/test/experimental/query-tests/security/CWE-625/PermissiveDotRegex.expected

Lines changed: 48 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,52 @@
11
edges
2-
| DotRegexServlet.java:11:30:11:46 | PROTECTED_PATTERN : String | DotRegexServlet.java:20:31:20:47 | PROTECTED_PATTERN |
3-
| DotRegexServlet.java:11:30:11:46 | PROTECTED_PATTERN : String | DotRegexServlet.java:58:36:58:52 | PROTECTED_PATTERN |
4-
| DotRegexServlet.java:11:30:11:46 | PROTECTED_PATTERN : String | DotRegexServlet.java:76:37:76:53 | PROTECTED_PATTERN |
5-
| DotRegexServlet.java:11:50:11:64 | "/protected/.*" : String | DotRegexServlet.java:11:30:11:46 | PROTECTED_PATTERN : String |
6-
| DotRegexServlet.java:18:19:18:39 | getPathInfo(...) : String | DotRegexServlet.java:21:25:21:30 | source |
7-
| DotRegexServlet.java:37:19:37:39 | getPathInfo(...) : String | DotRegexServlet.java:40:25:40:30 | source |
8-
| DotRegexServlet.java:56:19:56:41 | getRequestURI(...) : String | DotRegexServlet.java:58:21:58:26 | source |
9-
| DotRegexServlet.java:74:19:74:39 | getPathInfo(...) : String | DotRegexServlet.java:76:56:76:61 | source |
10-
| DotRegexServlet.java:92:19:92:39 | getPathInfo(...) : String | DotRegexServlet.java:95:25:95:30 | source |
2+
| DotRegexFilter.java:16:30:16:46 | PROTECTED_PATTERN : String | DotRegexFilter.java:31:31:31:47 | PROTECTED_PATTERN |
3+
| DotRegexFilter.java:16:50:16:64 | "/protected/.*" : String | DotRegexFilter.java:16:30:16:46 | PROTECTED_PATTERN : String |
4+
| DotRegexFilter.java:29:19:29:43 | getPathInfo(...) : String | DotRegexFilter.java:32:25:32:30 | source |
5+
| DotRegexFilter.java:50:19:50:43 | getPathInfo(...) : String | DotRegexFilter.java:53:25:53:30 | source |
6+
| DotRegexServlet.java:12:30:12:46 | PROTECTED_PATTERN : String | DotRegexServlet.java:21:31:21:47 | PROTECTED_PATTERN |
7+
| DotRegexServlet.java:12:30:12:46 | PROTECTED_PATTERN : String | DotRegexServlet.java:59:36:59:52 | PROTECTED_PATTERN |
8+
| DotRegexServlet.java:12:30:12:46 | PROTECTED_PATTERN : String | DotRegexServlet.java:77:37:77:53 | PROTECTED_PATTERN |
9+
| DotRegexServlet.java:12:30:12:46 | PROTECTED_PATTERN : String | DotRegexServlet.java:114:31:114:47 | PROTECTED_PATTERN |
10+
| DotRegexServlet.java:12:50:12:64 | "/protected/.*" : String | DotRegexServlet.java:12:30:12:46 | PROTECTED_PATTERN : String |
11+
| DotRegexServlet.java:19:19:19:39 | getPathInfo(...) : String | DotRegexServlet.java:22:25:22:30 | source |
12+
| DotRegexServlet.java:38:19:38:39 | getPathInfo(...) : String | DotRegexServlet.java:41:25:41:30 | source |
13+
| DotRegexServlet.java:57:19:57:41 | getRequestURI(...) : String | DotRegexServlet.java:59:21:59:26 | source |
14+
| DotRegexServlet.java:75:19:75:39 | getPathInfo(...) : String | DotRegexServlet.java:77:56:77:61 | source |
15+
| DotRegexServlet.java:93:19:93:39 | getPathInfo(...) : String | DotRegexServlet.java:96:25:96:30 | source |
16+
| DotRegexServlet.java:112:19:112:39 | getPathInfo(...) : String | DotRegexServlet.java:115:25:115:30 | source |
17+
| DotRegexServlet.java:133:19:133:39 | getPathInfo(...) : String | DotRegexServlet.java:136:25:136:30 | source |
1118
nodes
12-
| DotRegexServlet.java:11:30:11:46 | PROTECTED_PATTERN : String | semmle.label | PROTECTED_PATTERN : String |
13-
| DotRegexServlet.java:11:50:11:64 | "/protected/.*" : String | semmle.label | "/protected/.*" : String |
14-
| DotRegexServlet.java:18:19:18:39 | getPathInfo(...) : String | semmle.label | getPathInfo(...) : String |
15-
| DotRegexServlet.java:20:31:20:47 | PROTECTED_PATTERN | semmle.label | PROTECTED_PATTERN |
16-
| DotRegexServlet.java:21:25:21:30 | source | semmle.label | source |
17-
| DotRegexServlet.java:37:19:37:39 | getPathInfo(...) : String | semmle.label | getPathInfo(...) : String |
18-
| DotRegexServlet.java:40:25:40:30 | source | semmle.label | source |
19-
| DotRegexServlet.java:56:19:56:41 | getRequestURI(...) : String | semmle.label | getRequestURI(...) : String |
20-
| DotRegexServlet.java:58:21:58:26 | source | semmle.label | source |
21-
| DotRegexServlet.java:58:36:58:52 | PROTECTED_PATTERN | semmle.label | PROTECTED_PATTERN |
22-
| DotRegexServlet.java:74:19:74:39 | getPathInfo(...) : String | semmle.label | getPathInfo(...) : String |
23-
| DotRegexServlet.java:76:37:76:53 | PROTECTED_PATTERN | semmle.label | PROTECTED_PATTERN |
24-
| DotRegexServlet.java:76:56:76:61 | source | semmle.label | source |
25-
| DotRegexServlet.java:92:19:92:39 | getPathInfo(...) : String | semmle.label | getPathInfo(...) : String |
26-
| DotRegexServlet.java:95:25:95:30 | source | semmle.label | source |
19+
| DotRegexFilter.java:16:30:16:46 | PROTECTED_PATTERN : String | semmle.label | PROTECTED_PATTERN : String |
20+
| DotRegexFilter.java:16:50:16:64 | "/protected/.*" : String | semmle.label | "/protected/.*" : String |
21+
| DotRegexFilter.java:29:19:29:43 | getPathInfo(...) : String | semmle.label | getPathInfo(...) : String |
22+
| DotRegexFilter.java:31:31:31:47 | PROTECTED_PATTERN | semmle.label | PROTECTED_PATTERN |
23+
| DotRegexFilter.java:32:25:32:30 | source | semmle.label | source |
24+
| DotRegexFilter.java:50:19:50:43 | getPathInfo(...) : String | semmle.label | getPathInfo(...) : String |
25+
| DotRegexFilter.java:53:25:53:30 | source | semmle.label | source |
26+
| DotRegexServlet.java:12:30:12:46 | PROTECTED_PATTERN : String | semmle.label | PROTECTED_PATTERN : String |
27+
| DotRegexServlet.java:12:50:12:64 | "/protected/.*" : String | semmle.label | "/protected/.*" : String |
28+
| DotRegexServlet.java:19:19:19:39 | getPathInfo(...) : String | semmle.label | getPathInfo(...) : String |
29+
| DotRegexServlet.java:21:31:21:47 | PROTECTED_PATTERN | semmle.label | PROTECTED_PATTERN |
30+
| DotRegexServlet.java:22:25:22:30 | source | semmle.label | source |
31+
| DotRegexServlet.java:38:19:38:39 | getPathInfo(...) : String | semmle.label | getPathInfo(...) : String |
32+
| DotRegexServlet.java:41:25:41:30 | source | semmle.label | source |
33+
| DotRegexServlet.java:57:19:57:41 | getRequestURI(...) : String | semmle.label | getRequestURI(...) : String |
34+
| DotRegexServlet.java:59:21:59:26 | source | semmle.label | source |
35+
| DotRegexServlet.java:59:36:59:52 | PROTECTED_PATTERN | semmle.label | PROTECTED_PATTERN |
36+
| DotRegexServlet.java:75:19:75:39 | getPathInfo(...) : String | semmle.label | getPathInfo(...) : String |
37+
| DotRegexServlet.java:77:37:77:53 | PROTECTED_PATTERN | semmle.label | PROTECTED_PATTERN |
38+
| DotRegexServlet.java:77:56:77:61 | source | semmle.label | source |
39+
| DotRegexServlet.java:93:19:93:39 | getPathInfo(...) : String | semmle.label | getPathInfo(...) : String |
40+
| DotRegexServlet.java:96:25:96:30 | source | semmle.label | source |
41+
| DotRegexServlet.java:112:19:112:39 | getPathInfo(...) : String | semmle.label | getPathInfo(...) : String |
42+
| DotRegexServlet.java:114:31:114:47 | PROTECTED_PATTERN | semmle.label | PROTECTED_PATTERN |
43+
| DotRegexServlet.java:115:25:115:30 | source | semmle.label | source |
44+
| DotRegexServlet.java:133:19:133:39 | getPathInfo(...) : String | semmle.label | getPathInfo(...) : String |
45+
| DotRegexServlet.java:136:25:136:30 | source | semmle.label | source |
2746
subpaths
2847
#select
29-
| DotRegexServlet.java:21:25:21:30 | source | DotRegexServlet.java:18:19:18:39 | getPathInfo(...) : String | DotRegexServlet.java:21:25:21:30 | source | Potentially authentication bypass due to $@. | DotRegexServlet.java:18:19:18:39 | getPathInfo(...) | user-provided value |
30-
| DotRegexServlet.java:58:21:58:26 | source | DotRegexServlet.java:56:19:56:41 | getRequestURI(...) : String | DotRegexServlet.java:58:21:58:26 | source | Potentially authentication bypass due to $@. | DotRegexServlet.java:56:19:56:41 | getRequestURI(...) | user-provided value |
31-
| DotRegexServlet.java:76:56:76:61 | source | DotRegexServlet.java:74:19:74:39 | getPathInfo(...) : String | DotRegexServlet.java:76:56:76:61 | source | Potentially authentication bypass due to $@. | DotRegexServlet.java:74:19:74:39 | getPathInfo(...) | user-provided value |
48+
| DotRegexFilter.java:32:25:32:30 | source | DotRegexFilter.java:29:19:29:43 | getPathInfo(...) : String | DotRegexFilter.java:32:25:32:30 | source | Potentially authentication bypass due to $@. | DotRegexFilter.java:29:19:29:43 | getPathInfo(...) | user-provided value |
49+
| DotRegexServlet.java:22:25:22:30 | source | DotRegexServlet.java:19:19:19:39 | getPathInfo(...) : String | DotRegexServlet.java:22:25:22:30 | source | Potentially authentication bypass due to $@. | DotRegexServlet.java:19:19:19:39 | getPathInfo(...) | user-provided value |
50+
| DotRegexServlet.java:59:21:59:26 | source | DotRegexServlet.java:57:19:57:41 | getRequestURI(...) : String | DotRegexServlet.java:59:21:59:26 | source | Potentially authentication bypass due to $@. | DotRegexServlet.java:57:19:57:41 | getRequestURI(...) | user-provided value |
51+
| DotRegexServlet.java:77:56:77:61 | source | DotRegexServlet.java:75:19:75:39 | getPathInfo(...) : String | DotRegexServlet.java:77:56:77:61 | source | Potentially authentication bypass due to $@. | DotRegexServlet.java:75:19:75:39 | getPathInfo(...) | user-provided value |
52+
| DotRegexServlet.java:115:25:115:30 | source | DotRegexServlet.java:112:19:112:39 | getPathInfo(...) : String | DotRegexServlet.java:115:25:115:30 | source | Potentially authentication bypass due to $@. | DotRegexServlet.java:112:19:112:39 | getPathInfo(...) | user-provided value |

0 commit comments

Comments
 (0)