From 5f54ea3877acc06b5d6fbbba38f45acf4f879a47 Mon Sep 17 00:00:00 2001
From: Ana Nunes da Silva <ananunesdasilva@gmail.com>
Date: Fri, 31 May 2024 17:51:52 +0100
Subject: [PATCH] Add safe trix tags to html sanitizer; Use custom html
 sanitizer in product description.

---
 app/models/spree/product.rb          |  6 +-
 app/services/html_sanitizer.rb       |  6 +-
 spec/services/html_sanitizer_spec.rb | 94 +++++++++++++++++++++-------
 3 files changed, 81 insertions(+), 25 deletions(-)

diff --git a/app/models/spree/product.rb b/app/models/spree/product.rb
index e51f23ac7c..1f79ebf95c 100755
--- a/app/models/spree/product.rb
+++ b/app/models/spree/product.rb
@@ -306,12 +306,12 @@ module Spree
 
     # Remove any unsupported HTML.
     def description
-      Rails::HTML::SafeListSanitizer.new.sanitize(super)
+      HtmlSanitizer.sanitize(super)
     end
 
-    # # Remove any unsupported HTML.
+    # Remove any unsupported HTML.
     def description=(html)
-      super(Rails::HTML::SafeListSanitizer.new.sanitize(html))
+      super(HtmlSanitizer.sanitize(html))
     end
 
     private
diff --git a/app/services/html_sanitizer.rb b/app/services/html_sanitizer.rb
index df3c608219..e6d81d7ecd 100644
--- a/app/services/html_sanitizer.rb
+++ b/app/services/html_sanitizer.rb
@@ -6,10 +6,14 @@
 # We offer an editor which supports certain tags but you can't insert just any
 # HTML, which would be dangerous.
 class HtmlSanitizer
+  ALLOWED_TAGS = %w[h1 h2 h3 h4 p br b i u a strong em del pre blockquote ul ol li hr figure].freeze
+  ALLOWED_ATTRIBUTES = %w[href target].freeze
+  ALLOWED_TRIX_DATA_ATTRIBUTES = %w[data-trix-attachment].freeze
+
   def self.sanitize(html)
     @sanitizer ||= Rails::HTML5::SafeListSanitizer.new
     @sanitizer.sanitize(
-      html, tags: %w[h1 h2 h3 h4 p br b i u a], attributes: %w[href target],
+      html, tags: ALLOWED_TAGS, attributes: (ALLOWED_ATTRIBUTES + ALLOWED_TRIX_DATA_ATTRIBUTES)
     )
   end
 end
diff --git a/spec/services/html_sanitizer_spec.rb b/spec/services/html_sanitizer_spec.rb
index c98d695b35..090d4bd3e9 100644
--- a/spec/services/html_sanitizer_spec.rb
+++ b/spec/services/html_sanitizer_spec.rb
@@ -5,33 +5,85 @@ require 'spec_helper'
 RSpec.describe HtmlSanitizer do
   subject { described_class }
 
-  it "removes dangerous tags" do
-    html = "Hello <script>alert</script>!"
-    expect(subject.sanitize(html))
-      .to eq "Hello alert!"
+  context "when HTML has supported tags" do
+    it "keeps supported tags" do
+      html = "Hello <b>alert</b>! <br>How are you?"
+      expect(subject.sanitize(html))
+        .to eq "Hello <b>alert</b>! <br>How are you?"
+    end
+
+    it "handles nested tags" do
+      html = '<ul><li>Item 1</li><li><strong>Item 2</strong></li></ul>'
+      expect(subject.sanitize(html)).to eq(html)
+    end
   end
 
-  it "keeps supported tags" do
-    html = "Hello <b>alert</b>! <br>How are you?"
-    expect(subject.sanitize(html))
-      .to eq "Hello <b>alert</b>! <br>How are you?"
+  context "when HTML has dangerous tags" do
+    it "removes script tags" do
+      html = "Hello <script>alert</script>!"
+      expect(subject.sanitize(html)).to eq "Hello alert!"
+    end
+
+    it "removes iframe tags" do
+      html = "Content <iframe src='http://malicious-site.com'></iframe>"
+      expect(subject.sanitize(html)).to eq "Content "
+    end
+
+    it "removes object tags" do
+      html = "<object data='malicious-file.swf'></object>"
+      expect(subject.sanitize(html)).to eq ""
+    end
+
+    it "removes embed tags" do
+      html = "<embed src='malicious-video.mp4' type='video/mp4'>"
+      expect(subject.sanitize(html)).to eq ""
+    end
+
+    it "removes link tags" do
+      html = "<link rel='stylesheet' href='http://malicious-site.com/style.css'>"
+      expect(subject.sanitize(html)).to eq ""
+    end
+
+    it "removes link tags" do
+      html = "<base href='http://phishing-site.com/'>"
+      expect(subject.sanitize(html)).to eq ""
+    end
+
+    it "removes form tags" do
+      html = "<form action='http://malicious-site.com/submit' method='post'>...</form>"
+      expect(subject.sanitize(html)).to eq "..."
+    end
+
+    it "removes combined dangerous tags" do
+      html = "<script>alert</script><iframe scr='http://malicious-site.com'></iframe>"
+      expect(subject.sanitize(html)).to eq "alert"
+    end
   end
 
-  it "keeps supported attributes" do
-    html = 'Hello <a href="#focus">alert</a>!'
-    expect(subject.sanitize(html))
-      .to eq 'Hello <a href="#focus">alert</a>!'
+  context "when HTML has supported attributes" do
+    it "keeps supported attributes" do
+      html = 'Hello <a href="#focus">alert</a>!'
+      expect(subject.sanitize(html))
+        .to eq 'Hello <a href="#focus">alert</a>!'
+    end
   end
 
-  it "removes unsupported attributes" do
-    html = 'Hello <a href="#focus" onclick="alert">alert</a>!'
-    expect(subject.sanitize(html))
-      .to eq 'Hello <a href="#focus">alert</a>!'
-  end
+  context "when HTML has dangerous attributes" do
+    it "removes unsupported attributes" do
+      html = 'Hello <a href="#focus" onclick="alert">alert</a>!'
+      expect(subject.sanitize(html))
+        .to eq 'Hello <a href="#focus">alert</a>!'
+    end
 
-  it "removes dangerous attribute values" do
-    html = 'Hello <a href="javascript:alert(\"boo!\")">you</a>!'
-    expect(subject.sanitize(html))
-      .to eq 'Hello <a>you</a>!'
+    it "removes dangerous attribute values" do
+      html = 'Hello <a href="javascript:alert(\"boo!\")">you</a>!'
+      expect(subject.sanitize(html))
+        .to eq 'Hello <a>you</a>!'
+    end
+
+    it "keeps only Trix-specific data attributes" do
+      html = '<figure data-trix-attachment="{...}" data-script="">...</figure>'
+      expect(subject.sanitize(html)).to eq('<figure data-trix-attachment="{...}">...</figure>')
+    end
   end
 end