hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-06-19 03:41:07 +02:00
Code Issues Packages Projects Releases Wiki Activity

Don't use inline expectations when alerts in erb files

Browse Source
This commit is contained in:
Owen Mansel-Chan
2026-06-15 22:33:29 +01:00
parent 84e7c2de6c
commit 5e606b7bef
9 changed files with 50 additions and 53 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

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

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

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

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

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

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

@@ -6,34 +6,34 @@ class BarsController < ApplicationController
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

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

@@ -5,7 +5,7 @@ class StoresController < ApplicationController
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)

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

@@ -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] %>

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

@@ -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) %>

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

@@ -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 %>

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

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