Merge pull request #255 from github/reflected-xss

rb/reflected-xss query

ql/lib/codeql/ruby/controlflow/CfgNodes.qll

@@ -355,6 +355,13 @@ module ExprNodes {
     final override VariableReadAccess getExpr() { result = ExprCfgNode.super.getExpr() }
   }
 
+  /** A control-flow node that wraps a `InstanceVariableWriteAccess` AST expression. */
+  class InstanceVariableWriteAccessCfgNode extends ExprCfgNode {
+    override InstanceVariableWriteAccess e;
+
+    final override InstanceVariableWriteAccess getExpr() { result = ExprCfgNode.super.getExpr() }
+  }
+
   /** A control-flow node that wraps a `StringInterpolationComponent` AST expression. */
   class StringInterpolationComponentCfgNode extends StmtSequenceCfgNode {
     StringInterpolationComponentCfgNode() { this.getNode() instanceof StringInterpolationComponent }

ql/lib/codeql/ruby/dataflow/internal/DataFlowDispatch.qll

@@ -314,3 +314,16 @@ predicate mayBenefitFromCallContext(DataFlowCall call, Callable c) { none() }
  * restricted to those `call`s for which a context might make a difference.
  */
 DataFlowCallable viableImplInCallContext(DataFlowCall call, DataFlowCall ctx) { none() }
+
+/**
+ * Holds if `e` is an `ExprNode` that may be returned by a call to `c`.
+ */
+predicate exprNodeReturnedFrom(DataFlow::ExprNode e, DataFlowCallable c) {
+  exists(ReturnNode r |
+    r.getEnclosingCallable() = c and
+    (
+      r.(ExplicitReturnNode).getReturningNode().getReturnedValueNode() = e.asExpr() or
+      r.(ExprReturnNode) = e
+    )
+  )
+}

ql/lib/codeql/ruby/frameworks/ActionController.qll

@@ -134,6 +134,13 @@ private class ActionControllerHtmlSafeCall extends HtmlSafeCall {
   }
 }
 
+// A call to `html_escape` from within a controller.
+private class ActionControllerHtmlEscapeCall extends HtmlEscapeCall {
+  ActionControllerHtmlEscapeCall() {
+    this.getEnclosingModule() instanceof ActionControllerControllerClass
+  }
+}
+
 /**
  * A call to the `redirect_to` method, used in an action to redirect to a
  * specific URL/path or to a different action in this controller.

ql/lib/codeql/ruby/frameworks/ActionView.qll

@@ -20,11 +20,34 @@ abstract class HtmlSafeCall extends MethodCall {
   HtmlSafeCall() { this.getMethodName() = "html_safe" }
 }
 
-// A call to `html_safe` from within a template or view component.
+// A call to `html_safe` from within a template.
 private class ActionViewHtmlSafeCall extends HtmlSafeCall {
   ActionViewHtmlSafeCall() { inActionViewContext(this) }
 }
 
+/**
+ * A call to a method named "html_escape", "html_escape_once", or "h".
+ */
+abstract class HtmlEscapeCall extends MethodCall {
+  // "h" is aliased to "html_escape" in ActiveSupport
+  HtmlEscapeCall() { this.getMethodName() = ["html_escape", "html_escape_once", "h"] }
+}
+
+class RailsHtmlEscaping extends Escaping::Range, DataFlow::CallNode {
+  RailsHtmlEscaping() { this.asExpr().getExpr() instanceof HtmlEscapeCall }
+
+  override DataFlow::Node getAnInput() { result = this.getArgument(0) }
+
+  override DataFlow::Node getOutput() { result = this }
+
+  override string getKind() { result = Escaping::getHtmlKind() }
+}
+
+// A call to `html_escape` from within a template.
+private class ActionViewHtmlEscapeCall extends HtmlEscapeCall {
+  ActionViewHtmlEscapeCall() { inActionViewContext(this) }
+}
+
 // A call in a context where some commonly used `ActionView` methods are available.
 private class ActionViewContextCall extends MethodCall {
   ActionViewContextCall() {
@@ -40,7 +63,7 @@ class RawCall extends ActionViewContextCall {
   RawCall() { this.getMethodName() = "raw" }
 }
 
-// A call to the `params` method within the context of a template or view component.
+// A call to the `params` method within the context of a template.
 private class ActionViewParamsCall extends ActionViewContextCall, ParamsCall { }
 
 /**
@@ -100,7 +123,7 @@ abstract class RenderCall extends MethodCall {
   // TODO: implicit renders in controller actions
 }
 
-// A call to the `render` method within the context of a template or view component.
+// A call to the `render` method within the context of a template.
 private class ActionViewRenderCall extends RenderCall, ActionViewContextCall { }
 
 /**
@@ -110,7 +133,7 @@ abstract class RenderToCall extends MethodCall {
   RenderToCall() { this.getMethodName() = ["render_to_body", "render_to_string"] }
 }
 
-// A call to `render_to` from within a template or view component.
+// A call to `render_to` from within a template.
 private class ActionViewRenderToCall extends ActionViewContextCall, RenderToCall { }
 
 /**
@@ -122,7 +145,12 @@ private class ActionViewRenderToCall extends ActionViewContextCall, RenderToCall
 class LinkToCall extends ActionViewContextCall {
   LinkToCall() { this.getMethodName() = "link_to" }
 
-  // TODO: the path can also be specified through other optional arguments
-  Expr getPathArgument() { result = this.getArgument(1) }
+  Expr getPathArgument() {
+    // When `link_to` is called with a block, it uses the first argument as the
+    // path, and otherwise the second argument.
+    exists(this.getBlock()) and result = this.getArgument(0)
+    or
+    not exists(this.getBlock()) and result = this.getArgument(1)
+  }
 }
 // TODO: model flow in/out of template files properly,

ql/lib/codeql/ruby/security/ReflectedXSSCustomizations.qll

@@ -0,0 +1,200 @@
+private import ruby
+private import codeql.ruby.DataFlow
+private import codeql.ruby.CFG
+private import codeql.ruby.Concepts
+private import codeql.ruby.Frameworks
+private import codeql.ruby.frameworks.ActionController
+private import codeql.ruby.frameworks.ActionView
+private import codeql.ruby.dataflow.RemoteFlowSources
+private import codeql.ruby.dataflow.BarrierGuards
+import codeql.ruby.dataflow.internal.DataFlowDispatch
+private import codeql.ruby.typetracking.TypeTracker
+
+/**
+ * Provides default sources, sinks and sanitizers for detecting
+ * "reflected server-side cross-site scripting"
+ * vulnerabilities, as well as extension points for adding your own.
+ */
+module ReflectedXSS {
+  /**
+   * A data flow source for "reflected server-side cross-site scripting" vulnerabilities.
+   */
+  abstract class Source extends DataFlow::Node { }
+
+  /**
+   * A data flow sink for "reflected server-side cross-site scripting" vulnerabilities.
+   */
+  abstract class Sink extends DataFlow::Node { }
+
+  /**
+   * A sanitizer for "reflected server-side cross-site scripting" vulnerabilities.
+   */
+  abstract class Sanitizer extends DataFlow::Node { }
+
+  /**
+   * A sanitizer guard for "reflected server-side cross-site scripting" vulnerabilities.
+   */
+  abstract class SanitizerGuard extends DataFlow::BarrierGuard { }
+
+  /**
+   * A source of remote user input, considered as a flow source.
+   */
+  class RemoteFlowSourceAsSource extends Source, RemoteFlowSource { }
+
+  private class ErbOutputMethodCallArgumentNode extends DataFlow::Node {
+    private MethodCall call;
+
+    ErbOutputMethodCallArgumentNode() {
+      exists(ErbOutputDirective d |
+        call = d.getTerminalStmt() and
+        this.asExpr().getExpr() = call.getAnArgument()
+      )
+    }
+
+    MethodCall getCall() { result = call }
+  }
+
+  /**
+   * An `html_safe` call marking the output as not requiring HTML escaping,
+   * considered as a flow sink.
+   */
+  class HtmlSafeCallAsSink extends Sink {
+    HtmlSafeCallAsSink() {
+      exists(HtmlSafeCall c, ErbOutputDirective d |
+        this.asExpr().getExpr() = c.getReceiver() and
+        c = d.getTerminalStmt()
+      )
+    }
+  }
+
+  /**
+   * An argument to a call to the `raw` method, considered as a flow sink.
+   */
+  class RawCallArgumentAsSink extends Sink, ErbOutputMethodCallArgumentNode {
+    RawCallArgumentAsSink() { this.getCall() instanceof RawCall }
+  }
+
+  /**
+   * A argument to a call to the `link_to` method, which does not expect
+   * unsanitized user-input, considered as a flow sink.
+   */
+  class LinkToCallArgumentAsSink extends Sink, ErbOutputMethodCallArgumentNode {
+    LinkToCallArgumentAsSink() {
+      this.asExpr().getExpr() = this.getCall().(LinkToCall).getPathArgument()
+    }
+  }
+
+  /**
+   * An HTML escaping, considered as a sanitizer.
+   */
+  class HtmlEscapingAsSanitizer extends Sanitizer {
+    HtmlEscapingAsSanitizer() { this = any(HtmlEscaping esc).getOutput() }
+  }
+
+  /**
+   * A comparison with a constant string, considered as a sanitizer-guard.
+   */
+  class StringConstCompareAsSanitizerGuard extends SanitizerGuard, StringConstCompare { }
+
+  /**
+   * An inclusion check against an array of constant strings, considered as a sanitizer-guard.
+   */
+  class StringConstArrayInclusionCallAsSanitizerGuard extends SanitizerGuard,
+    StringConstArrayInclusionCall { }
+
+  /**
+   * A `VariableWriteAccessCfgNode` that is not succeeded (locally) by another
+   * write to that variable.
+   */
+  private class FinalInstanceVarWrite extends CfgNodes::ExprNodes::InstanceVariableWriteAccessCfgNode {
+    private InstanceVariable var;
+
+    FinalInstanceVarWrite() {
+      var = this.getExpr().getVariable() and
+      not exists(CfgNodes::ExprNodes::InstanceVariableWriteAccessCfgNode succWrite |
+        succWrite.getExpr().getVariable() = var
+      |
+        succWrite = this.getASuccessor+()
+      )
+    }
+
+    InstanceVariable getVariable() { result = var }
+
+    AssignExpr getAnAssignExpr() { result.getLeftOperand() = this.getExpr() }
+  }
+
+  /**
+   * An additional step that is taint-preserving in the context of reflected XSS.
+   */
+  predicate isAdditionalXSSTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
+    // node1 is a `locals` argument to a render call...
+    exists(RenderCall call, Pair kvPair, string hashKey |
+      call.getLocals().getAKeyValuePair() = kvPair and
+      kvPair.getValue() = node1.asExpr().getExpr() and
+      kvPair.getKey().(StringlikeLiteral).getValueText() = hashKey and
+      // `node2` appears in the rendered template file
+      call.getTemplateFile() = node2.getLocation().getFile() and
+      (
+        // node2 is an element reference against `local_assigns`
+        exists(
+          CfgNodes::ExprNodes::ElementReferenceCfgNode refNode, DataFlow::Node argNode,
+          CfgNodes::ExprNodes::StringlikeLiteralCfgNode strNode
+        |
+          refNode = node2.asExpr() and
+          argNode.asExpr() = refNode.getArgument(0) and
+          refNode.getReceiver().getExpr().(MethodCall).getMethodName() = "local_assigns" and
+          argNode.getALocalSource() = DataFlow::exprNode(strNode) and
+          strNode.getExpr().getValueText() = hashKey
+        )
+        or
+        // ...node2 is a "method call" to a "method" with `hashKey` as its name
+        // TODO: This may be a variable read in reality that we interpret as a method call
+        exists(MethodCall varAcc |
+          varAcc = node2.asExpr().(CfgNodes::ExprNodes::MethodCallCfgNode).getExpr() and
+          varAcc.getMethodName() = hashKey
+        )
+      )
+    )
+    or
+    // instance variables in the controller
+    exists(
+      ActionControllerActionMethod action, VariableReadAccess viewVarRead, AssignExpr ae,
+      FinalInstanceVarWrite controllerVarWrite
+    |
+      viewVarRead = node2.asExpr().(CfgNodes::ExprNodes::VariableReadAccessCfgNode).getExpr() and
+      action.getDefaultTemplateFile() = viewVarRead.getLocation().getFile() and
+      // match read to write on variable name
+      viewVarRead.getVariable().getName() = controllerVarWrite.getVariable().getName() and
+      // propagate taint from assignment RHS expr to variable read access in view
+      ae = controllerVarWrite.getAnAssignExpr() and
+      node1.asExpr().getExpr() = ae.getRightOperand() and
+      ae.getParent+() = action
+    )
+    or
+    // flow from template into controller helper method
+    exists(
+      ErbFile template, ActionControllerHelperMethod helperMethod,
+      CfgNodes::ExprNodes::MethodCallCfgNode helperMethodCall, int argIdx
+    |
+      template = node1.getLocation().getFile() and
+      helperMethod.getName() = helperMethodCall.getExpr().getMethodName() and
+      helperMethod.getControllerClass() = getAssociatedControllerClass(template) and
+      helperMethodCall.getArgument(argIdx) = node1.asExpr() and
+      helperMethod.getParameter(argIdx) = node2.asExpr().getExpr()
+    )
+    or
+    // flow out of controller helper method into template
+    exists(
+      ErbFile template, ActionControllerHelperMethod helperMethod,
+      CfgNodes::ExprNodes::MethodCallCfgNode helperMethodCall
+    |
+      template = node2.getLocation().getFile() and
+      helperMethod.getName() = helperMethodCall.getExpr().getMethodName() and
+      helperMethod.getControllerClass() = getAssociatedControllerClass(template) and
+      // `node1` is an expr node that may be returned by the helper method
+      exprNodeReturnedFrom(node1, helperMethod) and
+      // `node2` is a call to the helper method
+      node2.asExpr() = helperMethodCall
+    )
+  }
+}

ql/lib/codeql/ruby/security/ReflectedXSSQuery.qll

@@ -0,0 +1,39 @@
+/**
+ * Provides a taint-tracking configuration for detecting "reflected server-side cross-site scripting" vulnerabilities.
+ *
+ * Note, for performance reasons: only import this file if
+ * `ReflectedXSS::Configuration` is needed, otherwise
+ * `ReflectedXSSCustomizations` should be imported instead.
+ */
+
+private import ruby
+import codeql.ruby.DataFlow
+import codeql.ruby.TaintTracking
+
+/**
+ * Provides a taint-tracking configuration for detecting "reflected server-side cross-site scripting" vulnerabilities.
+ */
+module ReflectedXSS {
+  import ReflectedXSSCustomizations::ReflectedXSS
+
+  /**
+   * A taint-tracking configuration for detecting "reflected server-side cross-site scripting" vulnerabilities.
+   */
+  class Configuration extends TaintTracking::Configuration {
+    Configuration() { this = "ReflectedXSS" }
+
+    override predicate isSource(DataFlow::Node source) { source instanceof Source }
+
+    override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
+
+    override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer }
+
+    override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
+      guard instanceof SanitizerGuard
+    }
+
+    override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
+      isAdditionalXSSTaintStep(node1, node2)
+    }
+  }
+}

ql/src/queries/security/cwe-079/ReflectedXSS.qhelp

@@ -0,0 +1,55 @@
+<!DOCTYPE qhelp PUBLIC
+ "-//Semmle//qhelp//EN"
+ "qhelp.dtd">
+<qhelp>
+
+  <overview>
+    <p>
+      Directly writing user input (for example, an HTTP request parameter) to a webpage,
+      without properly sanitizing the input first, allows for a cross-site scripting
+      vulnerability.
+    </p>
+  </overview>
+
+  <recommendation>
+    <p>
+      To guard against cross-site scripting, escape user input before writing it
+      to the page. Some frameworks, such as Rails, perform this escaping
+      implicitly and by default.
+    </p>
+
+    <p>
+      Take care when using methods such as <code>html_safe</code> or
+      <code>raw</code>. They can be used to emit a string without escaping
+      it, and should only be used when the string has already been manually
+      escaped (for example, with the Rails <code>html_escape</code> method), or when
+      the content is otherwise guaranteed to be safe (such as a hard-coded string).
+    </p>
+  </recommendation>
+
+  <example>
+    <p>
+      The following example is safe because the
+      <code>params[:user_name]</code> content within the output tags will be
+      HTML-escaped automatically before being emitted.
+    </p>
+    <sample src="examples/safe.html.erb" />
+
+    <p>
+      However, the following example is unsafe because user-controlled input is
+      emitted without escaping, since it is marked as <code>html_safe</code>.
+    </p>
+    <sample src="examples/reflective_xss.html.erb" />
+  </example>
+
+  <references>
+    <li>
+      OWASP:
+      <a href="https://cheatsheetseries.owasp.org/cheatsheets/Ruby_on_Rails_Cheat_Sheet.html#cross-site-scripting-xss">XSS
+        Ruby on Rails Cheatsheet</a>.
+    </li>
+    <li>
+      Wikipedia: <a href="http://en.wikipedia.org/wiki/Cross-site_scripting">Cross-site scripting</a>.
+    </li>
+  </references>
+</qhelp>

ql/src/queries/security/cwe-079/ReflectedXSS.ql

@@ -0,0 +1,23 @@
+/**
+ * @name Reflected server-side cross-site scripting
+ * @description Writing user input directly to a web page
+ *              allows for a cross-site scripting vulnerability.
+ * @kind path-problem
+ * @problem.severity error
+ * @sub-severity high
+ * @precision high
+ * @id rb/reflected-xss
+ * @tags security
+ *       external/cwe/cwe-079
+ *       external/cwe/cwe-116
+ */
+
+import ruby
+import codeql.ruby.security.ReflectedXSSQuery
+import codeql.ruby.DataFlow
+import DataFlow::PathGraph
+
+from ReflectedXSS::Configuration config, DataFlow::PathNode source, DataFlow::PathNode sink
+where config.hasFlowPath(source, sink)
+select sink.getNode(), source, sink, "Cross-site scripting vulnerability due to $@.",
+  source.getNode(), "a user-provided value"

ql/src/queries/security/cwe-079/examples/reflective_xss.html.erb

@@ -0,0 +1 @@
+<p>Hello <%= params[:user_name].html_safe %>!</p>

ql/src/queries/security/cwe-079/examples/safe.html.erb

@@ -0,0 +1 @@
+<p>Hello <%= params[:user_name] %>!</p>

ql/test/query-tests/security/cwe-079/ReflectedXSS.expected

@@ -0,0 +1,60 @@
+edges
+| app/controllers/foo/bars_controller.rb:9:12:9:17 | call to params :  | app/controllers/foo/bars_controller.rb:9:12:9:29 | ...[...] :  |
+| app/controllers/foo/bars_controller.rb:9:12:9:29 | ...[...] :  | app/views/foo/bars/show.html.erb:47:5:47:13 | call to user_name |
+| app/controllers/foo/bars_controller.rb:13:5:13:37 | ... = ... :  | app/views/foo/bars/show.html.erb:51:5:51:18 | call to user_name_memo |
+| app/controllers/foo/bars_controller.rb:13:20:13:25 | call to params :  | app/controllers/foo/bars_controller.rb:13:5:13:37 | ... = ... :  |
+| app/controllers/foo/bars_controller.rb:17:21:17:26 | call to params :  | app/controllers/foo/bars_controller.rb:17:21:17:36 | ...[...] :  |
+| app/controllers/foo/bars_controller.rb:17:21:17:36 | ...[...] :  | app/views/foo/bars/show.html.erb:2:18:2:30 | @user_website |
+| app/controllers/foo/bars_controller.rb:18:10:18:15 | call to params :  | app/controllers/foo/bars_controller.rb:19:22:19:23 | dt :  |
+| app/controllers/foo/bars_controller.rb:18:10:18:15 | call to params :  | app/controllers/foo/bars_controller.rb:23:53:23:54 | dt :  |
+| app/controllers/foo/bars_controller.rb:19:22:19:23 | dt :  | app/views/foo/bars/show.html.erb:41:3:41:16 | @instance_text |
+| app/controllers/foo/bars_controller.rb:23:53:23:54 | dt :  | app/views/foo/bars/show.html.erb:5:9:5:20 | call to display_text |
+| app/controllers/foo/bars_controller.rb:23:53:23:54 | dt :  | app/views/foo/bars/show.html.erb:8:9:8:36 | ...[...] |
+| app/controllers/foo/bars_controller.rb:23:53:23:54 | dt :  | app/views/foo/bars/show.html.erb:12:9:12:26 | ...[...] |
+| app/controllers/foo/bars_controller.rb:23:53:23:54 | dt :  | app/views/foo/bars/show.html.erb:36:3:36:14 | call to display_text |
+| app/controllers/foo/bars_controller.rb:23:53:23:54 | dt :  | app/views/foo/bars/show.html.erb:44:76:44:87 | call to display_text :  |
+| app/views/foo/bars/show.html.erb:44:64:44:87 | ... + ... :  | app/views/foo/bars/_widget.html.erb:5:9:5:20 | call to display_text |
+| app/views/foo/bars/show.html.erb:44:64:44:87 | ... + ... :  | app/views/foo/bars/_widget.html.erb:8:9:8:36 | ...[...] |
+| app/views/foo/bars/show.html.erb:44:76:44:87 | call to display_text :  | app/views/foo/bars/show.html.erb:44:64:44:87 | ... + ... :  |
+| app/views/foo/bars/show.html.erb:54:29:54:34 | call to params :  | app/views/foo/bars/show.html.erb:54:29:54:44 | ...[...] |
+| app/views/foo/bars/show.html.erb:57:13:57:18 | call to params :  | app/views/foo/bars/show.html.erb:57:13:57:28 | ...[...] |
+nodes
+| app/controllers/foo/bars_controller.rb:9:12:9:17 | call to params :  | semmle.label | call to params :  |
+| app/controllers/foo/bars_controller.rb:9:12:9:29 | ...[...] :  | semmle.label | ...[...] :  |
+| app/controllers/foo/bars_controller.rb:13:5:13:37 | ... = ... :  | semmle.label | ... = ... :  |
+| app/controllers/foo/bars_controller.rb:13:20:13:25 | call to params :  | semmle.label | call to params :  |
+| app/controllers/foo/bars_controller.rb:17:21:17:26 | call to params :  | semmle.label | call to params :  |
+| app/controllers/foo/bars_controller.rb:17:21:17:36 | ...[...] :  | semmle.label | ...[...] :  |
+| app/controllers/foo/bars_controller.rb:18:10:18:15 | call to params :  | semmle.label | call to params :  |
+| app/controllers/foo/bars_controller.rb:19:22:19:23 | dt :  | semmle.label | dt :  |
+| app/controllers/foo/bars_controller.rb:23:53:23:54 | dt :  | semmle.label | dt :  |
+| app/views/foo/bars/_widget.html.erb:5:9:5:20 | call to display_text | semmle.label | call to display_text |
+| app/views/foo/bars/_widget.html.erb:8:9:8:36 | ...[...] | semmle.label | ...[...] |
+| app/views/foo/bars/show.html.erb:2:18:2:30 | @user_website | semmle.label | @user_website |
+| app/views/foo/bars/show.html.erb:5:9:5:20 | call to display_text | semmle.label | call to display_text |
+| app/views/foo/bars/show.html.erb:8:9:8:36 | ...[...] | semmle.label | ...[...] |
+| app/views/foo/bars/show.html.erb:12:9:12:26 | ...[...] | semmle.label | ...[...] |
+| app/views/foo/bars/show.html.erb:36:3:36:14 | call to display_text | semmle.label | call to display_text |
+| app/views/foo/bars/show.html.erb:41:3:41:16 | @instance_text | semmle.label | @instance_text |
+| app/views/foo/bars/show.html.erb:44:64:44:87 | ... + ... :  | semmle.label | ... + ... :  |
+| app/views/foo/bars/show.html.erb:44:76:44:87 | call to display_text :  | semmle.label | call to display_text :  |
+| app/views/foo/bars/show.html.erb:47:5:47:13 | call to user_name | semmle.label | call to user_name |
+| app/views/foo/bars/show.html.erb:51:5:51:18 | call to user_name_memo | semmle.label | call to user_name_memo |
+| app/views/foo/bars/show.html.erb:54:29:54:34 | call to params :  | semmle.label | call to params :  |
+| app/views/foo/bars/show.html.erb:54:29:54:44 | ...[...] | semmle.label | ...[...] |
+| app/views/foo/bars/show.html.erb:57:13:57:18 | call to params :  | semmle.label | call to params :  |
+| app/views/foo/bars/show.html.erb:57:13:57:28 | ...[...] | semmle.label | ...[...] |
+subpaths
+#select
+| app/views/foo/bars/_widget.html.erb:5:9:5:20 | call to display_text | app/controllers/foo/bars_controller.rb:18:10:18:15 | call to params :  | app/views/foo/bars/_widget.html.erb:5:9:5:20 | call to display_text | Cross-site scripting vulnerability due to $@. | app/controllers/foo/bars_controller.rb:18:10:18:15 | call to params | a user-provided value |
+| app/views/foo/bars/_widget.html.erb:8:9:8:36 | ...[...] | app/controllers/foo/bars_controller.rb:18:10:18:15 | call to params :  | app/views/foo/bars/_widget.html.erb:8:9:8:36 | ...[...] | Cross-site scripting vulnerability due to $@. | app/controllers/foo/bars_controller.rb:18:10:18:15 | call to params | a user-provided value |
+| app/views/foo/bars/show.html.erb:2:18:2:30 | @user_website | app/controllers/foo/bars_controller.rb:17:21:17:26 | call to params :  | app/views/foo/bars/show.html.erb:2:18:2:30 | @user_website | Cross-site scripting vulnerability due to $@. | app/controllers/foo/bars_controller.rb:17:21:17:26 | call to params | a user-provided value |
+| app/views/foo/bars/show.html.erb:5:9:5:20 | call to display_text | app/controllers/foo/bars_controller.rb:18:10:18:15 | call to params :  | app/views/foo/bars/show.html.erb:5:9:5:20 | call to display_text | Cross-site scripting vulnerability due to $@. | app/controllers/foo/bars_controller.rb:18:10:18:15 | call to params | a user-provided value |
+| app/views/foo/bars/show.html.erb:8:9:8:36 | ...[...] | app/controllers/foo/bars_controller.rb:18:10:18:15 | call to params :  | app/views/foo/bars/show.html.erb:8:9:8:36 | ...[...] | Cross-site scripting vulnerability due to $@. | app/controllers/foo/bars_controller.rb:18:10:18:15 | call to params | a user-provided value |
+| app/views/foo/bars/show.html.erb:12:9:12:26 | ...[...] | app/controllers/foo/bars_controller.rb:18:10:18:15 | call to params :  | app/views/foo/bars/show.html.erb:12:9:12:26 | ...[...] | Cross-site scripting vulnerability due to $@. | app/controllers/foo/bars_controller.rb:18:10:18:15 | call to params | a user-provided value |
+| app/views/foo/bars/show.html.erb:36:3:36:14 | call to display_text | app/controllers/foo/bars_controller.rb:18:10:18:15 | call to params :  | app/views/foo/bars/show.html.erb:36:3:36:14 | call to display_text | Cross-site scripting vulnerability due to $@. | app/controllers/foo/bars_controller.rb:18:10:18:15 | call to params | a user-provided value |
+| app/views/foo/bars/show.html.erb:41:3:41:16 | @instance_text | app/controllers/foo/bars_controller.rb:18:10:18:15 | call to params :  | app/views/foo/bars/show.html.erb:41:3:41:16 | @instance_text | Cross-site scripting vulnerability due to $@. | app/controllers/foo/bars_controller.rb:18:10:18:15 | call to params | a user-provided value |
+| app/views/foo/bars/show.html.erb:47:5:47:13 | call to user_name | app/controllers/foo/bars_controller.rb:9:12:9:17 | call to params :  | app/views/foo/bars/show.html.erb:47:5:47:13 | call to user_name | Cross-site scripting vulnerability due to $@. | app/controllers/foo/bars_controller.rb:9:12:9:17 | call to params | a user-provided value |
+| app/views/foo/bars/show.html.erb:51:5:51:18 | call to user_name_memo | app/controllers/foo/bars_controller.rb:13:20:13:25 | call to params :  | app/views/foo/bars/show.html.erb:51:5:51:18 | call to user_name_memo | Cross-site scripting vulnerability due to $@. | app/controllers/foo/bars_controller.rb:13:20:13:25 | call to params | a user-provided value |
+| app/views/foo/bars/show.html.erb:54:29:54:44 | ...[...] | app/views/foo/bars/show.html.erb:54:29:54:34 | call to params :  | app/views/foo/bars/show.html.erb:54:29:54:44 | ...[...] | Cross-site scripting vulnerability due to $@. | app/views/foo/bars/show.html.erb:54:29:54:34 | call to params | a user-provided value |
+| app/views/foo/bars/show.html.erb:57:13:57:28 | ...[...] | app/views/foo/bars/show.html.erb:57:13:57:18 | call to params :  | app/views/foo/bars/show.html.erb:57:13:57:28 | ...[...] | Cross-site scripting vulnerability due to $@. | app/views/foo/bars/show.html.erb:57:13:57:18 | call to params | a user-provided value |

ql/test/query-tests/security/cwe-079/ReflectedXSS.qlref

@@ -0,0 +1 @@
+queries/security/cwe-079/ReflectedXSS.ql

ql/test/query-tests/security/cwe-079/app/controllers/foo/bars_controller.rb

@@ -0,0 +1,25 @@
+class BarsController < ApplicationController
+  helper_method :user_name, :user_name_memo
+
+  def index
+    render template: "foo/bars/index"
+  end
+
+  def user_name
+    return params[:user_name]
+  end
+
+  def user_name_memo
+    @user_name ||= params[:user_name]
+  end
+
+  def show
+    @user_website = params[:website]
+    dt = params[:text]
+    @instance_text = dt
+    @safe_foo = params[:text]
+    @safe_foo = "safe_foo"
+    @html_escaped = ERB::Util.html_escape(params[:text])
+    render "foo/bars/show", locals: { display_text: dt, safe_text: "hello" }
+  end
+end

ql/test/query-tests/security/cwe-079/app/views/foo/bars/_widget.html.erb

@@ -0,0 +1,11 @@
+<%# BAD: A local rendered raw as an instance variable %>
+<%= raw @display_text %>
+
+<%# BAD: A local rendered raw as a local variable %>
+<%= raw display_text %>
+
+<%# BAD: A local rendered raw via the local_assigns hash %>
+<%= raw local_assigns[:display_text] %>
+
+<%# GOOD: A local rendered with default escaping via the local_assigns hash %>
+<%= local_assigns[:display_text] %>

ql/test/query-tests/security/cwe-079/app/views/foo/bars/show.html.erb

@@ -0,0 +1,71 @@
+<%# BAD: An instance variable rendered without escaping %>
+<a href="<%= raw @user_website %>">website</a>
+
+<%# BAD: A local rendered raw as a local variable %>
+<%= raw display_text %>
+
+<%# BAD: A local rendered raw via the local_assigns hash %>
+<%= raw local_assigns[:display_text] %>
+
+<% key = :display_text %>
+<%# BAD: A local rendered raw via the locals_assigns hash %>
+<%= raw local_assigns[key] %>
+
+<ul>
+<% for key in [:display_text, :safe_text] do %>
+  <%# BAD: A local rendered raw via the locals hash %>
+  <%# TODO: we miss that `key` can take `:display_text` as a value here %>
+  <li><%= raw local_assigns[key] %></li>
+<% end %>
+</ul>
+
+<%# GOOD: A local rendered with default escaping via the local_assigns hash %>
+<%= local_assigns[display_text] %>
+
+<%# GOOD: default escaping of rendered text %>
+<%=
+  full_text = prefix + local_assigns[:display_text]
+  full_text
+%>
+
+<%# GOOD: default escaping of rendered text (from instance var) %>
+<%= @instance_text %>
+
+<%# BAD: html_safe marks string as not requiring HTML escaping %>
+<%=
+  display_text.html_safe
+%>
+
+<%# BAD: html_safe marks string as not requiring HTML escaping %>
+<%=
+  @instance_text.html_safe
+%>
+
+<%= render partial: 'foo/bars/widget', locals: { display_text: "widget_" + display_text } %>
+
+<%# BAD: user_name is a helper method that returns unsanitized user-input %>
+<%= user_name.html_safe %>
+
+<%# BAD: user_name_memo is a helper method that returns unsanitized user-input %>
+<%# TODO: we miss this because the return value from user_name_memo is not properly linked to this call %>
+<%= user_name_memo.html_safe %>
+
+<%# BAD: unsanitized user-input should not be passed to link_to as the URL %>
+<%= link_to "user website", params[:website] %>
+
+<%# BAD: unsanitized user-input should not be passed to link_to as the URL %>
+<%= link_to params[:website], class: "user-link" do %>
+  user website
+<% end %>
+
+<%# GOOD: @safe_foo is a hardcoded string here at runtime %>
+<%= @safe_foo.html_safe %>
+
+<%# GOOD: @html_escaped is manually escaped in the controller %>
+<%= @html_escaped.html_safe %>
+
+<%# GOOD: @html_escaped is manually escaped in the controller %>
+<%=
+  html_escaped_in_template = h params[:text]
+  html_escaped_in_template.html_safe
+%>