Don't use inline expectations when alerts in erb files

Author: owen-mc

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

@@ -1,2 +1 @@
-query: queries/security/cwe-079/ReflectedXSS.ql
-postprocess: utils/test/InlineExpectationsTestQuery.ql
+queries/security/cwe-079/ReflectedXSS.ql

ruby/ql/test/query-tests/security/cwe-079/StoredXSS.qlref

@@ -1,2 +1 @@
-query: queries/security/cwe-079/StoredXSS.ql
-postprocess: utils/test/InlineExpectationsTestQuery.ql
+queries/security/cwe-079/StoredXSS.ql

ruby/ql/test/query-tests/security/cwe-079/UnsafeHtmlConstruction.qlref

@@ -1,2 +1 @@
-query: queries/security/cwe-079/UnsafeHtmlConstruction.ql
-postprocess: utils/test/InlineExpectationsTestQuery.ql
+queries/security/cwe-079/UnsafeHtmlConstruction.ql

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

@@ -6,34 +6,34 @@ def index
   end
 
   def user_name
-    return params[:user_name] # $ Source[rb/reflected-xss]
+    return params[:user_name]
   end
 
   def user_name_memo
-    @user_name ||= params[:user_name] # $ Source[rb/reflected-xss]
+    @user_name ||= params[:user_name]
   end
 
   def show
-    @user_website = params[:website] # $ Source[rb/reflected-xss]
-    dt = params[:text] # $ Source[rb/reflected-xss]
+    @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])
     @header_escaped = ERB::Util.html_escape(cookies[:foo]) # OK - cookies not controllable by 3rd party
-    response.header["content-type"] = params[:content_type] # $ Alert[rb/reflected-xss]
+    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
 
   def make_safe_html
-    str = params[:user_name] # $ Source[rb/reflected-xss]
-    str.html_safe # $ Alert[rb/reflected-xss]
+    str = params[:user_name]
+    str.html_safe
 
-    translate("welcome", name: params[:user_name]).html_safe # $ Alert[rb/reflected-xss] // NOT OK - translate preserves taint
-    t("welcome", name: params[:user_name]).html_safe # $ Alert[rb/reflected-xss] // NOT OK - t is an alias of translate
+    translate("welcome", name: params[:user_name]).html_safe # NOT OK - translate preserves taint
+    t("welcome", name: params[:user_name]).html_safe # NOT OK - t is an alias of translate
     t("welcome_html", name: params[:user_name]).html_safe # OK - t escapes html when key ends in _html
-    I18n.t("welcome_html", name: params[:user_name]).html_safe # $ Alert[rb/reflected-xss] // NOT OK - I18n.t does not escape html
-    I18n.translate("welcome_html", name: params[:user_name]).html_safe # $ Alert[rb/reflected-xss] // NOT OK - alias
+    I18n.t("welcome_html", name: params[:user_name]).html_safe # NOT OK - I18n.t does not escape html
+    I18n.translate("welcome_html", name: params[:user_name]).html_safe # NOT OK - alias
   end
 end

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

@@ -5,7 +5,7 @@ def user_handle
   end
 
   def show
-    dt = File.read("foo.txt") # $ Source[rb/stored-xss]
+    dt = File.read("foo.txt")
     @instance_text = dt
     @user = User.find 1
     @safe_user_handle = ERB::Util.html_escape(@user.handle)

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

@@ -2,10 +2,10 @@
 <%= raw @display_text %>
 
 <%# BAD: A local rendered raw as a local variable %>
-<%= raw display_text %> <%# $ Alert[rb/reflected-xss], Alert[rb/stored-xss] %>
+<%= raw display_text %>
 
 <%# BAD: A local rendered raw via the local_assigns hash %>
-<%= raw local_assigns[:display_text] %> <%# $ Alert[rb/reflected-xss], Alert[rb/stored-xss] %>
+<%= raw local_assigns[:display_text] %>
 
 <%# GOOD: A local rendered with default escaping via the local_assigns hash %>
 <%= local_assigns[:display_text] %>

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

@@ -1,20 +1,20 @@
 <%# BAD: An instance variable rendered without escaping %>
-<a href="<%= raw @user_website %>">website</a> <%# $ Alert[rb/reflected-xss] %>
+<a href="<%= raw @user_website %>">website</a>
 
 <%# BAD: A local rendered raw as a local variable %>
-<%= raw display_text %> <%# $ Alert[rb/reflected-xss] %>
+<%= raw display_text %>
 
 <%# BAD: A local rendered raw via the local_assigns hash %>
-<%= raw local_assigns[:display_text] %> <%# $ Alert[rb/reflected-xss] %>
+<%= raw local_assigns[:display_text] %>
 
 <% key = :display_text %>
 <%# BAD: A local rendered raw via the locals_assigns hash %>
-<%= raw local_assigns[key] %> <%# $ Alert[rb/reflected-xss] %>
+<%= raw local_assigns[key] %>
 
 <ul>
 <% for key in [:display_text, :safe_text] do %>
   <%# BAD: A local rendered raw via the locals hash %>
-  <li><%= raw local_assigns[key] %></li> <%# $ Alert[rb/reflected-xss] %>
+  <li><%= raw local_assigns[key] %></li>
 <% end %>
 </ul>
 
@@ -32,28 +32,28 @@
 
 <%# BAD: html_safe marks string as not requiring HTML escaping %>
 <%=
-  display_text.html_safe <%# $ Alert[rb/reflected-xss] %>
+  display_text.html_safe
 %>
 
 <%# BAD: html_safe marks string as not requiring HTML escaping %>
 <%=
-  @instance_text.html_safe <%# $ Alert[rb/reflected-xss] %>
+  @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 %> <%# $ Alert[rb/reflected-xss] %>
+<%= 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 %> <%# $ Alert[rb/reflected-xss] %>
+<%= 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] %> <%# $ Alert[rb/reflected-xss] %>
+<%= 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 %> <%# $ Alert[rb/reflected-xss] %>
+<%= link_to params[:website], class: "user-link" do %>
   user website
 <% end %>
 
@@ -70,20 +70,20 @@
 %>
 
 <%# BAD: simple_format called with sanitize: false %>
-<%= simple_format(params[:comment], sanitize: false) %> <%# $ Alert[rb/reflected-xss] %>
+<%= simple_format(params[:comment], sanitize: false) %>
 
 <%# BAD: javasript_include_tag called with remote input %>
-<%= javascript_include_tag params[:url] %> <%# $ Alert[rb/reflected-xss] %>
+<%= javascript_include_tag params[:url] %>
 
 <%# GOOD: input is sanitized %>
 <%= sanitize(params[:comment]).html_safe %>
 
 <%# BAD: A local rendered raw as a local variable %>
-<%== display_text %> <%# $ Alert[rb/reflected-xss] %>
+<%== display_text %>
 
 <%# BAD: translate preserves taint %>
-<%= raw translate("welcome", name: display_text) %> <%# $ Alert[rb/reflected-xss] %>
-<%= raw t("welcome", name: display_text) %> <%# $ Alert[rb/reflected-xss] %>
+<%= raw translate("welcome", name: display_text) %>
+<%= raw t("welcome", name: display_text) %>
 
 <%# GOOD: translate sanitizes for html keys %>
 <%= raw t("welcome1.html", name: display_text) %>

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

@@ -1,17 +1,17 @@
 <%# BAD: A local rendered raw as a local variable %>
-<%= raw display_text %> <%# $ Alert[rb/stored-xss] %>
+<%= raw display_text %>
 
 <%# BAD: A local rendered raw via the local_assigns hash %>
-<%= raw local_assigns[:display_text] %> <%# $ Alert[rb/stored-xss] %>
+<%= raw local_assigns[:display_text] %>
 
 <% key = :display_text %>
 <%# BAD: A local rendered raw via the locals_assigns hash %>
-<%= raw local_assigns[key] %> <%# $ Alert[rb/stored-xss] %>
+<%= raw local_assigns[key] %>
 
 <ul>
 <% for key in [:display_text, :safe_text] do %>
   <%# BAD: A local rendered raw via the locals hash %>
-  <li><%= raw local_assigns[key] %></li> <%# $ Alert[rb/stored-xss] %>
+  <li><%= raw local_assigns[key] %></li>
 <% end %>
 </ul>
 
@@ -29,12 +29,12 @@
 
 <%# BAD: html_safe marks string as not requiring HTML escaping %>
 <%=
-  display_text.html_safe <%# $ Alert[rb/stored-xss] %>
+  display_text.html_safe
 %>
 
 <%# BAD: html_safe marks string as not requiring HTML escaping %>
 <%=
-  @instance_text.html_safe <%# $ Alert[rb/stored-xss] %>
+  @instance_text.html_safe
 %>
 
 <%= render partial: 'foo/bars/widget', locals: { display_text: "widget_" + display_text } %>
@@ -43,7 +43,7 @@
 <%= user_name_handle.html_safe %>
 
 <%# BAD: Direct to a database value without escaping %>
-<%= @user.handle.html_safe %> <%# $ Alert[rb/stored-xss] %>
+<%= @user.handle.html_safe %>
 
 <%# BAD: Indirect to a database value without escaping %>
 <%= @user.raw_name.html_safe %>
@@ -60,7 +60,7 @@
 <%# BAD: Direct to a database value without escaping %>
 <%=
   some_user = User.find 1
-  some_user.handle.html_safe <%# $ Alert[rb/stored-xss] %>
+  some_user.handle.html_safe
 %>
 
 <%# BAD: Indirect to a database value without escaping (currently missed due to lack of 'self' handling in ORM tracking) %>
@@ -83,7 +83,7 @@
 
 <%# BAD: Kernel.sprintf is a taint-step %>
 <%=
-  sprintf("%s", @user.handle).html_safe <%# $ Alert[rb/stored-xss] %>
+  sprintf("%s", @user.handle).html_safe
 %>
 
 <%# GOOD: The `foo.bar.baz` is not recognized as a source %>

ruby/ql/test/query-tests/security/cwe-079/lib/unsafeHtml.rb

@@ -1,27 +1,27 @@
 class Foobar
-  def create_user_description(name) # $ Source[rb/html-constructed-from-input]
-    "<h2>#{name}</h2>".html_safe # $ Alert[rb/html-constructed-from-input] // NOT OK - the parameter is not escaped
+  def create_user_description(name)
+    "<h2>#{name}</h2>".html_safe # NOT OK - the parameter is not escaped
 
     # escape
     "<h2>#{ERB::Util.html_escape(name)}</h2>".html_safe # OK - the parameter is escaped
   end
 
-  def string_like_literal name # $ Source[rb/html-constructed-from-input]
+  def string_like_literal name
     h = <<-HTML
-        <h2>#{name}</h2> # $ Alert[rb/html-constructed-from-input]
+        <h2>#{name}</h2>
     HTML
     h.html_safe # NOT OK - the parameter is not escaped
   end
 
-  def sprintf_use name # $ Source[rb/html-constructed-from-input]
-    sprintf("<h2>%s</h2>", name).html_safe # $ Alert[rb/html-constructed-from-input] // NOT OK - the parameter is not escaped
+  def sprintf_use name
+    sprintf("<h2>%s</h2>", name).html_safe # NOT OK - the parameter is not escaped
 
     # escape
     sprintf("<h2>%s</h2>", ERB::Util.html_escape(name)).html_safe # OK - the parameter is escaped
   end
 
-  def create_user_description2(name) # $ Source[rb/html-constructed-from-input]
-    "<h2>#{name}</h2>".html_safe # $ Alert[rb/html-constructed-from-input] // NOT OK - the value is not necessarily HTML safe
+  def create_user_description2(name)
+    "<h2>#{name}</h2>".html_safe # NOT OK - the value is not necessarily HTML safe
 
     if name.html_safe?
       "<h2>#{name}</h2>".html_safe # OK - value is marked as being HTML safe