Ruby: Restrict XSS header sinks

Not all header writes are relevant to XSS. Restrict these to just
content-type and access-control-allow-origin.

Author: hmac

ruby/ql/lib/codeql/ruby/Concepts.qll

@@ -474,8 +474,8 @@ module Http {
      * extend `HeaderWriteAccess::Range` instead.
      */
     class HeaderWriteAccess extends DataFlow::Node instanceof HeaderWriteAccess::Range {
-      /** Gets the name of the header that is written to. */
-      string getName() { result = super.getName() }
+      /** Gets the (lower case) name of the header that is written to. */
+      string getName() { result = super.getName().toLowerCase() }
 
       /** Gets the value that is written to the header. */
       DataFlow::Node getValue() { result = super.getValue() }

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

@@ -107,7 +107,13 @@ private module Shared {
 
   /** A write to an HTTP response header, considered as a flow sink. */
   class HeaderWriteAsSink extends Sink {
-    HeaderWriteAsSink() { this = any(Http::Server::HeaderWriteAccess a).getValue() }
+    HeaderWriteAsSink() {
+      exists(Http::Server::HeaderWriteAccess a |
+        a.getName() = ["content-type", "access-control-allow-origin"]
+      |
+        this = a.getValue()
+      )
+    }
   }
 
   /**

ruby/ql/test/library-tests/frameworks/ActionController.expected

@@ -371,11 +371,11 @@ controllerTemplateFiles
 | app/controllers/foo/bars_controller.rb:3:1:46:3 | BarsController | app/views/foo/bars/_widget.html.erb:0:0:0:0 | app/views/foo/bars/_widget.html.erb |
 | app/controllers/foo/bars_controller.rb:3:1:46:3 | BarsController | app/views/foo/bars/show.html.erb:0:0:0:0 | app/views/foo/bars/show.html.erb |
 headerWriteAccesses
-| app/controllers/comments_controller.rb:15:5:15:35 | call to []= | Content-Type | app/controllers/comments_controller.rb:15:39:15:49 | ... = ... |
-| app/controllers/comments_controller.rb:16:5:16:46 | call to set_header | Content-Length | app/controllers/comments_controller.rb:16:43:16:45 | 100 |
-| app/controllers/comments_controller.rb:17:5:17:39 | call to []= | X-Custom-Header | app/controllers/comments_controller.rb:17:43:17:46 | ... = ... |
-| app/controllers/comments_controller.rb:18:5:18:39 | call to []= | X-Another-Custom-Header | app/controllers/comments_controller.rb:18:43:18:47 | ... = ... |
-| app/controllers/comments_controller.rb:19:5:19:49 | call to add_header | X-Yet-Another | app/controllers/comments_controller.rb:19:42:19:49 | "indeed" |
+| app/controllers/comments_controller.rb:15:5:15:35 | call to []= | content-type | app/controllers/comments_controller.rb:15:39:15:49 | ... = ... |
+| app/controllers/comments_controller.rb:16:5:16:46 | call to set_header | content-length | app/controllers/comments_controller.rb:16:43:16:45 | 100 |
+| app/controllers/comments_controller.rb:17:5:17:39 | call to []= | x-custom-header | app/controllers/comments_controller.rb:17:43:17:46 | ... = ... |
+| app/controllers/comments_controller.rb:18:5:18:39 | call to []= | x-another-custom-header | app/controllers/comments_controller.rb:18:43:18:47 | ... = ... |
+| app/controllers/comments_controller.rb:19:5:19:49 | call to add_header | x-yet-another | app/controllers/comments_controller.rb:19:42:19:49 | "indeed" |
 | app/controllers/comments_controller.rb:25:5:25:21 | call to location= | location | app/controllers/comments_controller.rb:25:25:25:36 | ... = ... |
 | app/controllers/comments_controller.rb:26:5:26:26 | call to cache_control= | cache-control | app/controllers/comments_controller.rb:26:30:26:36 | ... = ... |
 | app/controllers/comments_controller.rb:27:5:27:27 | call to _cache_control= | cache-control | app/controllers/comments_controller.rb:27:31:27:37 | ... = ... |

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

@@ -10,15 +10,15 @@ edges
 | 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:18:10:18:22 | ...[...] :  |
 | app/controllers/foo/bars_controller.rb:18:10:18:22 | ...[...] :  | app/controllers/foo/bars_controller.rb:19:22:19:23 | dt :  |
-| app/controllers/foo/bars_controller.rb:18:10:18:22 | ...[...] :  | app/controllers/foo/bars_controller.rb:25:53:25:54 | dt :  |
+| app/controllers/foo/bars_controller.rb:18:10:18:22 | ...[...] :  | app/controllers/foo/bars_controller.rb:26:53:26: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:24:39:24:44 | call to params :  | app/controllers/foo/bars_controller.rb:24:39:24:59 | ...[...] :  |
 | app/controllers/foo/bars_controller.rb:24:39:24:59 | ...[...] :  | app/controllers/foo/bars_controller.rb:24:39:24:59 | ... = ... |
-| app/controllers/foo/bars_controller.rb:25:53:25:54 | dt :  | app/views/foo/bars/show.html.erb:5:9:5:20 | call to display_text |
-| app/controllers/foo/bars_controller.rb:25:53:25:54 | dt :  | app/views/foo/bars/show.html.erb:8:9:8:36 | ...[...] |
-| app/controllers/foo/bars_controller.rb:25:53:25:54 | dt :  | app/views/foo/bars/show.html.erb:12:9:12:26 | ...[...] |
-| app/controllers/foo/bars_controller.rb:25:53:25:54 | dt :  | app/views/foo/bars/show.html.erb:36:3:36:14 | call to display_text |
-| app/controllers/foo/bars_controller.rb:25:53:25:54 | dt :  | app/views/foo/bars/show.html.erb:44:76:44:87 | call to display_text :  |
+| app/controllers/foo/bars_controller.rb:26:53:26:54 | dt :  | app/views/foo/bars/show.html.erb:5:9:5:20 | call to display_text |
+| app/controllers/foo/bars_controller.rb:26:53:26:54 | dt :  | app/views/foo/bars/show.html.erb:8:9:8:36 | ...[...] |
+| app/controllers/foo/bars_controller.rb:26:53:26:54 | dt :  | app/views/foo/bars/show.html.erb:12:9:12:26 | ...[...] |
+| app/controllers/foo/bars_controller.rb:26:53:26:54 | dt :  | app/views/foo/bars/show.html.erb:36:3:36:14 | call to display_text |
+| app/controllers/foo/bars_controller.rb:26:53:26: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 | ... + ... :  |
@@ -40,7 +40,7 @@ nodes
 | app/controllers/foo/bars_controller.rb:24:39:24:44 | call to params :  | semmle.label | call to params :  |
 | app/controllers/foo/bars_controller.rb:24:39:24:59 | ... = ... | semmle.label | ... = ... |
 | app/controllers/foo/bars_controller.rb:24:39:24:59 | ...[...] :  | semmle.label | ...[...] :  |
-| app/controllers/foo/bars_controller.rb:25:53:25:54 | dt :  | semmle.label | dt :  |
+| app/controllers/foo/bars_controller.rb:26:53:26: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 |

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

@@ -22,6 +22,7 @@ def show
     @html_escaped = ERB::Util.html_escape(params[:text])
     @header_escaped = ERB::Util.html_escape(cookies[:foo]) # OK - cookies not controllable by 3rd party
     response.header["content-type"] = params[:content_type]
+    response.header["x-customer-header"] = params[:bar] # OK - header not relevant to XSS
     render "foo/bars/show", locals: { display_text: dt, safe_text: "hello" }
   end
 end