From 0cc5764ab35f073f1e8e82d10dee257fee700199 Mon Sep 17 00:00:00 2001
From: Martin Edenhofer <me@zammad.com>
Date: Wed, 29 Sep 2021 10:13:40 +0200
Subject: [PATCH] Fixes #3365 - No script content (e. g. JavaScript) in emails

---
 config/initializers/html_sanitizer.rb              |  2 +-
 spec/models/channel/email_parser_spec.rb           |  2 +-
 .../concerns/has_xss_sanitized_note_examples.rb    |  4 ++--
 spec/models/ticket/article_spec.rb                 |  6 +++---
 test/unit/html_sanitizer_test.rb                   | 14 +++++++-------
 5 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/config/initializers/html_sanitizer.rb b/config/initializers/html_sanitizer.rb
index 8183f9a85..80ae25b52 100644
--- a/config/initializers/html_sanitizer.rb
+++ b/config/initializers/html_sanitizer.rb
@@ -5,11 +5,11 @@ Rails.application.config.html_sanitizer_tags_remove_content = %w[
   style
   comment
   meta
+  script
 ]
 
 # content of this tags will will be inserted html quoted
 Rails.application.config.html_sanitizer_tags_quote_content = %w[
-  script
 ]
 
 # only this tags are allowed
diff --git a/spec/models/channel/email_parser_spec.rb b/spec/models/channel/email_parser_spec.rb
index b553db6f3..9a30a3102 100644
--- a/spec/models/channel/email_parser_spec.rb
+++ b/spec/models/channel/email_parser_spec.rb
@@ -1258,7 +1258,7 @@ RSpec.describe Channel::EmailParser, type: :model do
         let(:content_type) { 'text/html' }
 
         it 'removes injected <script> tags from body' do
-          expect(article.body).to eq("no HTML alert('XSS')")
+          expect(article.body).to eq('no HTML')
         end
       end
 
diff --git a/spec/models/concerns/has_xss_sanitized_note_examples.rb b/spec/models/concerns/has_xss_sanitized_note_examples.rb
index c64d728a0..6a8952af6 100644
--- a/spec/models/concerns/has_xss_sanitized_note_examples.rb
+++ b/spec/models/concerns/has_xss_sanitized_note_examples.rb
@@ -5,8 +5,8 @@ RSpec.shared_examples 'HasXssSanitizedNote' do |model_factory:|
     context 'with injected JS' do
       subject { create(model_factory, note: 'test 123 <script type="text/javascript">alert("XSS!");</script> <b>some text</b>') }
 
-      it 'strips out <script> tag' do
-        expect(subject.note).to eq('test 123 alert("XSS!"); <b>some text</b>')
+      it 'strips out <script> tag with content' do
+        expect(subject.note).to eq('test 123  <b>some text</b>')
       end
     end
   end
diff --git a/spec/models/ticket/article_spec.rb b/spec/models/ticket/article_spec.rb
index abb1824d6..4dd3e102e 100644
--- a/spec/models/ticket/article_spec.rb
+++ b/spec/models/ticket/article_spec.rb
@@ -87,11 +87,11 @@ RSpec.describe Ticket::Article, type: :model do
 
       context 'when body contains only injected JS' do
         let(:body) { <<~RAW.chomp }
-          <script type="text/javascript">alert("XSS!");</script>
+          <script type="text/javascript">alert("XSS!");</script> some other text
         RAW
 
         it 'removes <script> tags' do
-          expect(article.body).to eq('alert("XSS!");')
+          expect(article.body).to eq(' some other text')
         end
       end
 
@@ -102,7 +102,7 @@ RSpec.describe Ticket::Article, type: :model do
 
         it 'removes <script> tags' do
           expect(article.body).to eq(<<~SANITIZED.chomp)
-            please tell me this doesn't work: alert("XSS!");
+            please tell me this doesn't work:#{' '}
           SANITIZED
         end
       end
diff --git a/test/unit/html_sanitizer_test.rb b/test/unit/html_sanitizer_test.rb
index c3d74e07d..0539c1359 100644
--- a/test/unit/html_sanitizer_test.rb
+++ b/test/unit/html_sanitizer_test.rb
@@ -6,18 +6,18 @@ class HtmlSanitizerTest < ActiveSupport::TestCase
 
   test 'xss' do
     assert_equal(HtmlSanitizer.strict('<b>123</b>'), '<b>123</b>')
-    assert_equal(HtmlSanitizer.strict('<script><b>123</b></script>'), '&lt;b&gt;123&lt;/b&gt;')
-    assert_equal(HtmlSanitizer.strict('<script><style><b>123</b></style></script>'), '&lt;style&gt;&lt;b&gt;123&lt;/b&gt;&lt;/style&gt;')
+    assert_equal(HtmlSanitizer.strict('<script><b>123</b></script>'), '')
+    assert_equal(HtmlSanitizer.strict('<script><style><b>123</b></style></script>'), '')
     assert_equal(HtmlSanitizer.strict('<abc><i><b>123</b><bbb>123</bbb></i></abc>'), '<i><b>123</b>123</i>')
     assert_equal(HtmlSanitizer.strict('<abc><i><b>123</b><bbb>123<i><ccc>abc</ccc></i></bbb></i></abc>'), '<i><b>123</b>123<i>abc</i></i>')
     assert_equal(HtmlSanitizer.strict('<not_existing>123</not_existing>'), '123')
-    assert_equal(HtmlSanitizer.strict('<script type="text/javascript">alert("XSS!");</script>'), 'alert("XSS!");')
+    assert_equal(HtmlSanitizer.strict('<script type="text/javascript">alert("XSS!");</script>'), '')
     assert_equal(HtmlSanitizer.strict('<SCRIPT SRC=http://xss.rocks/xss.js></SCRIPT>'), '')
     assert_equal(HtmlSanitizer.strict('<IMG SRC="javascript:alert(\'XSS\');">'), '')
     assert_equal(HtmlSanitizer.strict('<IMG SRC=javascript:alert(\'XSS\')>'), '')
     assert_equal(HtmlSanitizer.strict('<IMG SRC=JaVaScRiPt:alert(\'XSS\')>'), '')
     assert_equal(HtmlSanitizer.strict('<IMG SRC=`javascript:alert("RSnake says, \'XSS\'")`>'), '')
-    assert_equal(HtmlSanitizer.strict('<IMG """><SCRIPT>alert("XSS")</SCRIPT>">'), '<img>alert("XSS")"&gt;')
+    assert_equal(HtmlSanitizer.strict('<IMG """><SCRIPT>alert("XSS")</SCRIPT>">'), '<img>"&gt;')
     assert_equal(HtmlSanitizer.strict('<IMG SRC=# onmouseover="alert(\'xxs\')">'), '<img src="#">')
     assert_equal(HtmlSanitizer.strict('<IMG SRC="jav  ascript:alert(\'XSS\');">'), '')
     assert_equal(HtmlSanitizer.strict('<IMG SRC="jav&#x09;ascript:alert(\'XSS\');">'), '')
@@ -27,13 +27,13 @@ class HtmlSanitizerTest < ActiveSupport::TestCase
     assert_equal(HtmlSanitizer.strict('<SCRIPT/XSS SRC="http://xss.rocks/xss.js"></SCRIPT>'), '')
     assert_equal(HtmlSanitizer.strict('<BODY onload!#$%&()*~+-_.,:;?@[/|\]^`=alert("XSS")>'), '')
     assert_equal(HtmlSanitizer.strict('<SCRIPT/SRC="http://xss.rocks/xss.js"></SCRIPT>'), '')
-    assert_equal(HtmlSanitizer.strict('<<SCRIPT>alert("XSS");//<</SCRIPT>'), '&lt;alert("XSS");//&lt;')
+    assert_equal(HtmlSanitizer.strict('<<SCRIPT>alert("XSS");//<</SCRIPT>'), '&lt;')
     assert_equal(HtmlSanitizer.strict('<SCRIPT SRC=http://xss.rocks/xss.js?< B >'), '')
     assert_equal(HtmlSanitizer.strict('<SCRIPT SRC=//xss.rocks/.j>'), '')
     assert_equal(HtmlSanitizer.strict('<IMG SRC="javascript:alert(\'XSS\')"'), '')
     assert_equal(HtmlSanitizer.strict('<IMG SRC="javascript:alert(\'XSS\')" abc<b>123</b>'), '123')
     assert_equal(HtmlSanitizer.strict('<iframe src=http://xss.rocks/scriptlet.html <'), '')
-    assert_equal(HtmlSanitizer.strict('</script><script>alert(\'XSS\');</script>'), 'alert(\'XSS\');')
+    assert_equal(HtmlSanitizer.strict('</script><script>alert(\'XSS\');</script>'), '')
     assert_equal(HtmlSanitizer.strict('<STYLE>li {list-style-image: url("javascript:alert(\'XSS\')");}</STYLE><UL><LI>XSS</br>'), '<ul><li>XSS</li></ul>')
     assert_equal(HtmlSanitizer.strict('<IMG SRC=\'vbscript:msgbox("XSS")\'>'), '')
     assert_equal(HtmlSanitizer.strict('<IMG SRC="livescript:[code]">'), '')
@@ -73,7 +73,7 @@ tt  p://6 6.000146.0x7.147/">XSS</A>', true), '<a href="http://h%0Att%20%20p://6
     assert_equal(HtmlSanitizer.strict('<img[a][b]src=x[d]onerror[c]=[e]"alert(1)">'), '<img>')
     assert_equal(HtmlSanitizer.strict('<a href="[a]java[b]script[c]:alert(1)">XXX</a>'), '<a href="%5Ba%5Djava%5Bb%5Dscript%5Bc%5D:alert(1)">XXX</a>')
     assert_equal(HtmlSanitizer.strict('<a href="[a]java[b]script[c]:alert(1)">XXX</a>', true), '<a href="http://%5Ba%5Djava%5Bb%5Dscript%5Bc%5D:alert(1)" rel="nofollow noreferrer noopener" target="_blank" title="http://%5Ba%5Djava%5Bb%5Dscript%5Bc%5D:alert(1)">XXX</a>')
-    assert_equal(HtmlSanitizer.strict('<svg xmlns="http://www.w3.org/2000/svg"><script>alert(1)</script></svg>'), 'alert(1)')
+    assert_equal(HtmlSanitizer.strict('<svg xmlns="http://www.w3.org/2000/svg"><script>alert(1)</script></svg>'), '')
     assert_equal(HtmlSanitizer.strict('<a style="position:fixed;top:0;left:0;width: 260px;height:100vh;background-color:red;display: block;" href="http://example.com"></a>'), '<a href="http://example.com" rel="nofollow noreferrer noopener" target="_blank" title="http://example.com"></a>')
     assert_equal(HtmlSanitizer.strict('<a style="position:fixed;top:0;left:0;width: 260px;height:100vh;background-color:red;display: block;" href="http://example.com"></a>', true), '<a href="http://example.com" rel="nofollow noreferrer noopener" target="_blank" title="http://example.com"></a>')
     assert_equal(HtmlSanitizer.strict('<div>