hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2025-12-17 01:03:14 +01:00
Code Issues Packages Projects Releases Wiki Activity

Ruby: Restrict XSS header sinks

Browse Source 
Not all header writes are relevant to XSS. Restrict these to just
content-type and access-control-allow-origin.
This commit is contained in:
Harry Maclean
2022-10-17 08:14:31 +13:00
parent 8ae86cf443
commit 0e6322d673
5 changed files with 22 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
ruby/ql/lib/codeql/ruby/Concepts.qll
View File

@@ -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() }

8
ruby/ql/lib/codeql/ruby/security/XSS.qll
View File

@@ -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()
)
}
}
/**

10
ruby/ql/test/library-tests/frameworks/ActionController.expected
View File

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

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

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

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

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