Skip to content

Commit 8ae86cf

Browse files
committed
Ruby: Consider header writes as XSS sinks
1 parent 545222d commit 8ae86cf

File tree

3 files changed

+19
-7
lines changed

3 files changed

+19
-7
lines changed

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,11 @@ private module Shared {
105105
}
106106
}
107107

108+
/** A write to an HTTP response header, considered as a flow sink. */
109+
class HeaderWriteAsSink extends Sink {
110+
HeaderWriteAsSink() { this = any(Http::Server::HeaderWriteAccess a).getValue() }
111+
}
112+
108113
/**
109114
* An HTML escaping, considered as a sanitizer.
110115
*/

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

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,15 @@ edges
1010
| app/controllers/foo/bars_controller.rb:17:21:17:36 | ...[...] : | app/views/foo/bars/show.html.erb:2:18:2:30 | @user_website |
1111
| app/controllers/foo/bars_controller.rb:18:10:18:15 | call to params : | app/controllers/foo/bars_controller.rb:18:10:18:22 | ...[...] : |
1212
| app/controllers/foo/bars_controller.rb:18:10:18:22 | ...[...] : | app/controllers/foo/bars_controller.rb:19:22:19:23 | dt : |
13-
| app/controllers/foo/bars_controller.rb:18:10:18:22 | ...[...] : | app/controllers/foo/bars_controller.rb:24:53:24:54 | dt : |
13+
| app/controllers/foo/bars_controller.rb:18:10:18:22 | ...[...] : | app/controllers/foo/bars_controller.rb:25:53:25:54 | dt : |
1414
| app/controllers/foo/bars_controller.rb:19:22:19:23 | dt : | app/views/foo/bars/show.html.erb:41:3:41:16 | @instance_text |
15-
| app/controllers/foo/bars_controller.rb:24:53:24:54 | dt : | app/views/foo/bars/show.html.erb:5:9:5:20 | call to display_text |
16-
| app/controllers/foo/bars_controller.rb:24:53:24:54 | dt : | app/views/foo/bars/show.html.erb:8:9:8:36 | ...[...] |
17-
| app/controllers/foo/bars_controller.rb:24:53:24:54 | dt : | app/views/foo/bars/show.html.erb:12:9:12:26 | ...[...] |
18-
| app/controllers/foo/bars_controller.rb:24:53:24:54 | dt : | app/views/foo/bars/show.html.erb:36:3:36:14 | call to display_text |
19-
| app/controllers/foo/bars_controller.rb:24:53:24:54 | dt : | app/views/foo/bars/show.html.erb:44:76:44:87 | call to display_text : |
15+
| app/controllers/foo/bars_controller.rb:24:39:24:44 | call to params : | app/controllers/foo/bars_controller.rb:24:39:24:59 | ...[...] : |
16+
| app/controllers/foo/bars_controller.rb:24:39:24:59 | ...[...] : | app/controllers/foo/bars_controller.rb:24:39:24:59 | ... = ... |
17+
| 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 |
18+
| app/controllers/foo/bars_controller.rb:25:53:25:54 | dt : | app/views/foo/bars/show.html.erb:8:9:8:36 | ...[...] |
19+
| app/controllers/foo/bars_controller.rb:25:53:25:54 | dt : | app/views/foo/bars/show.html.erb:12:9:12:26 | ...[...] |
20+
| 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 |
21+
| 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 : |
2022
| 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 |
2123
| app/views/foo/bars/show.html.erb:44:64:44:87 | ... + ... : | app/views/foo/bars/_widget.html.erb:8:9:8:36 | ...[...] |
2224
| 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 | ... + ... : |
@@ -35,7 +37,10 @@ nodes
3537
| app/controllers/foo/bars_controller.rb:18:10:18:15 | call to params : | semmle.label | call to params : |
3638
| app/controllers/foo/bars_controller.rb:18:10:18:22 | ...[...] : | semmle.label | ...[...] : |
3739
| app/controllers/foo/bars_controller.rb:19:22:19:23 | dt : | semmle.label | dt : |
38-
| app/controllers/foo/bars_controller.rb:24:53:24:54 | dt : | semmle.label | dt : |
40+
| app/controllers/foo/bars_controller.rb:24:39:24:44 | call to params : | semmle.label | call to params : |
41+
| app/controllers/foo/bars_controller.rb:24:39:24:59 | ... = ... | semmle.label | ... = ... |
42+
| app/controllers/foo/bars_controller.rb:24:39:24:59 | ...[...] : | semmle.label | ...[...] : |
43+
| app/controllers/foo/bars_controller.rb:25:53:25:54 | dt : | semmle.label | dt : |
3944
| app/views/foo/bars/_widget.html.erb:5:9:5:20 | call to display_text | semmle.label | call to display_text |
4045
| app/views/foo/bars/_widget.html.erb:8:9:8:36 | ...[...] | semmle.label | ...[...] |
4146
| app/views/foo/bars/show.html.erb:2:18:2:30 | @user_website | semmle.label | @user_website |
@@ -58,6 +63,7 @@ nodes
5863
| app/views/foo/bars/show.html.erb:77:28:77:39 | ...[...] | semmle.label | ...[...] |
5964
subpaths
6065
#select
66+
| 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 |
6167
| app/views/foo/bars/_widget.html.erb:5:9:5:20 | call to display_text | app/controllers/foo/bars_controller.rb:18:10:18:15 | call to params : | app/views/foo/bars/_widget.html.erb:5:9:5:20 | 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 |
6268
| app/views/foo/bars/_widget.html.erb:8:9:8:36 | ...[...] | app/controllers/foo/bars_controller.rb:18:10:18:15 | call to params : | app/views/foo/bars/_widget.html.erb:8:9:8:36 | ...[...] | Cross-site scripting vulnerability due to a $@. | app/controllers/foo/bars_controller.rb:18:10:18:15 | call to params | user-provided value |
6369
| app/views/foo/bars/show.html.erb:2:18:2:30 | @user_website | app/controllers/foo/bars_controller.rb:17:21:17:26 | call to params : | app/views/foo/bars/show.html.erb:2:18:2:30 | @user_website | Cross-site scripting vulnerability due to a $@. | app/controllers/foo/bars_controller.rb:17:21:17:26 | call to params | user-provided value |

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ def show
2121
@safe_foo = "safe_foo"
2222
@html_escaped = ERB::Util.html_escape(params[:text])
2323
@header_escaped = ERB::Util.html_escape(cookies[:foo]) # OK - cookies not controllable by 3rd party
24+
response.header["content-type"] = params[:content_type]
2425
render "foo/bars/show", locals: { display_text: dt, safe_text: "hello" }
2526
end
2627
end

0 commit comments

Comments
 (0)