Skip to content

Commit 6a32c0c

Browse files
committed
update XSS tests
1 parent 6dc3ce3 commit 6a32c0c

5 files changed

Lines changed: 159 additions & 6 deletions

File tree

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
edges
2+
| app/controllers/foo/stores_controller.rb:8:10:8:29 | call to read : | app/controllers/foo/stores_controller.rb:9:22:9:23 | dt : |
3+
| app/controllers/foo/stores_controller.rb:8:10:8:29 | call to read : | app/controllers/foo/stores_controller.rb:13:55:13:56 | dt : |
4+
| app/controllers/foo/stores_controller.rb:9:22:9:23 | dt : | app/views/foo/stores/show.html.erb:38:3:38:16 | @instance_text |
5+
| app/controllers/foo/stores_controller.rb:12:28:12:48 | call to raw_name : | app/views/foo/stores/show.html.erb:84:5:84:24 | @other_user_raw_name |
6+
| app/controllers/foo/stores_controller.rb:13:55:13:56 | dt : | app/views/foo/stores/show.html.erb:2:9:2:20 | call to display_text |
7+
| app/controllers/foo/stores_controller.rb:13:55:13:56 | dt : | app/views/foo/stores/show.html.erb:5:9:5:36 | ...[...] |
8+
| app/controllers/foo/stores_controller.rb:13:55:13:56 | dt : | app/views/foo/stores/show.html.erb:9:9:9:26 | ...[...] |
9+
| app/controllers/foo/stores_controller.rb:13:55:13:56 | dt : | app/views/foo/stores/show.html.erb:33:3:33:14 | call to display_text |
10+
| app/controllers/foo/stores_controller.rb:13:55:13:56 | dt : | app/views/foo/stores/show.html.erb:41:76:41:87 | call to display_text : |
11+
| app/views/foo/stores/show.html.erb:41:64:41:87 | ... + ... : | app/views/foo/bars/_widget.html.erb:5:9:5:20 | call to display_text |
12+
| app/views/foo/stores/show.html.erb:41:64:41:87 | ... + ... : | app/views/foo/bars/_widget.html.erb:8:9:8:36 | ...[...] |
13+
| app/views/foo/stores/show.html.erb:41:76:41:87 | call to display_text : | app/views/foo/stores/show.html.erb:41:64:41:87 | ... + ... : |
14+
nodes
15+
| app/controllers/foo/stores_controller.rb:8:10:8:29 | call to read : | semmle.label | call to read : |
16+
| app/controllers/foo/stores_controller.rb:9:22:9:23 | dt : | semmle.label | dt : |
17+
| app/controllers/foo/stores_controller.rb:12:28:12:48 | call to raw_name : | semmle.label | call to raw_name : |
18+
| app/controllers/foo/stores_controller.rb:13:55:13:56 | dt : | semmle.label | dt : |
19+
| app/views/foo/bars/_widget.html.erb:5:9:5:20 | call to display_text | semmle.label | call to display_text |
20+
| app/views/foo/bars/_widget.html.erb:8:9:8:36 | ...[...] | semmle.label | ...[...] |
21+
| app/views/foo/stores/show.html.erb:2:9:2:20 | call to display_text | semmle.label | call to display_text |
22+
| app/views/foo/stores/show.html.erb:5:9:5:36 | ...[...] | semmle.label | ...[...] |
23+
| app/views/foo/stores/show.html.erb:9:9:9:26 | ...[...] | semmle.label | ...[...] |
24+
| app/views/foo/stores/show.html.erb:33:3:33:14 | call to display_text | semmle.label | call to display_text |
25+
| app/views/foo/stores/show.html.erb:38:3:38:16 | @instance_text | semmle.label | @instance_text |
26+
| app/views/foo/stores/show.html.erb:41:64:41:87 | ... + ... : | semmle.label | ... + ... : |
27+
| app/views/foo/stores/show.html.erb:41:76:41:87 | call to display_text : | semmle.label | call to display_text : |
28+
| app/views/foo/stores/show.html.erb:47:5:47:16 | call to handle | semmle.label | call to handle |
29+
| app/views/foo/stores/show.html.erb:50:5:50:18 | call to raw_name | semmle.label | call to raw_name |
30+
| app/views/foo/stores/show.html.erb:64:3:64:18 | call to handle | semmle.label | call to handle |
31+
| app/views/foo/stores/show.html.erb:70:3:70:20 | call to raw_name | semmle.label | call to raw_name |
32+
| app/views/foo/stores/show.html.erb:84:5:84:24 | @other_user_raw_name | semmle.label | @other_user_raw_name |
33+
subpaths
34+
#select
35+
| app/views/foo/bars/_widget.html.erb:5:9:5:20 | call to display_text | app/controllers/foo/stores_controller.rb:8:10:8:29 | call to read : | app/views/foo/bars/_widget.html.erb:5:9:5:20 | call to display_text | Cross-site scripting vulnerability due to $@ | app/controllers/foo/stores_controller.rb:8:10:8:29 | call to read | stored value |
36+
| app/views/foo/bars/_widget.html.erb:8:9:8:36 | ...[...] | app/controllers/foo/stores_controller.rb:8:10:8:29 | call to read : | app/views/foo/bars/_widget.html.erb:8:9:8:36 | ...[...] | Cross-site scripting vulnerability due to $@ | app/controllers/foo/stores_controller.rb:8:10:8:29 | call to read | stored value |
37+
| app/views/foo/stores/show.html.erb:2:9:2:20 | call to display_text | app/controllers/foo/stores_controller.rb:8:10:8:29 | call to read : | app/views/foo/stores/show.html.erb:2:9:2:20 | call to display_text | Cross-site scripting vulnerability due to $@ | app/controllers/foo/stores_controller.rb:8:10:8:29 | call to read | stored value |
38+
| app/views/foo/stores/show.html.erb:5:9:5:36 | ...[...] | app/controllers/foo/stores_controller.rb:8:10:8:29 | call to read : | app/views/foo/stores/show.html.erb:5:9:5:36 | ...[...] | Cross-site scripting vulnerability due to $@ | app/controllers/foo/stores_controller.rb:8:10:8:29 | call to read | stored value |
39+
| app/views/foo/stores/show.html.erb:9:9:9:26 | ...[...] | app/controllers/foo/stores_controller.rb:8:10:8:29 | call to read : | app/views/foo/stores/show.html.erb:9:9:9:26 | ...[...] | Cross-site scripting vulnerability due to $@ | app/controllers/foo/stores_controller.rb:8:10:8:29 | call to read | stored value |
40+
| app/views/foo/stores/show.html.erb:33:3:33:14 | call to display_text | app/controllers/foo/stores_controller.rb:8:10:8:29 | call to read : | app/views/foo/stores/show.html.erb:33:3:33:14 | call to display_text | Cross-site scripting vulnerability due to $@ | app/controllers/foo/stores_controller.rb:8:10:8:29 | call to read | stored value |
41+
| app/views/foo/stores/show.html.erb:38:3:38:16 | @instance_text | app/controllers/foo/stores_controller.rb:8:10:8:29 | call to read : | app/views/foo/stores/show.html.erb:38:3:38:16 | @instance_text | Cross-site scripting vulnerability due to $@ | app/controllers/foo/stores_controller.rb:8:10:8:29 | call to read | stored value |
42+
| app/views/foo/stores/show.html.erb:47:5:47:16 | call to handle | app/views/foo/stores/show.html.erb:47:5:47:16 | call to handle | app/views/foo/stores/show.html.erb:47:5:47:16 | call to handle | Cross-site scripting vulnerability due to $@ | app/views/foo/stores/show.html.erb:47:5:47:16 | call to handle | stored value |
43+
| app/views/foo/stores/show.html.erb:50:5:50:18 | call to raw_name | app/views/foo/stores/show.html.erb:50:5:50:18 | call to raw_name | app/views/foo/stores/show.html.erb:50:5:50:18 | call to raw_name | Cross-site scripting vulnerability due to $@ | app/views/foo/stores/show.html.erb:50:5:50:18 | call to raw_name | stored value |
44+
| app/views/foo/stores/show.html.erb:64:3:64:18 | call to handle | app/views/foo/stores/show.html.erb:64:3:64:18 | call to handle | app/views/foo/stores/show.html.erb:64:3:64:18 | call to handle | Cross-site scripting vulnerability due to $@ | app/views/foo/stores/show.html.erb:64:3:64:18 | call to handle | stored value |
45+
| app/views/foo/stores/show.html.erb:70:3:70:20 | call to raw_name | app/views/foo/stores/show.html.erb:70:3:70:20 | call to raw_name | app/views/foo/stores/show.html.erb:70:3:70:20 | call to raw_name | Cross-site scripting vulnerability due to $@ | app/views/foo/stores/show.html.erb:70:3:70:20 | call to raw_name | stored value |
46+
| app/views/foo/stores/show.html.erb:84:5:84:24 | @other_user_raw_name | app/controllers/foo/stores_controller.rb:12:28:12:48 | call to raw_name : | app/views/foo/stores/show.html.erb:84:5:84:24 | @other_user_raw_name | Cross-site scripting vulnerability due to $@ | app/controllers/foo/stores_controller.rb:12:28:12:48 | call to raw_name | stored value |

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

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,4 @@ def show
2222
@html_escaped = ERB::Util.html_escape(params[:text])
2323
render "foo/bars/show", locals: { display_text: dt, safe_text: "hello" }
2424
end
25-
26-
def show_stored
27-
dt = File.read("foo.txt")
28-
@instance_text = dt
29-
render "foo/bars/show", locals: { display_text: dt, safe_text: "hello" }
30-
end
3125
end
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
class StoresController < ApplicationController
2+
helper_method :user_handle
3+
def user_handle
4+
User.find(1).handle
5+
end
6+
7+
def show
8+
dt = File.read("foo.txt")
9+
@instance_text = dt
10+
@user = User.find 1
11+
@safe_user_handle = ERB::Util.html_escape(@user.handle)
12+
@other_user_raw_name = User.find(2).raw_name
13+
render "foo/stores/show", locals: { display_text: dt, safe_text: "hello" }
14+
end
15+
end
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
class User < ActiveRecord::Base
2+
def is_dummy_user?
3+
self.user_id == 0
4+
end
5+
6+
def raw_name
7+
me = self
8+
me.handle
9+
end
10+
11+
def display_name
12+
self.real_name || self.handle
13+
end
14+
end
Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
<%# BAD: A local rendered raw as a local variable %>
2+
<%= raw display_text %>
3+
4+
<%# BAD: A local rendered raw via the local_assigns hash %>
5+
<%= raw local_assigns[:display_text] %>
6+
7+
<% key = :display_text %>
8+
<%# BAD: A local rendered raw via the locals_assigns hash %>
9+
<%= raw local_assigns[key] %>
10+
11+
<ul>
12+
<% for key in [:display_text, :safe_text] do %>
13+
<%# BAD: A local rendered raw via the locals hash %>
14+
<%# TODO: we miss that `key` can take `:display_text` as a value here %>
15+
<li><%= raw local_assigns[key] %></li>
16+
<% end %>
17+
</ul>
18+
19+
<%# GOOD: A local rendered with default escaping via the local_assigns hash %>
20+
<%= local_assigns[display_text] %>
21+
22+
<%# GOOD: default escaping of rendered text %>
23+
<%=
24+
full_text = prefix + local_assigns[:display_text]
25+
full_text
26+
%>
27+
28+
<%# GOOD: default escaping of rendered text (from instance var) %>
29+
<%= @instance_text %>
30+
31+
<%# BAD: html_safe marks string as not requiring HTML escaping %>
32+
<%=
33+
display_text.html_safe
34+
%>
35+
36+
<%# BAD: html_safe marks string as not requiring HTML escaping %>
37+
<%=
38+
@instance_text.html_safe
39+
%>
40+
41+
<%= render partial: 'foo/bars/widget', locals: { display_text: "widget_" + display_text } %>
42+
43+
<%# BAD: user_name_handle is a helper method that returns unsanitized database content %>
44+
<%= user_name_handle.html_safe %>
45+
46+
<%# BAD: Direct to a database value without escaping %>
47+
<%= @user.handle.html_safe %>
48+
49+
<%# BAD: Indirect to a database value without escaping %>
50+
<%= @user.raw_name.html_safe %>
51+
52+
<%# GOOD: Direct to a database value with escaping %>
53+
<%= @user.handle %>
54+
55+
<%# GOOD: @safe_user_handle is manually escaped in the controller %>
56+
<%= @safe_user_handle %>
57+
58+
<%# GOOD: object_id is a built-in method, not an ORM access method %>
59+
<%= @user.object_id.html_safe %>
60+
61+
<%# BAD: Direct to a database value without escaping %>
62+
<%=
63+
some_user = User.find 1
64+
some_user.handle.html_safe
65+
%>
66+
67+
<%# BAD: Indirect to a database value without escaping %>
68+
<%=
69+
some_user = User.find 1
70+
some_user.raw_name.html_safe
71+
%>
72+
73+
<%# GOOD: Direct to a database value with escaping %>
74+
<%=
75+
some_user = User.find 1
76+
some_user.handle
77+
%>
78+
79+
<%# BAD: Indirect to a database value without escaping %>
80+
<%# TODO: we do not detect that `display_name` can return a DB field %>
81+
<%= @user.display_name.html_safe %>
82+
83+
<%# BAD: Indirect to a database value without escaping %>
84+
<%= @other_user_raw_name.html_safe %>

0 commit comments

Comments
 (0)