Skip to content

Commit 24addd5

Browse files
luchua-bcsmowton
luchua-bc
authored and
smowton
committed
Query to detect XSS with JavaServer Faces (JSF)
1 parent e71173d commit 24addd5

File tree

14 files changed

+2294
-1
lines changed

14 files changed

+2294
-1
lines changed

‎java/ql/lib/semmle/code/java/dataflow/FlowSources.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import semmle.code.java.frameworks.spring.SpringWebClient
2525
import semmle.code.java.frameworks.Guice
2626
import semmle.code.java.frameworks.struts.StrutsActions
2727
import semmle.code.java.frameworks.Thrift
28+
import semmle.code.java.frameworks.javaee.jsf.JSFRenderer
2829
private import semmle.code.java.dataflow.ExternalFlow
2930

3031
/** A data flow source of remote user input. */

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import semmle.code.java.frameworks.Servlets
55
import semmle.code.java.frameworks.android.WebView
66
import semmle.code.java.frameworks.spring.SpringController
77
import semmle.code.java.frameworks.spring.SpringHttp
8+
import semmle.code.java.frameworks.javaee.jsf.JSFRenderer
89
import semmle.code.java.dataflow.DataFlow
910
import semmle.code.java.dataflow.TaintTracking2
1011
import semmle.code.java.dataflow.ExternalFlow
@@ -87,6 +88,12 @@ private class DefaultXssSink extends XssSink {
8788
returnType instanceof RawClass
8889
)
8990
)
91+
or
92+
exists(FacesWriterSourceToWritingMethodFlowConfig writer, MethodAccess ma |
93+
ma.getMethod() instanceof WritingMethod and
94+
writer.hasFlowToExpr(ma.getQualifier()) and
95+
this.asExpr() = ma.getArgument(_)
96+
)
9097
}
9198
}
9299

@@ -158,3 +165,27 @@ predicate isXssVulnerableContentType(string s) {
158165
*/
159166
bindingset[s]
160167
predicate isXssSafeContentType(string s) { not isXssVulnerableContentType(s) }
168+
169+
/** An output stream or writer that writes to a JSF response. */
170+
class FacesWriterSource extends MethodAccess {
171+
FacesWriterSource() {
172+
this.getMethod() instanceof FacesGetResponseWriterMethod
173+
or
174+
this.getMethod() instanceof FacesGetResponseStreamMethod
175+
}
176+
}
177+
178+
/** A configuration that tracks data from a JSF writer to an output method. */
179+
private class FacesWriterSourceToWritingMethodFlowConfig extends TaintTracking2::Configuration {
180+
FacesWriterSourceToWritingMethodFlowConfig() {
181+
this = "XSS::FacesWriterSourceToWritingMethodFlowConfig"
182+
}
183+
184+
override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof FacesWriterSource }
185+
186+
override predicate isSink(DataFlow::Node sink) {
187+
exists(MethodAccess ma |
188+
sink.asExpr() = ma.getQualifier() and ma.getMethod() instanceof WritingMethod
189+
)
190+
}
191+
}

‎java/ql/src/Security/CWE/CWE-079/JsfXSS.java

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
@FacesRenderer(componentFamily = "", rendererType = "")
2+
public class JsfXSS extends Renderer
3+
{
4+
@Override
5+
public void encodeBegin(FacesContext facesContext, UIComponent component) throws IOException
6+
{
7+
super.encodeBegin(facesContext, component);
8+
9+
Map<String, String> requestParameters = facesContext.getExternalContext().getRequestParameterMap();
10+
String windowId = requestParameters.get("window_id");
11+
12+
ResponseWriter writer = facesContext.getResponseWriter();
13+
writer.write("<script type=\"text/javascript\">");
14+
writer.write("(function(){");
15+
{
16+
// BAD: directly output user input.
17+
writer.write("dswh.init('" + windowId + "','"
18+
+ "......" + "',"
19+
+ -1 + ",{");
20+
}
21+
{
22+
// GOOD: use the method `writeText` that performs escaping appropriate for the markup language being rendered.
23+
writer.write("dswh.init('");
24+
writer.writeText(windowId, null);
25+
writer.write("','"
26+
+ "......" + "',"
27+
+ -1 + ",{");
28+
}
29+
writer.write("});");
30+
writer.write("})();");
31+
writer.write("</script>");
32+
}
33+
}

‎java/ql/src/Security/CWE/CWE-079/XSS.qhelp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,15 @@ leaving the website vulnerable to cross-site scripting.</p>
2323

2424
<sample src="XSS.java" />
2525

26+
</example>
27+
28+
<example>
29+
30+
<p>The following example shows the page parameter being written directly to a custom JSF renderer
31+
of UI components, which leaves the website vulnerable to cross-site scripting.</p>
32+
33+
<sample src="JsfXSS.java" />
34+
2635
</example>
2736
<references>
2837

‎java/ql/src/semmle/code/java/frameworks/javaee/jsf/JSFRenderer.qll

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
/** Provides classes and predicates for working with JavaServer Faces renderer. */
2+
3+
import java
4+
private import semmle.code.java.dataflow.ExternalFlow
5+
6+
/**
7+
* The JSF class `FacesContext` for processing HTTP requests.
8+
*/
9+
class FacesContext extends RefType {
10+
FacesContext() {
11+
this.hasQualifiedName(["javax.faces.context", "jakarta.faces.context"], "FacesContext")
12+
}
13+
}
14+
15+
/**
16+
* The JSF class `ExternalContext` allowing JavaServer Faces based applications to run in
17+
* either a Servlet or a Portlet environment.
18+
*/
19+
class ExternalContext extends RefType {
20+
ExternalContext() {
21+
this.hasQualifiedName(["javax.faces.context", "jakarta.faces.context"], "ExternalContext")
22+
}
23+
}
24+
25+
/**
26+
* The base class type `UIComponent` for all user interface components in JavaServer Faces.
27+
*/
28+
class FacesUIComponent extends RefType {
29+
FacesUIComponent() {
30+
this.hasQualifiedName(["javax.faces.component", "jakarta.faces.component"], "UIComponent")
31+
}
32+
}
33+
34+
/**
35+
* The JSF class `Renderer` that converts internal representation of `UIComponent` into the output
36+
* stream (or writer) associated with the response we are creating for a particular request.
37+
*/
38+
class FacesRenderer extends RefType {
39+
FacesRenderer() {
40+
this.hasQualifiedName(["javax.faces.render", "jakarta.faces.render"], "Renderer")
41+
}
42+
}
43+
44+
/**
45+
* The JSF class `ResponseWriter` that outputs and producing elements and attributes for markup
46+
* languages like HTML and XML.
47+
*/
48+
class FacesResponseWriter extends RefType {
49+
FacesResponseWriter() {
50+
this.hasQualifiedName(["javax.faces.context", "jakarta.faces.context"], "ResponseWriter")
51+
}
52+
}
53+
54+
/**
55+
* The class `ResponseStream` that produces binary output.
56+
*/
57+
class FacesResponseStream extends RefType {
58+
FacesResponseStream() {
59+
this.hasQualifiedName(["javax.faces.context", "jakarta.faces.context"], "ResponseStream")
60+
}
61+
}
62+
63+
private class ExternalContextSource extends SourceModelCsv {
64+
override predicate row(string row) {
65+
row =
66+
[
67+
"javax.faces.context;ExternalContext;true;getRequestParameterMap;();;ReturnValue;remote",
68+
"javax.faces.context;ExternalContext;true;getRequestParameterNames;();;ReturnValue;remote",
69+
"javax.faces.context;ExternalContext;true;getRequestParameterValuesMap;();;ReturnValue;remote",
70+
"javax.faces.context;ExternalContext;true;getRequestPathInfo;();;ReturnValue;remote",
71+
"javax.faces.context;ExternalContext;true;getResource;(String);;ReturnValue;remote",
72+
"javax.faces.context;ExternalContext;true;getResourceAsStream;(String);;ReturnValue;remote",
73+
"javax.faces.context;ExternalContext;true;getResourcePaths;(String);;ReturnValue;remote",
74+
"javax.faces.context;ExternalContext;true;getRequestCookieMap;();;ReturnValue;remote",
75+
"javax.faces.context;ExternalContext;true;getRequestHeaderMap;();;ReturnValue;remote",
76+
"javax.faces.context;ExternalContext;true;getRequestHeaderValuesMap;();;ReturnValue;remote",
77+
"jakarta.faces.context;ExternalContext;true;getRequestParameterMap;();;ReturnValue;remote",
78+
"jakarta.faces.context;ExternalContext;true;getRequestParameterNames;();;ReturnValue;remote",
79+
"jakarta.faces.context;ExternalContext;true;getRequestParameterValuesMap;();;ReturnValue;remote",
80+
"jakarta.faces.context;ExternalContext;true;getRequestPathInfo;();;ReturnValue;remote",
81+
"jakarta.faces.context;ExternalContext;true;getResource;(String);;ReturnValue;remote",
82+
"jakarta.faces.context;ExternalContext;true;getResourceAsStream;(String);;ReturnValue;remote",
83+
"jakarta.faces.context;ExternalContext;true;getResourcePaths;(String);;ReturnValue;remote",
84+
"jakarta.faces.context;ExternalContext;true;getRequestCookieMap;();;ReturnValue;remote",
85+
"jakarta.faces.context;ExternalContext;true;getRequestHeaderMap;();;ReturnValue;remote",
86+
"jakarta.faces.context;ExternalContext;true;getRequestHeaderValuesMap;();;ReturnValue;remote"
87+
]
88+
}
89+
}
90+
91+
/**
92+
* The method `getResponseWriter()` declared in JSF `ExternalContext`.
93+
*/
94+
class FacesGetResponseWriterMethod extends Method {
95+
FacesGetResponseWriterMethod() {
96+
getDeclaringType() instanceof FacesContext and
97+
hasName("getResponseWriter") and
98+
getNumberOfParameters() = 0
99+
}
100+
}
101+
102+
/**
103+
* The method `getResponseStream()` declared in JSF `ExternalContext`.
104+
*/
105+
class FacesGetResponseStreamMethod extends Method {
106+
FacesGetResponseStreamMethod() {
107+
getDeclaringType() instanceof FacesContext and
108+
hasName("getResponseStream") and
109+
getNumberOfParameters() = 0
110+
}
111+
}
112+
113+
private class ExternalContextXssSink extends SinkModelCsv {
114+
override predicate row(string row) {
115+
row =
116+
[
117+
"javax.faces.context;ResponseWriter;true;write;;;Argument[0];xss",
118+
"javax.faces.context;ResponseStream;true;write;;;Argument[0];xss",
119+
"jakarta.faces.context;ResponseWriter;true;write;;;Argument[0];xss",
120+
"jakarta.faces.context;ResponseStream;true;write;;;Argument[0];xss"
121+
]
122+
}
123+
}

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

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
import java.io.IOException;
2+
import java.util.Map;
3+
4+
import javax.faces.component.UIComponent;
5+
import javax.faces.context.FacesContext;
6+
import javax.faces.context.ResponseWriter;
7+
import javax.faces.render.FacesRenderer;
8+
import javax.faces.render.Renderer;
9+
10+
@FacesRenderer(componentFamily = "", rendererType = "")
11+
public class JsfXSS extends Renderer
12+
{
13+
@Override
14+
// BAD: directly output user input.
15+
public void encodeBegin(FacesContext facesContext, UIComponent component) throws IOException
16+
{
17+
super.encodeBegin(facesContext, component);
18+
19+
Map<String, String> requestParameters = facesContext.getExternalContext().getRequestParameterMap();
20+
String windowId = requestParameters.get("window_id");
21+
22+
ResponseWriter writer = facesContext.getResponseWriter();
23+
writer.write("<script type=\"text/javascript\">");
24+
writer.write("(function(){");
25+
writer.write("dswh.init('" + windowId + "','" // $xss
26+
+ "......" + "',"
27+
+ -1 + ",{");
28+
writer.write("});");
29+
writer.write("})();");
30+
writer.write("</script>");
31+
}
32+
33+
// GOOD: use the method `writeText` that performs escaping appropriate for the markup language being rendered.
34+
public void encodeBegin2(FacesContext facesContext, UIComponent component) throws IOException
35+
{
36+
super.encodeBegin(facesContext, component);
37+
38+
Map<String, String> requestParameters = facesContext.getExternalContext().getRequestParameterMap();
39+
String windowId = requestParameters.get("window_id");
40+
41+
ResponseWriter writer = facesContext.getResponseWriter();
42+
writer.write("<script type=\"text/javascript\">");
43+
writer.write("(function(){");
44+
writer.write("dswh.init('");
45+
writer.writeText(windowId, null);
46+
writer.write("','"
47+
+ "......" + "',"
48+
+ -1 + ",{");
49+
writer.write("});");
50+
writer.write("})();");
51+
writer.write("</script>");
52+
}
53+
}

‎java/ql/test/query-tests/security/CWE-079/semmle/tests/options

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:${testdir}/../../../../../stubs/javax-ws-rs-api-2.1.1/:${testdir}/../../../../../stubs/springframework-5.3.8
1+
//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../../stubs/servlet-api-2.4:${testdir}/../../../../../stubs/javax-ws-rs-api-2.1.1/:${testdir}/../../../../../stubs/springframework-5.3.8:${testdir}/../../../../../stubs/javax-faces-2.3/

‎java/ql/test/stubs/javax-faces-2.3/javax/faces/component/UIComponent.java

Lines changed: 89 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)