Java: CWE-079 Query to detect XSS with JavaServer Faces (JSF)

java/change-notes/2021-09-14-jsf-support.md

@@ -0,0 +1,2 @@
+lgtm,codescanning
+* Added basic support for untrusted data sources and XSS-vulnerable sinks relating to the JavaServer Faces (JSF) framework.

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

@@ -83,6 +83,7 @@ private module Frameworks {
   private import semmle.code.java.frameworks.apache.Lang
   private import semmle.code.java.frameworks.guava.Guava
   private import semmle.code.java.frameworks.jackson.JacksonSerializability
+  private import semmle.code.java.frameworks.javaee.jsf.JSFRenderer
   private import semmle.code.java.frameworks.JavaxJson
   private import semmle.code.java.frameworks.JaxWS
   private import semmle.code.java.frameworks.JoddJson

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

@@ -25,6 +25,7 @@ import semmle.code.java.frameworks.spring.SpringWebClient
 import semmle.code.java.frameworks.Guice
 import semmle.code.java.frameworks.struts.StrutsActions
 import semmle.code.java.frameworks.Thrift
+import semmle.code.java.frameworks.javaee.jsf.JSFRenderer
 private import semmle.code.java.dataflow.ExternalFlow
 
 /** A data flow source of remote user input. */

java/ql/lib/semmle/code/java/frameworks/javaee/jsf/JSFRenderer.qll

@@ -0,0 +1,63 @@
+/** Provides classes and predicates for working with JavaServer Faces renderer. */
+
+import java
+private import semmle.code.java.dataflow.ExternalFlow
+
+/**
+ * The JSF class `FacesContext` for processing HTTP requests.
+ */
+class FacesContext extends RefType {
+  FacesContext() {
+    this.hasQualifiedName(["javax.faces.context", "jakarta.faces.context"], "FacesContext")
+  }
+}
+
+private class ExternalContextSource extends SourceModelCsv {
+  override predicate row(string row) {
+    row =
+      ["javax.", "jakarta."] +
+        [
+          "faces.context;ExternalContext;true;getRequestParameterMap;();;ReturnValue;remote",
+          "faces.context;ExternalContext;true;getRequestParameterNames;();;ReturnValue;remote",
+          "faces.context;ExternalContext;true;getRequestParameterValuesMap;();;ReturnValue;remote",
+          "faces.context;ExternalContext;true;getRequestPathInfo;();;ReturnValue;remote",
+          "faces.context;ExternalContext;true;getRequestCookieMap;();;ReturnValue;remote",
+          "faces.context;ExternalContext;true;getRequestHeaderMap;();;ReturnValue;remote",
+          "faces.context;ExternalContext;true;getRequestHeaderValuesMap;();;ReturnValue;remote"
+        ]
+  }
+}
+
+/**
+ * The method `getResponseWriter()` declared in JSF `ExternalContext`.
+ */
+class FacesGetResponseWriterMethod extends Method {
+  FacesGetResponseWriterMethod() {
+    getDeclaringType() instanceof FacesContext and
+    hasName("getResponseWriter") and
+    getNumberOfParameters() = 0
+  }
+}
+
+/**
+ * The method `getResponseStream()` declared in JSF `ExternalContext`.
+ */
+class FacesGetResponseStreamMethod extends Method {
+  FacesGetResponseStreamMethod() {
+    getDeclaringType() instanceof FacesContext and
+    hasName("getResponseStream") and
+    getNumberOfParameters() = 0
+  }
+}
+
+private class ExternalContextXssSink extends SinkModelCsv {
+  override predicate row(string row) {
+    row =
+      [
+        "javax.faces.context;ResponseWriter;true;write;;;Argument[0];xss",
+        "javax.faces.context;ResponseStream;true;write;;;Argument[0];xss",
+        "jakarta.faces.context;ResponseWriter;true;write;;;Argument[0];xss",
+        "jakarta.faces.context;ResponseStream;true;write;;;Argument[0];xss"
+      ]
+  }
+}

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

@@ -5,6 +5,7 @@ import semmle.code.java.frameworks.Servlets
 import semmle.code.java.frameworks.android.WebView
 import semmle.code.java.frameworks.spring.SpringController
 import semmle.code.java.frameworks.spring.SpringHttp
+import semmle.code.java.frameworks.javaee.jsf.JSFRenderer
 import semmle.code.java.dataflow.DataFlow
 import semmle.code.java.dataflow.TaintTracking2
 import semmle.code.java.dataflow.ExternalFlow
@@ -40,7 +41,7 @@ private class DefaultXssSink extends XssSink {
   DefaultXssSink() {
     sinkNode(this, "xss")
     or
-    exists(ServletWriterSourceToWritingMethodFlowConfig writer, MethodAccess ma |
+    exists(XssVulnerableWriterSourceToWritingMethodFlowConfig writer, MethodAccess ma |
       ma.getMethod() instanceof WritingMethod and
       writer.hasFlowToExpr(ma.getQualifier()) and
       this.asExpr() = ma.getArgument(_)
@@ -101,12 +102,14 @@ private class DefaultXSSSanitizer extends XssSanitizer {
 }
 
 /** A configuration that tracks data from a servlet writer to an output method. */
-private class ServletWriterSourceToWritingMethodFlowConfig extends TaintTracking2::Configuration {
-  ServletWriterSourceToWritingMethodFlowConfig() {
-    this = "XSS::ServletWriterSourceToWritingMethodFlowConfig"
+private class XssVulnerableWriterSourceToWritingMethodFlowConfig extends TaintTracking2::Configuration {
+  XssVulnerableWriterSourceToWritingMethodFlowConfig() {
+    this = "XSS::XssVulnerableWriterSourceToWritingMethodFlowConfig"
   }
 
-  override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof ServletWriterSource }
+  override predicate isSource(DataFlow::Node src) {
+    src.asExpr() instanceof XssVulnerableWriterSource
+  }
 
   override predicate isSink(DataFlow::Node sink) {
     exists(MethodAccess ma |
@@ -128,9 +131,9 @@ private class WritingMethod extends Method {
   }
 }
 
-/** An output stream or writer that writes to a servlet response. */
-class ServletWriterSource extends MethodAccess {
-  ServletWriterSource() {
+/** An output stream or writer that writes to a servlet, JSP or JSF response. */
+class XssVulnerableWriterSource extends MethodAccess {
+  XssVulnerableWriterSource() {
     this.getMethod() instanceof ServletResponseGetWriterMethod
     or
     this.getMethod() instanceof ServletResponseGetOutputStreamMethod
@@ -139,9 +142,18 @@ class ServletWriterSource extends MethodAccess {
       m.getDeclaringType().getQualifiedName() = "javax.servlet.jsp.JspContext" and
       m.getName() = "getOut"
     )
+    or
+    this.getMethod() instanceof FacesGetResponseWriterMethod
+    or
+    this.getMethod() instanceof FacesGetResponseStreamMethod
   }
 }
 
+/**
+ * DEPRECATED: Use `XssVulnerableWriterSource` instead.
+ */
+deprecated class ServletWriterSource = XssVulnerableWriterSource;
+
 /**
  * Holds if `s` is an HTTP Content-Type vulnerable to XSS.
  */

java/ql/src/Security/CWE/CWE-209/StackTraceExposure.ql

@@ -36,7 +36,9 @@ class ServletWriterSourceToPrintStackTraceMethodFlowConfig extends TaintTracking
     this = "StackTraceExposure::ServletWriterSourceToPrintStackTraceMethodFlowConfig"
   }
 
-  override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof ServletWriterSource }
+  override predicate isSource(DataFlow::Node src) {
+    src.asExpr() instanceof XssVulnerableWriterSource
+  }
 
   override predicate isSink(DataFlow::Node sink) {
     exists(MethodAccess ma |

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

@@ -0,0 +1,69 @@
+import java.io.IOException;
+import java.util.Map;
+
+import javax.faces.component.UIComponent;
+import javax.faces.context.ExternalContext;
+import javax.faces.context.FacesContext;
+import javax.faces.context.ResponseWriter;
+import javax.faces.render.FacesRenderer;
+import javax.faces.render.Renderer;
+import javax.servlet.http.Cookie;
+
+@FacesRenderer(componentFamily = "", rendererType = "")
+public class JsfXSS extends Renderer
+{
+    @Override
+    // BAD: directly output user input.
+    public void encodeBegin(FacesContext facesContext, UIComponent component) throws IOException
+    {
+        super.encodeBegin(facesContext, component);
+
+        Map<String, String> requestParameters = facesContext.getExternalContext().getRequestParameterMap();
+        String windowId = requestParameters.get("window_id");
+
+        ResponseWriter writer = facesContext.getResponseWriter();
+        writer.write("<script type=\"text/javascript\">");
+        writer.write("(function(){");
+        writer.write("dswh.init('" + windowId + "','" // $xss
+                + "......" + "',"
+                + -1 + ",{");
+        writer.write("});");
+        writer.write("})();");
+        writer.write("</script>");
+    }
+
+    // GOOD: use the method `writeText` that performs escaping appropriate for the markup language being rendered.
+    public void encodeBegin2(FacesContext facesContext, UIComponent component) throws IOException
+    {
+        super.encodeBegin(facesContext, component);
+
+        Map<String, String> requestParameters = facesContext.getExternalContext().getRequestParameterMap();
+        String windowId = requestParameters.get("window_id");
+
+        ResponseWriter writer = facesContext.getResponseWriter();
+        writer.write("<script type=\"text/javascript\">");
+        writer.write("(function(){");
+        writer.write("dswh.init('");
+        writer.writeText(windowId, null);
+        writer.write("','"
+                + "......" + "',"
+                + -1 + ",{");
+        writer.write("});");
+        writer.write("})();");
+        writer.write("</script>");
+    }
+
+    public void testAllSources(FacesContext facesContext) throws IOException
+    {
+        ExternalContext ec = facesContext.getExternalContext();
+        ResponseWriter writer = facesContext.getResponseWriter();
+        writer.write(ec.getRequestParameterMap().keySet().iterator().next()); // $xss
+        writer.write(ec.getRequestParameterNames().next()); // $xss
+        writer.write(ec.getRequestParameterValuesMap().get("someKey")[0]); // $xss
+        writer.write(ec.getRequestParameterValuesMap().keySet().iterator().next()); // $xss
+        writer.write(ec.getRequestPathInfo()); // $xss
+        writer.write(((Cookie)ec.getRequestCookieMap().get("someKey")).getName()); // $xss
+        writer.write(ec.getRequestHeaderMap().get("someKey")); // $xss
+        writer.write(ec.getRequestHeaderValuesMap().get("someKey")[0]); // $xss
+    }
+}

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

@@ -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
+//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/