Skip to content

Commit 099311a

Browse files
author
Paolo Tranquilli
committed
Merge branch 'main' into redsun82/cargo-upgrade
2 parents 5bd30e7 + 74a2495 commit 099311a

File tree

12 files changed

+184
-69
lines changed

12 files changed

+184
-69
lines changed

cpp/ql/lib/semmle/code/cpp/dataflow/ExternalFlow.qll

+5-6
Original file line numberDiff line numberDiff line change
@@ -869,12 +869,11 @@ private predicate elementSpecMatchesSignature(
869869
bindingset[nameWithoutArgs]
870870
pragma[inline_late]
871871
private Class getClassAndNameImpl(Function method, string nameWithoutArgs) {
872-
exists(string memberName | result = method.getClassAndName(memberName) |
873-
nameWithoutArgs = "operator " + method.(ConversionOperator).getDestType()
874-
or
875-
not method instanceof ConversionOperator and
876-
memberName = nameWithoutArgs
877-
)
872+
result = method.getDeclaringType() and
873+
nameWithoutArgs = "operator " + method.(ConversionOperator).getDestType()
874+
or
875+
result = method.getClassAndName(nameWithoutArgs) and
876+
not method instanceof ConversionOperator
878877
}
879878

880879
/**

go/ql/lib/semmle/go/dataflow/internal/DataFlowUtil.qll

+10-2
Original file line numberDiff line numberDiff line change
@@ -379,15 +379,23 @@ module BarrierGuard<guardChecksSig/3 guardChecks> {
379379
)
380380
}
381381

382+
bindingset[inp, c]
383+
pragma[inline_late]
384+
private Node getInputNode(FunctionInput inp, CallNode c) { result = inp.getNode(c) }
385+
386+
bindingset[outp, c]
387+
pragma[inline_late]
388+
private Node getOutputNode(FunctionOutput outp, CallNode c) { result = outp.getNode(c) }
389+
382390
pragma[noinline]
383391
private predicate guardingCall(
384392
Node g, Function f, FunctionInput inp, FunctionOutput outp, DataFlow::Property p, CallNode c,
385393
Node nd, Node resNode
386394
) {
387395
guardingFunction(g, f, inp, outp, p) and
388396
c = f.getACall() and
389-
nd = inp.getNode(c) and
390-
localFlow(pragma[only_bind_out](outp.getNode(c)), resNode)
397+
nd = getInputNode(inp, c) and
398+
localFlow(getOutputNode(outp, c), resNode)
391399
}
392400

393401
private predicate onlyPossibleReturnSatisfyingProperty(

java/ql/lib/semmle/code/java/frameworks/Servlets.qll

+10
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,16 @@ class ResponseSetHeaderMethod extends Method {
315315
}
316316
}
317317

318+
/**
319+
* The method `setContentType` declared in `javax.servlet.http.HttpServletResponse`.
320+
*/
321+
class ResponseSetContentTypeMethod extends Method {
322+
ResponseSetContentTypeMethod() {
323+
this.getDeclaringType() instanceof ServletResponse and
324+
this.hasName("setContentType")
325+
}
326+
}
327+
318328
/**
319329
* A class that has `javax.servlet.Servlet` as an ancestor.
320330
*/

java/ql/lib/semmle/code/java/security/XSS.qll

+24-3
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,25 @@ private class WritingMethod extends Method {
9292
/** An output stream or writer that writes to a servlet, JSP or JSF response. */
9393
class XssVulnerableWriterSource extends MethodCall {
9494
XssVulnerableWriterSource() {
95-
this.getMethod() instanceof ServletResponseGetWriterMethod
96-
or
97-
this.getMethod() instanceof ServletResponseGetOutputStreamMethod
95+
(
96+
this.getMethod() instanceof ServletResponseGetWriterMethod
97+
or
98+
this.getMethod() instanceof ServletResponseGetOutputStreamMethod
99+
) and
100+
not exists(MethodCall mc, Expr contentType |
101+
mc.getMethod() instanceof ResponseSetContentTypeMethod and
102+
contentType = mc.getArgument(0)
103+
or
104+
(
105+
mc.getMethod() instanceof ResponseAddHeaderMethod or
106+
mc.getMethod() instanceof ResponseSetHeaderMethod
107+
) and
108+
mc.getArgument(0).(CompileTimeConstantExpr).getStringValue().toLowerCase() = "content-type" and
109+
contentType = mc.getArgument(1)
110+
|
111+
isXssSafeContentTypeString(contentType.(CompileTimeConstantExpr).getStringValue()) and
112+
DataFlow::localExprFlow(mc.getQualifier(), this.getQualifier())
113+
)
98114
or
99115
exists(Method m | m = this.getMethod() |
100116
m.hasQualifiedName("javax.servlet.jsp", "JspContext", "getOut")
@@ -106,6 +122,11 @@ class XssVulnerableWriterSource extends MethodCall {
106122
}
107123
}
108124

125+
pragma[nomagic]
126+
private predicate isXssSafeContentTypeString(string s) {
127+
s = any(CompileTimeConstantExpr cte).getStringValue() and isXssSafeContentType(s)
128+
}
129+
109130
/**
110131
* A xss vulnerable writer source node.
111132
*/
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: majorAnalysis
3+
---
4+
* Fixed false positive alerts in the java query "Cross-site scripting" (`java/xss`) when `javax.servlet.http.HttpServletResponse` is used with a content type which is not exploitable.

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

+25-25
Original file line numberDiff line numberDiff line change
@@ -19,18 +19,18 @@ public static Response specificContentType(boolean safeContentType, boolean chai
1919
if(!safeContentType) {
2020
if(chainDirectly) {
2121
if(contentTypeFirst)
22-
return builder.type(MediaType.TEXT_HTML).entity(userControlled).build(); // $xss
22+
return builder.type(MediaType.TEXT_HTML).entity(userControlled).build(); // $ xss
2323
else
24-
return builder.entity(userControlled).type(MediaType.TEXT_HTML).build(); // $xss
24+
return builder.entity(userControlled).type(MediaType.TEXT_HTML).build(); // $ xss
2525
}
2626
else {
2727
if(contentTypeFirst) {
2828
Response.ResponseBuilder builder2 = builder.type(MediaType.TEXT_HTML);
29-
return builder2.entity(userControlled).build(); // $xss
29+
return builder2.entity(userControlled).build(); // $ xss
3030
}
3131
else {
3232
Response.ResponseBuilder builder2 = builder.entity(userControlled);
33-
return builder2.type(MediaType.TEXT_HTML).build(); // $xss
33+
return builder2.type(MediaType.TEXT_HTML).build(); // $ xss
3434
}
3535
}
3636
}
@@ -105,39 +105,39 @@ else if(route == 8) {
105105
else {
106106
if(route == 0) {
107107
// via ok, as a string literal:
108-
return Response.ok("text/html").entity(userControlled).build(); // $xss
108+
return Response.ok("text/html").entity(userControlled).build(); // $ xss
109109
}
110110
else if(route == 1) {
111111
// via ok, as a string constant:
112-
return Response.ok(MediaType.TEXT_HTML).entity(userControlled).build(); // $xss
112+
return Response.ok(MediaType.TEXT_HTML).entity(userControlled).build(); // $ xss
113113
}
114114
else if(route == 2) {
115115
// via ok, as a MediaType constant:
116-
return Response.ok(MediaType.TEXT_HTML_TYPE).entity(userControlled).build(); // $xss
116+
return Response.ok(MediaType.TEXT_HTML_TYPE).entity(userControlled).build(); // $ xss
117117
}
118118
else if(route == 3) {
119119
// via ok, as a Variant, via constructor:
120-
return Response.ok(new Variant(MediaType.TEXT_HTML_TYPE, "language", "encoding")).entity(userControlled).build(); // $xss
120+
return Response.ok(new Variant(MediaType.TEXT_HTML_TYPE, "language", "encoding")).entity(userControlled).build(); // $ xss
121121
}
122122
else if(route == 4) {
123123
// via ok, as a Variant, via static method:
124-
return Response.ok(Variant.mediaTypes(MediaType.TEXT_HTML_TYPE).build()).entity(userControlled).build(); // $xss
124+
return Response.ok(Variant.mediaTypes(MediaType.TEXT_HTML_TYPE).build()).entity(userControlled).build(); // $ xss
125125
}
126126
else if(route == 5) {
127127
// via ok, as a Variant, via instance method:
128-
return Response.ok(Variant.languages(Locale.UK).mediaTypes(MediaType.TEXT_HTML_TYPE).build()).entity(userControlled).build(); // $xss
128+
return Response.ok(Variant.languages(Locale.UK).mediaTypes(MediaType.TEXT_HTML_TYPE).build()).entity(userControlled).build(); // $ xss
129129
}
130130
else if(route == 6) {
131131
// via builder variant, before entity:
132-
return Response.ok().variant(new Variant(MediaType.TEXT_HTML_TYPE, "language", "encoding")).entity(userControlled).build(); // $xss
132+
return Response.ok().variant(new Variant(MediaType.TEXT_HTML_TYPE, "language", "encoding")).entity(userControlled).build(); // $ xss
133133
}
134134
else if(route == 7) {
135135
// via builder variant, after entity:
136-
return Response.ok().entity(userControlled).variant(new Variant(MediaType.TEXT_HTML_TYPE, "language", "encoding")).build(); // $xss
136+
return Response.ok().entity(userControlled).variant(new Variant(MediaType.TEXT_HTML_TYPE, "language", "encoding")).build(); // $ xss
137137
}
138138
else if(route == 8) {
139139
// provide entity via ok, then content-type via builder:
140-
return Response.ok(userControlled).type(MediaType.TEXT_HTML_TYPE).build(); // $xss
140+
return Response.ok(userControlled).type(MediaType.TEXT_HTML_TYPE).build(); // $ xss
141141
}
142142
}
143143

@@ -162,27 +162,27 @@ public static Response methodContentTypeSafeStringLiteral(String userControlled)
162162

163163
@GET @Produces(MediaType.TEXT_HTML)
164164
public static Response methodContentTypeUnsafe(String userControlled) {
165-
return Response.ok(userControlled).build(); // $xss
165+
return Response.ok(userControlled).build(); // $ xss
166166
}
167167

168168
@POST @Produces(MediaType.TEXT_HTML)
169169
public static Response methodContentTypeUnsafePost(String userControlled) {
170-
return Response.ok(userControlled).build(); // $xss
170+
return Response.ok(userControlled).build(); // $ xss
171171
}
172172

173173
@GET @Produces("text/html")
174174
public static Response methodContentTypeUnsafeStringLiteral(String userControlled) {
175-
return Response.ok(userControlled).build(); // $xss
175+
return Response.ok(userControlled).build(); // $ xss
176176
}
177177

178178
@GET @Produces({MediaType.TEXT_HTML, MediaType.APPLICATION_JSON})
179179
public static Response methodContentTypeMaybeSafe(String userControlled) {
180-
return Response.ok(userControlled).build(); // $xss
180+
return Response.ok(userControlled).build(); // $ xss
181181
}
182182

183183
@GET @Produces(MediaType.APPLICATION_JSON)
184184
public static Response methodContentTypeSafeOverriddenWithUnsafe(String userControlled) {
185-
return Response.ok().type(MediaType.TEXT_HTML).entity(userControlled).build(); // $xss
185+
return Response.ok().type(MediaType.TEXT_HTML).entity(userControlled).build(); // $ xss
186186
}
187187

188188
@GET @Produces(MediaType.TEXT_HTML)
@@ -205,12 +205,12 @@ public String testDirectReturn(String userControlled) {
205205

206206
@GET @Produces({"text/html"})
207207
public Response overridesWithUnsafe(String userControlled) {
208-
return Response.ok(userControlled).build(); // $xss
208+
return Response.ok(userControlled).build(); // $ xss
209209
}
210210

211211
@GET
212212
public Response overridesWithUnsafe2(String userControlled) {
213-
return Response.ok().type(MediaType.TEXT_HTML).entity(userControlled).build(); // $xss
213+
return Response.ok().type(MediaType.TEXT_HTML).entity(userControlled).build(); // $ xss
214214
}
215215
}
216216

@@ -219,12 +219,12 @@ public Response overridesWithUnsafe2(String userControlled) {
219219
public static class ClassContentTypeUnsafe {
220220
@GET
221221
public Response test(String userControlled) {
222-
return Response.ok(userControlled).build(); // $xss
222+
return Response.ok(userControlled).build(); // $ xss
223223
}
224224

225225
@GET
226226
public String testDirectReturn(String userControlled) {
227-
return userControlled; // $xss
227+
return userControlled; // $ xss
228228
}
229229

230230
@GET @Produces({"application/json"})
@@ -240,12 +240,12 @@ public Response overridesWithSafe2(String userControlled) {
240240

241241
@GET
242242
public static Response entityWithNoMediaType(String userControlled) {
243-
return Response.ok(userControlled).build(); // $xss
243+
return Response.ok(userControlled).build(); // $ xss
244244
}
245245

246246
@GET
247247
public static String stringWithNoMediaType(String userControlled) {
248-
return userControlled; // $xss
248+
return userControlled; // $ xss
249249
}
250250

251-
}
251+
}

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

+9-9
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ public void encodeBegin(FacesContext facesContext, UIComponent component) throws
2626
writer.write("(function(){");
2727
writer.write("dswh.init('" + windowId + "','"
2828
+ "......" + "',"
29-
+ -1 + ",{"); // $xss
29+
+ -1 + ",{"); // $ xss
3030
writer.write("});");
3131
writer.write("})();");
3232
writer.write("</script>");
@@ -57,13 +57,13 @@ public void testAllSources(FacesContext facesContext) throws IOException
5757
{
5858
ExternalContext ec = facesContext.getExternalContext();
5959
ResponseWriter writer = facesContext.getResponseWriter();
60-
writer.write(ec.getRequestParameterMap().keySet().iterator().next()); // $xss
61-
writer.write(ec.getRequestParameterNames().next()); // $xss
62-
writer.write(ec.getRequestParameterValuesMap().get("someKey")[0]); // $xss
63-
writer.write(ec.getRequestParameterValuesMap().keySet().iterator().next()); // $xss
64-
writer.write(ec.getRequestPathInfo()); // $xss
65-
writer.write(((Cookie)ec.getRequestCookieMap().get("someKey")).getName()); // $xss
66-
writer.write(ec.getRequestHeaderMap().get("someKey")); // $xss
67-
writer.write(ec.getRequestHeaderValuesMap().get("someKey")[0]); // $xss
60+
writer.write(ec.getRequestParameterMap().keySet().iterator().next()); // $ xss
61+
writer.write(ec.getRequestParameterNames().next()); // $ xss
62+
writer.write(ec.getRequestParameterValuesMap().get("someKey")[0]); // $ xss
63+
writer.write(ec.getRequestParameterValuesMap().keySet().iterator().next()); // $ xss
64+
writer.write(ec.getRequestPathInfo()); // $ xss
65+
writer.write(((Cookie)ec.getRequestCookieMap().get("someKey")).getName()); // $ xss
66+
writer.write(ec.getRequestHeaderMap().get("someKey")); // $ xss
67+
writer.write(ec.getRequestHeaderValuesMap().get("someKey")[0]); // $ xss
6868
}
6969
}

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
public class SetJavascriptEnabled {
77
public static void configureWebViewUnsafe(WebView view) {
88
WebSettings settings = view.getSettings();
9-
settings.setJavaScriptEnabled(true); // $javascriptEnabled
9+
settings.setJavaScriptEnabled(true); // $ javascriptEnabled
1010
}
1111

1212
public static void configureWebViewSafe(WebView view) {

0 commit comments

Comments
 (0)