Skip to content

Commit 374859e

Browse files
authored
Merge pull request github#6156 from smowton/smowton/feature/jax-rs-content-type-sensitivity
Jax RS XSS Tests
2 parents a0e768b + dd70f2c commit 374859e

File tree

9 files changed

+465
-57
lines changed

9 files changed

+465
-57
lines changed

java/ql/test/library-tests/frameworks/JaxWs/JaxRs.ql

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,16 @@ class JaxRsTest extends InlineExpectationsTest {
2424
resourceMethod.getLocation() = location and
2525
element = resourceMethod.toString() and
2626
if exists(resourceMethod.getProducesAnnotation())
27-
then value = resourceMethod.getProducesAnnotation().getADeclaredContentType()
28-
else value = ""
27+
then
28+
value = resourceMethod.getProducesAnnotation().getADeclaredContentType() and
29+
value != ""
30+
else
31+
// Filter out empty strings that stem from using stubs.
32+
// If we built the test against the real JAR then the field
33+
// access against e.g. MediaType.APPLICATION_JSON wouldn't
34+
// be a CompileTimeConstantExpr at all, whereas in the stubs
35+
// it is and is defined empty.
36+
value = ""
2937
)
3038
or
3139
tag = "RootResourceClass" and
@@ -135,7 +143,13 @@ class JaxRsTest extends InlineExpectationsTest {
135143
exists(JaxRSProducesAnnotation producesAnnotation |
136144
producesAnnotation.getLocation() = location and
137145
element = producesAnnotation.toString() and
138-
value = producesAnnotation.getADeclaredContentType()
146+
value = producesAnnotation.getADeclaredContentType() and
147+
value != ""
148+
// Filter out empty strings that stem from using stubs.
149+
// If we built the test against the real JAR then the field
150+
// access against e.g. MediaType.APPLICATION_JSON wouldn't
151+
// be a CompileTimeConstantExpr at all, whereas in the stubs
152+
// it is and is defined empty.
139153
)
140154
or
141155
tag = "ConsumesAnnotation" and
Lines changed: 247 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,247 @@
1+
import javax.ws.rs.GET;
2+
import javax.ws.rs.POST;
3+
import javax.ws.rs.Path;
4+
import javax.ws.rs.Produces;
5+
import javax.ws.rs.core.MediaType;
6+
import javax.ws.rs.core.Response;
7+
import javax.ws.rs.core.Variant;
8+
9+
import java.util.Locale;
10+
11+
@Path("")
12+
public class JaxXSS {
13+
14+
@GET
15+
public static Response specificContentType(boolean safeContentType, boolean chainDirectly, boolean contentTypeFirst, String userControlled) {
16+
17+
Response.ResponseBuilder builder = Response.ok();
18+
19+
if(!safeContentType) {
20+
if(chainDirectly) {
21+
if(contentTypeFirst)
22+
return builder.type(MediaType.TEXT_HTML).entity(userControlled).build(); // $xss
23+
else
24+
return builder.entity(userControlled).type(MediaType.TEXT_HTML).build(); // $xss
25+
}
26+
else {
27+
if(contentTypeFirst) {
28+
Response.ResponseBuilder builder2 = builder.type(MediaType.TEXT_HTML);
29+
return builder2.entity(userControlled).build(); // $xss
30+
}
31+
else {
32+
Response.ResponseBuilder builder2 = builder.entity(userControlled);
33+
return builder2.type(MediaType.TEXT_HTML).build(); // $xss
34+
}
35+
}
36+
}
37+
else {
38+
if(chainDirectly) {
39+
if(contentTypeFirst)
40+
return builder.type(MediaType.APPLICATION_JSON).entity(userControlled).build(); // $SPURIOUS: xss
41+
else
42+
return builder.entity(userControlled).type(MediaType.APPLICATION_JSON).build(); // $SPURIOUS: xss
43+
}
44+
else {
45+
if(contentTypeFirst) {
46+
Response.ResponseBuilder builder2 = builder.type(MediaType.APPLICATION_JSON);
47+
return builder2.entity(userControlled).build(); // $SPURIOUS: xss
48+
}
49+
else {
50+
Response.ResponseBuilder builder2 = builder.entity(userControlled);
51+
return builder2.type(MediaType.APPLICATION_JSON).build(); // $SPURIOUS: xss
52+
}
53+
}
54+
}
55+
56+
}
57+
58+
@GET
59+
public static Response specificContentTypeSetterMethods(int route, boolean safeContentType, String userControlled) {
60+
61+
// Test the remarkably many routes to setting a content-type in Jax-RS, besides the ResponseBuilder.entity method used above:
62+
63+
if(safeContentType) {
64+
if(route == 0) {
65+
// via ok, as a string literal:
66+
return Response.ok(userControlled, "application/json").build(); // $SPURIOUS: xss
67+
}
68+
else if(route == 1) {
69+
// via ok, as a string constant:
70+
return Response.ok(userControlled, MediaType.APPLICATION_JSON).build(); // $SPURIOUS: xss
71+
}
72+
else if(route == 2) {
73+
// via ok, as a MediaType constant:
74+
return Response.ok(userControlled, MediaType.APPLICATION_JSON_TYPE).build(); // $SPURIOUS: xss
75+
}
76+
else if(route == 3) {
77+
// via ok, as a Variant, via constructor:
78+
return Response.ok(userControlled, new Variant(MediaType.APPLICATION_JSON_TYPE, "language", "encoding")).build(); // $SPURIOUS: xss
79+
}
80+
else if(route == 4) {
81+
// via ok, as a Variant, via static method:
82+
return Response.ok(userControlled, Variant.mediaTypes(MediaType.APPLICATION_JSON_TYPE).build().get(0)).build(); // $SPURIOUS: xss
83+
}
84+
else if(route == 5) {
85+
// via ok, as a Variant, via instance method:
86+
return Response.ok(userControlled, Variant.languages(Locale.UK).mediaTypes(MediaType.APPLICATION_JSON_TYPE).build().get(0)).build(); // $SPURIOUS: xss
87+
}
88+
else if(route == 6) {
89+
// via builder variant, before entity:
90+
return Response.ok().variant(new Variant(MediaType.APPLICATION_JSON_TYPE, "language", "encoding")).entity(userControlled).build(); // $SPURIOUS: xss
91+
}
92+
else if(route == 7) {
93+
// via builder variant, after entity:
94+
return Response.ok().entity(userControlled).variant(new Variant(MediaType.APPLICATION_JSON_TYPE, "language", "encoding")).build(); // $SPURIOUS: xss
95+
}
96+
else if(route == 8) {
97+
// provide entity via ok, then content-type via builder:
98+
return Response.ok(userControlled).type(MediaType.APPLICATION_JSON_TYPE).build(); // $SPURIOUS: xss
99+
}
100+
}
101+
else {
102+
if(route == 0) {
103+
// via ok, as a string literal:
104+
return Response.ok("text/html").entity(userControlled).build(); // $xss
105+
}
106+
else if(route == 1) {
107+
// via ok, as a string constant:
108+
return Response.ok(MediaType.TEXT_HTML).entity(userControlled).build(); // $xss
109+
}
110+
else if(route == 2) {
111+
// via ok, as a MediaType constant:
112+
return Response.ok(MediaType.TEXT_HTML_TYPE).entity(userControlled).build(); // $xss
113+
}
114+
else if(route == 3) {
115+
// via ok, as a Variant, via constructor:
116+
return Response.ok(new Variant(MediaType.TEXT_HTML_TYPE, "language", "encoding")).entity(userControlled).build(); // $xss
117+
}
118+
else if(route == 4) {
119+
// via ok, as a Variant, via static method:
120+
return Response.ok(Variant.mediaTypes(MediaType.TEXT_HTML_TYPE).build()).entity(userControlled).build(); // $xss
121+
}
122+
else if(route == 5) {
123+
// via ok, as a Variant, via instance method:
124+
return Response.ok(Variant.languages(Locale.UK).mediaTypes(MediaType.TEXT_HTML_TYPE).build()).entity(userControlled).build(); // $xss
125+
}
126+
else if(route == 6) {
127+
// via builder variant, before entity:
128+
return Response.ok().variant(new Variant(MediaType.TEXT_HTML_TYPE, "language", "encoding")).entity(userControlled).build(); // $xss
129+
}
130+
else if(route == 7) {
131+
// via builder variant, after entity:
132+
return Response.ok().entity(userControlled).variant(new Variant(MediaType.TEXT_HTML_TYPE, "language", "encoding")).build(); // $xss
133+
}
134+
else if(route == 8) {
135+
// provide entity via ok, then content-type via builder:
136+
return Response.ok(userControlled).type(MediaType.TEXT_HTML_TYPE).build(); // $xss
137+
}
138+
}
139+
140+
return null;
141+
142+
}
143+
144+
@GET @Produces(MediaType.APPLICATION_JSON)
145+
public static Response methodContentTypeSafe(String userControlled) {
146+
return Response.ok(userControlled).build();
147+
}
148+
149+
@POST @Produces(MediaType.APPLICATION_JSON)
150+
public static Response methodContentTypeSafePost(String userControlled) {
151+
return Response.ok(userControlled).build();
152+
}
153+
154+
@GET @Produces("application/json")
155+
public static Response methodContentTypeSafeStringLiteral(String userControlled) {
156+
return Response.ok(userControlled).build();
157+
}
158+
159+
@GET @Produces(MediaType.TEXT_HTML)
160+
public static Response methodContentTypeUnsafe(String userControlled) {
161+
return Response.ok(userControlled).build(); // $MISSING: xss
162+
}
163+
164+
@POST @Produces(MediaType.TEXT_HTML)
165+
public static Response methodContentTypeUnsafePost(String userControlled) {
166+
return Response.ok(userControlled).build(); // $MISSING: xss
167+
}
168+
169+
@GET @Produces("text/html")
170+
public static Response methodContentTypeUnsafeStringLiteral(String userControlled) {
171+
return Response.ok(userControlled).build(); // $MISSING: xss
172+
}
173+
174+
@GET @Produces({MediaType.TEXT_HTML, MediaType.APPLICATION_JSON})
175+
public static Response methodContentTypeMaybeSafe(String userControlled) {
176+
return Response.ok(userControlled).build(); // $MISSING: xss
177+
}
178+
179+
@GET @Produces(MediaType.APPLICATION_JSON)
180+
public static Response methodContentTypeSafeOverriddenWithUnsafe(String userControlled) {
181+
return Response.ok().type(MediaType.TEXT_HTML).entity(userControlled).build(); // $MISSING: xss
182+
}
183+
184+
@GET @Produces(MediaType.TEXT_HTML)
185+
public static Response methodContentTypeUnsafeOverriddenWithSafe(String userControlled) {
186+
return Response.ok().type(MediaType.APPLICATION_JSON).entity(userControlled).build();
187+
}
188+
189+
@Path("/abc")
190+
@Produces({"application/json"})
191+
public static class ClassContentTypeSafe {
192+
@GET
193+
public Response test(String userControlled) {
194+
return Response.ok(userControlled).build();
195+
}
196+
197+
@GET
198+
public String testDirectReturn(String userControlled) {
199+
return userControlled;
200+
}
201+
202+
@GET @Produces({"text/html"})
203+
public Response overridesWithUnsafe(String userControlled) {
204+
return Response.ok(userControlled).build(); // $MISSING: xss
205+
}
206+
207+
@GET
208+
public Response overridesWithUnsafe2(String userControlled) {
209+
return Response.ok().type(MediaType.TEXT_HTML).entity(userControlled).build(); // $MISSING: xss
210+
}
211+
}
212+
213+
@Path("/abc")
214+
@Produces({"text/html"})
215+
public static class ClassContentTypeUnsafe {
216+
@GET
217+
public Response test(String userControlled) {
218+
return Response.ok(userControlled).build(); // $MISSING: xss
219+
}
220+
221+
@GET
222+
public String testDirectReturn(String userControlled) {
223+
return userControlled; // $MISSING: xss
224+
}
225+
226+
@GET @Produces({"application/json"})
227+
public Response overridesWithSafe(String userControlled) {
228+
return Response.ok(userControlled).build();
229+
}
230+
231+
@GET
232+
public Response overridesWithSafe2(String userControlled) {
233+
return Response.ok().type(MediaType.APPLICATION_JSON).entity(userControlled).build();
234+
}
235+
}
236+
237+
@GET
238+
public static Response entityWithNoMediaType(String userControlled) {
239+
return Response.ok(userControlled).build(); // $xss
240+
}
241+
242+
@GET
243+
public static String stringWithNoMediaType(String userControlled) {
244+
return userControlled; // $xss
245+
}
246+
247+
}
Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +0,0 @@
1-
edges
2-
| XSS.java:23:21:23:48 | getParameter(...) : String | XSS.java:23:5:23:70 | ... + ... |
3-
| XSS.java:38:67:38:87 | getPathInfo(...) : String | XSS.java:38:30:38:87 | ... + ... |
4-
| XSS.java:41:36:41:56 | getPathInfo(...) : String | XSS.java:41:36:41:67 | getBytes(...) |
5-
nodes
6-
| XSS.java:23:5:23:70 | ... + ... | semmle.label | ... + ... |
7-
| XSS.java:23:21:23:48 | getParameter(...) : String | semmle.label | getParameter(...) : String |
8-
| XSS.java:38:30:38:87 | ... + ... | semmle.label | ... + ... |
9-
| XSS.java:38:67:38:87 | getPathInfo(...) : String | semmle.label | getPathInfo(...) : String |
10-
| XSS.java:41:36:41:56 | getPathInfo(...) : String | semmle.label | getPathInfo(...) : String |
11-
| XSS.java:41:36:41:67 | getBytes(...) | semmle.label | getBytes(...) |
12-
#select
13-
| XSS.java:23:5:23:70 | ... + ... | XSS.java:23:21:23:48 | getParameter(...) : String | XSS.java:23:5:23:70 | ... + ... | Cross-site scripting vulnerability due to $@. | XSS.java:23:21:23:48 | getParameter(...) | user-provided value |
14-
| XSS.java:38:30:38:87 | ... + ... | XSS.java:38:67:38:87 | getPathInfo(...) : String | XSS.java:38:30:38:87 | ... + ... | Cross-site scripting vulnerability due to $@. | XSS.java:38:67:38:87 | getPathInfo(...) | user-provided value |
15-
| XSS.java:41:36:41:67 | getBytes(...) | XSS.java:41:36:41:56 | getPathInfo(...) : String | XSS.java:41:36:41:67 | getBytes(...) | Cross-site scripting vulnerability due to $@. | XSS.java:41:36:41:56 | getPathInfo(...) | user-provided value |

java/ql/test/query-tests/security/CWE-079/semmle/tests/XSS.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response)
2020
throws ServletException, IOException {
2121
// BAD: a request parameter is written directly to the Servlet response stream
2222
response.getWriter().print(
23-
"The page \"" + request.getParameter("page") + "\" was not found.");
23+
"The page \"" + request.getParameter("page") + "\" was not found."); // $xss
2424

2525
// GOOD: servlet API encodes the error message HTML for the HTML context
2626
response.sendError(HttpServletResponse.SC_NOT_FOUND,
@@ -35,10 +35,10 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response)
3535
"The page \"" + capitalizeName(request.getParameter("page")) + "\" was not found.");
3636

3737
// BAD: outputting the path of the resource
38-
response.getWriter().print("The path section of the URL was " + request.getPathInfo());
38+
response.getWriter().print("The path section of the URL was " + request.getPathInfo()); // $xss
3939

4040
// BAD: typical XSS, this time written to an OutputStream instead of a Writer
41-
response.getOutputStream().write(request.getPathInfo().getBytes());
41+
response.getOutputStream().write(request.getPathInfo().getBytes()); // $xss
4242
}
4343

4444

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
import java
2+
import semmle.code.java.dataflow.FlowSources
3+
import semmle.code.java.security.XSS
4+
import TestUtilities.InlineExpectationsTest
5+
6+
class XSSConfig extends TaintTracking::Configuration {
7+
XSSConfig() { this = "XSSConfig" }
8+
9+
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
10+
11+
override predicate isSink(DataFlow::Node sink) { sink instanceof XssSink }
12+
13+
override predicate isSanitizer(DataFlow::Node node) { node instanceof XssSanitizer }
14+
15+
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
16+
any(XssAdditionalTaintStep s).step(node1, node2)
17+
}
18+
}
19+
20+
class XssTest extends InlineExpectationsTest {
21+
XssTest() { this = "XssTest" }
22+
23+
override string getARelevantTag() { result = ["xss"] }
24+
25+
override predicate hasActualResult(Location location, string element, string tag, string value) {
26+
tag = "xss" and
27+
exists(DataFlow::Node src, DataFlow::Node sink, XSSConfig conf | conf.hasFlow(src, sink) |
28+
sink.getLocation() = location and
29+
element = sink.toString() and
30+
value = ""
31+
)
32+
}
33+
}

java/ql/test/query-tests/security/CWE-079/semmle/tests/XSS.qlref

Lines changed: 0 additions & 1 deletion
This file was deleted.
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../../stubs/servlet-api-2.4
1+
//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../../stubs/servlet-api-2.4:${testdir}/../../../../../stubs/javax-ws-rs-api-2.1.1/

0 commit comments

Comments
 (0)