Merge pull request #12518 from anansilva/12448-sanitise-html-product-description

Sanitise HTML in product description [read-only]

app/models/spree/product.rb

@@ -304,6 +304,16 @@ module Spree
       )
     end
 
+    # Remove any unsupported HTML.
+    def description
+      HtmlSanitizer.sanitize(super)
+    end
+
+    # Remove any unsupported HTML.
+    def description=(html)
+      super(HtmlSanitizer.sanitize(html))
+    end
+
     private
 
     def update_units

app/services/html_sanitizer.rb

@@ -6,10 +6,16 @@
 # We offer an editor which supports certain tags but you can't insert just any
 # HTML, which would be dangerous.
 class HtmlSanitizer
+  # div is required by Trix editor
+  ALLOWED_TAGS = %w[h1 h2 h3 h4 div 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

spec/models/spree/product_spec.rb

@@ -748,6 +748,18 @@ module Spree
         expect(e.variants.reload).to be_empty
       end
     end
+
+    describe "serialisation" do
+      it "sanitises HTML in description" do
+        subject.description = "Hello <script>alert</script> dearest <b>monster</b>."
+        expect(subject.description).to eq "Hello alert dearest <b>monster</b>."
+      end
+
+      it "sanitises existing HTML in description" do
+        subject[:description] = "Hello <script>alert</script> dearest <b>monster</b>."
+        expect(subject.description).to eq "Hello alert dearest <b>monster</b>."
+      end
+    end
   end
 
   RSpec.describe "product import" do

spec/services/html_sanitizer_spec.rb

@@ -5,33 +5,97 @@ 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 regular tags" do
+      supported_tags = %w[h1 h2 h3 h4 div p b i u a strong em del pre blockquote ul ol li figure]
+      supported_tags.each do |tag|
+        html = "<#{tag}>Content</#{tag}>"
+        sanitized_html = subject.sanitize(html)
+        expect(sanitized_html).to eq(html), "Expected '#{tag}' to be preserved."
+      end
+    end
+
+    it "keeps supported void tags" do
+      supported_tags = %w[br hr]
+      supported_tags.each do |tag|
+        html = "<#{tag}>"
+        sanitized_html = subject.sanitize(html)
+        expect(sanitized_html).to eq(html), "Expected '#{tag}' to be preserved."
+      end
+    end
+
+    it "handles nested tags" do
+      html = '<div><ul><li>Item 1</li><li><strong>Item 2</strong></li></ul></div>'
+      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 base 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