Skip to content

Instantly share code, notes, and snippets.

Embed
What would you like to do?
From b5aeef5703dab7da9ebb47cc20e4c8b64f7f5866 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Thu, 12 Mar 2020 10:25:48 -0700
Subject: [PATCH] Fix possible XSS vector in JS escape helper
This commit escapes dollar signs and backticks to prevent JS XSS issues
when using the `j` or `javascript_escape` helper
CVE-2020-5267
---
actionview/lib/action_view/helpers/javascript_helper.rb | 6 ++++--
actionview/test/template/javascript_helper_test.rb | 8 ++++++++
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/actionview/lib/action_view/helpers/javascript_helper.rb b/actionview/lib/action_view/helpers/javascript_helper.rb
index acc50f8a62..5d966ba3aa 100644
--- a/actionview/lib/action_view/helpers/javascript_helper.rb
+++ b/actionview/lib/action_view/helpers/javascript_helper.rb
@@ -12,7 +12,9 @@ module JavaScriptHelper
"\n" => '\n',
"\r" => '\n',
'"' => '\\"',
- "'" => "\\'"
+ "'" => "\\'",
+ "`" => "\\`",
+ "$" => "\\$"
}
JS_ESCAPE_MAP["\342\200\250".dup.force_encoding(Encoding::UTF_8).encode!] = "&#x2028;"
@@ -26,7 +28,7 @@ module JavaScriptHelper
# $('some_element').replaceWith('<%= j render 'some/element_template' %>');
def escape_javascript(javascript)
if javascript
- result = javascript.gsub(/(\\|<\/|\r\n|\342\200\250|\342\200\251|[\n\r"'])/u) { |match| JS_ESCAPE_MAP[match] }
+ result = javascript.gsub(/(\\|<\/|\r\n|\342\200\250|\342\200\251|[\n\r"']|[`]|[$])/u) { |match| JS_ESCAPE_MAP[match] }
javascript.html_safe? ? result.html_safe : result
else
""
diff --git a/actionview/test/template/javascript_helper_test.rb b/actionview/test/template/javascript_helper_test.rb
index a72bc6c2fe..de24245e51 100644
--- a/actionview/test/template/javascript_helper_test.rb
+++ b/actionview/test/template/javascript_helper_test.rb
@@ -32,6 +32,14 @@ def test_escape_javascript
assert_equal %(dont <\\/close> tags), j(%(dont </close> tags))
end
+ def test_escape_backtick
+ assert_equal "\\`", escape_javascript("`")
+ end
+
+ def test_escape_dollar_sign
+ assert_equal "\\$", escape_javascript("$")
+ end
+
def test_escape_javascript_with_safebuffer
given = %('quoted' "double-quoted" new-line:\n </closed>)
expect = %(\\'quoted\\' \\"double-quoted\\" new-line:\\n <\\/closed>)
--
2.21.0
From 1251d8817264744163fb12a3dba05ce61be5371b Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Thu, 12 Mar 2020 10:25:48 -0700
Subject: [PATCH] Fix possible XSS vector in JS escape helper
This commit escapes dollar signs and backticks to prevent JS XSS issues
when using the `j` or `javascript_escape` helper
CVE-2020-5267
---
actionview/lib/action_view/helpers/javascript_helper.rb | 6 ++++--
actionview/test/template/javascript_helper_test.rb | 8 ++++++++
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/actionview/lib/action_view/helpers/javascript_helper.rb b/actionview/lib/action_view/helpers/javascript_helper.rb
index b680cb1bd3..b04b1cb43e 100644
--- a/actionview/lib/action_view/helpers/javascript_helper.rb
+++ b/actionview/lib/action_view/helpers/javascript_helper.rb
@@ -12,7 +12,9 @@ module JavaScriptHelper
"\n" => '\n',
"\r" => '\n',
'"' => '\\"',
- "'" => "\\'"
+ "'" => "\\'",
+ "`" => "\\`",
+ "$" => "\\$"
}
JS_ESCAPE_MAP[(+"\342\200\250").force_encoding(Encoding::UTF_8).encode!] = "&#x2028;"
@@ -29,7 +31,7 @@ def escape_javascript(javascript)
if javascript.empty?
result = ""
else
- result = javascript.gsub(/(\\|<\/|\r\n|\342\200\250|\342\200\251|[\n\r"'])/u) { |match| JS_ESCAPE_MAP[match] }
+ result = javascript.gsub(/(\\|<\/|\r\n|\342\200\250|\342\200\251|[\n\r"']|[`]|[$])/u) { |match| JS_ESCAPE_MAP[match] }
end
javascript.html_safe? ? result.html_safe : result
end
diff --git a/actionview/test/template/javascript_helper_test.rb b/actionview/test/template/javascript_helper_test.rb
index f974e5ae0c..4b7284d15b 100644
--- a/actionview/test/template/javascript_helper_test.rb
+++ b/actionview/test/template/javascript_helper_test.rb
@@ -36,6 +36,14 @@ def test_escape_javascript
assert_equal %(dont <\\/close> tags), j(%(dont </close> tags))
end
+ def test_escape_backtick
+ assert_equal "\\`", escape_javascript("`")
+ end
+
+ def test_escape_dollar_sign
+ assert_equal "\\$", escape_javascript("$")
+ end
+
def test_escape_javascript_with_safebuffer
given = %('quoted' "double-quoted" new-line:\n </closed>)
expect = %(\\'quoted\\' \\"double-quoted\\" new-line:\\n <\\/closed>)
--
2.21.0
@mathieujobin

This comment has been minimized.

Copy link

@mathieujobin mathieujobin commented Jun 3, 2020

Adapted for Rails 4.2

mathieujobin/rails@a77ed78

@mathieujobin

This comment has been minimized.

Copy link

@mathieujobin mathieujobin commented Jun 3, 2020

Same patch can be cherry-picked on top of 5.0 mathieujobin/rails@aa9857c

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment