Merge pull request #3160 from evazion/feat-note-sanitization

Improve note sanitization

app/controllers/note_previews_controller.rb

@@ -2,7 +2,7 @@ class NotePreviewsController < ApplicationController
   respond_to :json
 
   def show
-    @body = DText.sanitize(params[:body].to_s)
+    @body = NoteSanitizer.sanitize(params[:body].to_s)
     respond_with(@body) do |format|
       format.json do
         render :json => {:body => @body}.to_json

app/logical/d_text.rb

@@ -369,30 +369,6 @@ class DText
     s
   end
 
-  def self.sanitize(text)
-    text.gsub!(/<( |-|3|:|>|\Z)/, "&lt;\\1")
-
-    Sanitize.clean(
-      text,
-      :elements => %w(code center tn h1 h2 h3 h4 h5 h6 a span div blockquote br p ul li ol em strong small big b i font u s pre ruby rb rt rp),
-      :attributes => {
-        "a" => %w(href title style),
-        "span" => %w(class style),
-        "div" => %w(class style align),
-        "p" => %w(class style align),
-        "font" => %w(color size style)
-      },
-      :protocols => {
-        "a" => {
-          "href" => ["http", "https", :relative]
-        }
-      },
-      :css => Sanitize::Config::RELAXED[:css].merge({
-        :protocols => []
-      })
-    )
-  end
-
   # extract the first paragraph `needle` occurs in.
   def self.excerpt(dtext, needle)
     dtext = dtext.gsub(/\r\n|\r|\n/, "\n")

app/logical/note_sanitizer.rb

@@ -0,0 +1,79 @@
+module NoteSanitizer
+  ALLOWED_ELEMENTS = %w(
+    code center tn h1 h2 h3 h4 h5 h6 a span div blockquote br p ul li ol em
+    strong small big b i font u s pre ruby rb rt rp rtc sub sup hr wbr
+  )
+
+  ALLOWED_ATTRIBUTES = {
+    :all => %w(style title),
+    "a" => %w(href),
+    "span" => %w(class),
+    "div" => %w(class align),
+    "p" => %w(class align),
+    "font" => %w(color size),
+  }
+
+  ALLOWED_PROPERTIES = %w(
+    background background-color
+    border border-color border-image border-radius border-style border-width
+    border-bottom border-bottom-color border-bottom-left-radius border-bottom-right-radius border-bottom-style border-bottom-width
+    border-left border-left-color border-left-style border-left-width
+    border-right border-right-color border-right-style border-right-width
+    border-top border-top-color border-top-left-radious border-top-right-radius border-top-style border-top-width
+    bottom left right top
+    box-shadow
+    clear
+    color
+    display
+    filter
+    float
+    font font-family font-size font-size-adjust font-style font-variant font-weight
+    height width
+    letter-spacing
+    line-height
+    list-style list-style-position list-style-type
+    margin margin-bottom margin-left margin-right margin-top
+    opacity
+    outline outline-color outline-offset outline-width outline-style
+    padding padding-bottom padding-left padding-right padding-top
+    perspective perspective-origin
+    position
+    text-align
+    text-decoration text-decoration-color text-decoration-line text-decoration-style
+    text-indent
+    text-shadow
+    text-transform
+    transform transform-origin
+    white-space
+    word-break
+    word-spacing
+    word-wrap overflow-wrap
+    writing-mode
+    vertical-align
+  )
+
+  def self.sanitize(text)
+    text.gsub!(/<( |-|3|:|>|\Z)/, "&lt;\\1")
+
+    Sanitize.clean(
+      text,
+      :elements => ALLOWED_ELEMENTS,
+      :attributes => ALLOWED_ATTRIBUTES,
+      :add_attributes => {
+        "a" => { "rel" => "nofollow" },
+      },
+      :protocols => {
+        "a" => {
+          "href" => ["http", "https", :relative]
+        }
+      },
+      :css => {
+        allow_comments: false,
+        allow_hacks: false,
+        at_rules: [],
+        protocols: [],
+        properties: ALLOWED_PROPERTIES,
+      }
+    )
+  end
+end

app/views/notes/_note.html.erb

@@ -1 +1 @@
-<article data-width="<%= note.width %>" data-height="<%= note.height %>" data-x="<%= note.x %>" data-y="<%= note.y %>" data-id="<%= note.id %>" data-body="<%= note.body %>"><%= raw DText.sanitize(note.body) %></article>
+<article data-width="<%= note.width %>" data-height="<%= note.height %>" data-x="<%= note.x %>" data-y="<%= note.y %>" data-id="<%= note.id %>" data-body="<%= note.body %>"><%= raw NoteSanitizer.sanitize(note.body) %></article>

test/unit/note_sanitizer_test.rb

@@ -0,0 +1,25 @@
+require 'test_helper'
+
+class NoteSanitizerTest < ActiveSupport::TestCase
+  context "Sanitizing a note" do
+    should "strip unsafe tags" do
+      body = '<p>test</p> <script>alert("owned")</script>'
+      assert_equal('<p>test</p> alert("owned")', NoteSanitizer.sanitize(body))
+    end
+
+    should "strip unsafe css" do
+      body = '<p style="background-image: url(http://www.google.com);">test</p>'
+      assert_equal("<p>test</p>", NoteSanitizer.sanitize(body))
+    end
+
+    should "allow style attributes on every tag" do
+      body = '<p style="font-size: 1em;">test</p>'
+      assert_equal('<p style="font-size: 1em;">test</p>', NoteSanitizer.sanitize(body))
+    end
+
+    should "mark links as nofollow" do
+      body = '<a href="http://www.google.com">google</a>'
+      assert_equal('<a href="http://www.google.com" rel="nofollow">google</a>', NoteSanitizer.sanitize(body))
+    end
+  end
+end