Ruby: Recognise raw Erb output as XSS sink

Author: hmac

ruby/ql/lib/codeql/ruby/ast/Erb.qll

@@ -252,14 +252,32 @@ class ErbGraphqlDirective extends ErbDirective {
 class ErbOutputDirective extends ErbDirective {
   private Erb::OutputDirective g;
 
-  ErbOutputDirective() { this = TOutputDirective(g) }
+  ErbOutputDirective() { this = TOutputDirective(g) or this = TRawOutputDirective(g) }
 
   override ErbCode getToken() { toGenerated(result) = g.getChild() }
 
+  /**
+   * Holds if this is a raw Erb output directive.
+   * ```erb
+   * <%== foo %>
+   * ```
+   */
+  predicate isRaw() { this = TRawOutputDirective(g) }
+
   final override string toString() {
-    result = "<%=" + this.getToken().toString() + "%>"
+    this.isRaw() and
+    (
+      result = "<%==" + this.getToken().toString() + "%>"
+      or
+      not exists(this.getToken()) and result = "<%==%>"
+    )
     or
-    not exists(this.getToken()) and result = "<%=%>"
+    not this.isRaw() and
+    (
+      result = "<%=" + this.getToken().toString() + "%>"
+      or
+      not exists(this.getToken()) and result = "<%=%>"
+    )
   }
 
   final override string getAPrimaryQlClass() { result = "ErbOutputDirective" }

ruby/ql/lib/codeql/ruby/ast/internal/Erb.qll

@@ -9,12 +9,17 @@ private module Cached {
     TCommentDirective(Erb::CommentDirective g) or
     TDirective(Erb::Directive g) or
     TGraphqlDirective(Erb::GraphqlDirective g) or
-    TOutputDirective(Erb::OutputDirective g) or
+    TOutputDirective(Erb::OutputDirective g) { getOpeningTag(g) != "<%==" } or
+    TRawOutputDirective(Erb::OutputDirective g) { getOpeningTag(g) = "<%==" } or
     TTemplate(Erb::Template g) or
     TToken(Erb::Token g) or
     TComment(Erb::Comment g) or
     TCode(Erb::Code g)
 
+  private string getOpeningTag(Erb::OutputDirective g) {
+    erb_tokeninfo(any(Erb::Token t | erb_ast_node_info(t, g, 0, _)), 0, result)
+  }
+
   /**
    * Gets the underlying TreeSitter entity for a given erb AST node.
    */
@@ -24,6 +29,7 @@ private module Cached {
     n = TDirective(result) or
     n = TGraphqlDirective(result) or
     n = TOutputDirective(result) or
+    n = TRawOutputDirective(result) or
     n = TTemplate(result) or
     n = TToken(result) or
     n = TComment(result) or
@@ -38,6 +44,7 @@ import Cached
 
 TAstNode fromGenerated(Erb::AstNode n) { n = toGenerated(result) }
 
-class TDirectiveNode = TCommentDirective or TDirective or TGraphqlDirective or TOutputDirective;
+class TDirectiveNode =
+  TCommentDirective or TDirective or TGraphqlDirective or TOutputDirective or TRawOutputDirective;
 
 class TTokenNode = TToken or TComment or TCode;

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

@@ -48,6 +48,18 @@ private module Shared {
     MethodCall getCall() { result = call }
   }
 
+  /**
+   * A value interpolated using a raw erb output directive, which does not perform HTML escaping.
+   * ```erb
+   * <%== sink %>
+   * ```
+   */
+  class ErbRawOutputDirective extends Sink {
+    ErbRawOutputDirective() {
+      exists(ErbOutputDirective d | d.isRaw() | this.asExpr().getExpr() = d.getTerminalStmt())
+    }
+  }
+
   /**
    * An `html_safe` call marking the output as not requiring HTML escaping,
    * considered as a flow sink.

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

@@ -21,6 +21,7 @@ edges
 | app/controllers/foo/bars_controller.rb:26:53:26:54 | dt | app/views/foo/bars/show.html.erb:17:15:17:27 | call to local_assigns [element :display_text] | provenance |  |
 | app/controllers/foo/bars_controller.rb:26:53:26:54 | dt | app/views/foo/bars/show.html.erb:35:3:35:14 | call to display_text | provenance |  |
 | app/controllers/foo/bars_controller.rb:26:53:26:54 | dt | app/views/foo/bars/show.html.erb:43:76:43:87 | call to display_text | provenance |  |
+| app/controllers/foo/bars_controller.rb:26:53:26:54 | dt | app/views/foo/bars/show.html.erb:82:6:82:17 | call to display_text | provenance |  |
 | app/controllers/foo/bars_controller.rb:30:5:30:7 | str | app/controllers/foo/bars_controller.rb:31:5:31:7 | str | provenance |  |
 | app/controllers/foo/bars_controller.rb:30:11:30:16 | call to params | app/controllers/foo/bars_controller.rb:30:11:30:28 | ...[...] | provenance |  |
 | app/controllers/foo/bars_controller.rb:30:11:30:28 | ...[...] | app/controllers/foo/bars_controller.rb:30:5:30:7 | str | provenance |  |
@@ -90,6 +91,7 @@ nodes
 | app/views/foo/bars/show.html.erb:73:19:73:34 | ...[...] | semmle.label | ...[...] |
 | app/views/foo/bars/show.html.erb:76:28:76:33 | call to params | semmle.label | call to params |
 | app/views/foo/bars/show.html.erb:76:28:76:39 | ...[...] | semmle.label | ...[...] |
+| app/views/foo/bars/show.html.erb:82:6:82:17 | call to display_text | semmle.label | call to display_text |
 subpaths
 #select
 | app/controllers/foo/bars_controller.rb:24:39:24:59 | ... = ... | app/controllers/foo/bars_controller.rb:24:39:24:44 | call to params | app/controllers/foo/bars_controller.rb:24:39:24:59 | ... = ... | Cross-site scripting vulnerability due to a $@. | app/controllers/foo/bars_controller.rb:24:39:24:44 | call to params | user-provided value |
@@ -109,3 +111,4 @@ subpaths
 | app/views/foo/bars/show.html.erb:56:13:56:28 | ...[...] | app/views/foo/bars/show.html.erb:56:13:56:18 | call to params | app/views/foo/bars/show.html.erb:56:13:56:28 | ...[...] | Cross-site scripting vulnerability due to a $@. | app/views/foo/bars/show.html.erb:56:13:56:18 | call to params | user-provided value |
 | app/views/foo/bars/show.html.erb:73:19:73:34 | ...[...] | app/views/foo/bars/show.html.erb:73:19:73:24 | call to params | app/views/foo/bars/show.html.erb:73:19:73:34 | ...[...] | Cross-site scripting vulnerability due to a $@. | app/views/foo/bars/show.html.erb:73:19:73:24 | call to params | user-provided value |
 | app/views/foo/bars/show.html.erb:76:28:76:39 | ...[...] | app/views/foo/bars/show.html.erb:76:28:76:33 | call to params | app/views/foo/bars/show.html.erb:76:28:76:39 | ...[...] | Cross-site scripting vulnerability due to a $@. | app/views/foo/bars/show.html.erb:76:28:76:33 | call to params | user-provided value |
+| app/views/foo/bars/show.html.erb:82:6:82:17 | call to display_text | app/controllers/foo/bars_controller.rb:18:10:18:15 | call to params | app/views/foo/bars/show.html.erb:82:6:82:17 | call to display_text | Cross-site scripting vulnerability due to a $@. | app/controllers/foo/bars_controller.rb:18:10:18:15 | call to params | user-provided value |

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

@@ -77,3 +77,6 @@
 
 <%# GOOD: input is sanitized %>
 <%= sanitize(params[:comment]).html_safe %>
+
+<%# BAD: A local rendered raw as a local variable %>
+<%== display_text %>