Merge pull request #392 from basecamp/sanitize-edit-history-xss

Sanitize markdown output in edit history and TOC edit views

Author: djmb

app/views/leaves/_edit.html.erb

@@ -6,7 +6,7 @@
 
   <%= link_to leafable_path(leaf), class: "toc__thumbnail", data: { turbo_frame: "_top" } do %>
     <%= leaf.section.body if leaf.section? %>
-    <%= leaf.leafable.body.to_html if leaf.page? %>
+    <%= sanitize_content(leaf.leafable.body.to_html) if leaf.page? %>
     <%= image_tag leaf.leafable.image.variant(:large) if leaf.picture&.image&.attached? %>
   <% end %>
 

app/views/pages/edits/show.html.erb

@@ -51,7 +51,7 @@
         <% end %>
       </header>
 
-      <%= @edit.page.body.to_html %>
+      <%= sanitize_content(@edit.page.body.to_html) %>
     <% end %>
   </section>
 
@@ -62,6 +62,6 @@
       <% end %>
     </header>
 
-    <%= @leaf.page.body.to_html %>
+    <%= sanitize_content(@leaf.page.body.to_html) %>
   </section>
 </div>

test/controllers/pages/edits_controller_test.rb

@@ -23,4 +23,25 @@ class Pages::EditsControllerTest < ActionDispatch::IntegrationTest
     assert_response :success
     assert_select "p", /such a great handbook/
   end
+
+  test "show sanitizes dangerous content in previous version" do
+    leaf = books(:handbook).press Page.new(body: %(<img src=x onerror="alert(1)">)), title: "XSS Test"
+    leaf.edit leafable_params: { body: "Clean content now" }
+
+    get page_edit_url(leaf, leaf.edits.last)
+
+    assert_response :success
+    assert_match '<img src="x">', response.body
+    assert_no_match(/onerror/, response.body)
+  end
+
+  test "show sanitizes dangerous content in current version" do
+    leaves(:welcome_page).edit leafable_params: { body: %(<img src=x onerror="alert(1)">) }
+
+    get page_edit_url(leaves(:welcome_page), leaves(:welcome_page).edits.last)
+
+    assert_response :success
+    assert_match '<img src="x">', response.body
+    assert_no_match(/onerror/, response.body)
+  end
 end