From 990f173b3d1159f7c76ad4cc67aa28b12315cda7 Mon Sep 17 00:00:00 2001 From: evazion Date: Thu, 15 Jun 2017 20:30:19 -0500 Subject: [PATCH 1/6] notes: move sanitization from d_text.rb to note_sanitizer.rb. --- app/controllers/note_previews_controller.rb | 2 +- app/logical/d_text.rb | 24 -------------------- app/logical/note_sanitizer.rb | 25 +++++++++++++++++++++ app/views/notes/_note.html.erb | 2 +- 4 files changed, 27 insertions(+), 26 deletions(-) create mode 100644 app/logical/note_sanitizer.rb diff --git a/app/controllers/note_previews_controller.rb b/app/controllers/note_previews_controller.rb index 81c464689..e14f0b1c5 100644 --- a/app/controllers/note_previews_controller.rb +++ b/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 diff --git a/app/logical/d_text.rb b/app/logical/d_text.rb index 78aaf56d0..0ac557ee3 100644 --- a/app/logical/d_text.rb +++ b/app/logical/d_text.rb @@ -369,30 +369,6 @@ class DText s end - def self.sanitize(text) - text.gsub!(/<( |-|3|:|>|\Z)/, "<\\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") diff --git a/app/logical/note_sanitizer.rb b/app/logical/note_sanitizer.rb new file mode 100644 index 000000000..baf53bfd0 --- /dev/null +++ b/app/logical/note_sanitizer.rb @@ -0,0 +1,25 @@ +module NoteSanitizer + def self.sanitize(text) + text.gsub!(/<( |-|3|:|>|\Z)/, "<\\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 +end diff --git a/app/views/notes/_note.html.erb b/app/views/notes/_note.html.erb index fcf1b3dff..15e374b34 100644 --- a/app/views/notes/_note.html.erb +++ b/app/views/notes/_note.html.erb @@ -1 +1 @@ -
<%= raw DText.sanitize(note.body) %>
+
<%= raw NoteSanitizer.sanitize(note.body) %>
From 85e32b5eb29a5dcf91eae40f4c79945d82d5026b Mon Sep 17 00:00:00 2001 From: evazion Date: Thu, 15 Jun 2017 12:51:37 -0500 Subject: [PATCH 2/6] notes: add sanitization tests. --- test/unit/note_sanitizer_test.rb | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 test/unit/note_sanitizer_test.rb diff --git a/test/unit/note_sanitizer_test.rb b/test/unit/note_sanitizer_test.rb new file mode 100644 index 000000000..8a089825d --- /dev/null +++ b/test/unit/note_sanitizer_test.rb @@ -0,0 +1,15 @@ +require 'test_helper' + +class NoteSanitizerTest < ActiveSupport::TestCase + context "Sanitizing a note" do + should "strip unsafe tags" do + body = '

test

' + assert_equal('

test

alert("owned")', NoteSanitizer.sanitize(body)) + end + + should "strip unsafe css" do + body = '

test

' + assert_equal("

test

", NoteSanitizer.sanitize(body)) + end + end +end From 869ccad6ba87d8d7ca848b0254a59065b7456bf8 Mon Sep 17 00:00:00 2001 From: evazion Date: Thu, 15 Jun 2017 20:34:38 -0500 Subject: [PATCH 3/6] notes: allow all elements to have style/title attributes. --- app/logical/note_sanitizer.rb | 24 ++++++++++++++++-------- test/unit/note_sanitizer_test.rb | 5 +++++ 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/app/logical/note_sanitizer.rb b/app/logical/note_sanitizer.rb index baf53bfd0..ad486bdf4 100644 --- a/app/logical/note_sanitizer.rb +++ b/app/logical/note_sanitizer.rb @@ -1,17 +1,25 @@ 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 + ) + + ALLOWED_ATTRIBUTES = { + :all => %w(style title), + "a" => %w(href), + "span" => %w(class), + "div" => %w(class align), + "p" => %w(class align), + "font" => %w(color size), + } + def self.sanitize(text) text.gsub!(/<( |-|3|:|>|\Z)/, "<\\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) - }, + :elements => ALLOWED_ELEMENTS, + :attributes => ALLOWED_ATTRIBUTES, :protocols => { "a" => { "href" => ["http", "https", :relative] diff --git a/test/unit/note_sanitizer_test.rb b/test/unit/note_sanitizer_test.rb index 8a089825d..3394219ea 100644 --- a/test/unit/note_sanitizer_test.rb +++ b/test/unit/note_sanitizer_test.rb @@ -11,5 +11,10 @@ class NoteSanitizerTest < ActiveSupport::TestCase body = '

test

' assert_equal("

test

", NoteSanitizer.sanitize(body)) end + + should "allow style attributes on every tag" do + body = '

test

' + assert_equal('

test

', NoteSanitizer.sanitize(body)) + end end end From 9570bf026c3239a022d3a0e2745a0346cb4044b4 Mon Sep 17 00:00:00 2001 From: evazion Date: Thu, 15 Jun 2017 20:36:07 -0500 Subject: [PATCH 4/6] notes: allow , , ,
, tags. --- app/logical/note_sanitizer.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/logical/note_sanitizer.rb b/app/logical/note_sanitizer.rb index ad486bdf4..904a09c24 100644 --- a/app/logical/note_sanitizer.rb +++ b/app/logical/note_sanitizer.rb @@ -1,7 +1,7 @@ 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 + strong small big b i font u s pre ruby rb rt rp rtc sub sup hr wbr ) ALLOWED_ATTRIBUTES = { From 845b278b1e214d3dd133bef20c31ad7c3d4cc00d Mon Sep 17 00:00:00 2001 From: evazion Date: Thu, 15 Jun 2017 20:36:58 -0500 Subject: [PATCH 5/6] notes: make allowed css properties explicit. --- app/logical/note_sanitizer.rb | 49 ++++++++++++++++++++++++++++++++--- 1 file changed, 46 insertions(+), 3 deletions(-) diff --git a/app/logical/note_sanitizer.rb b/app/logical/note_sanitizer.rb index 904a09c24..e786dc1f2 100644 --- a/app/logical/note_sanitizer.rb +++ b/app/logical/note_sanitizer.rb @@ -13,6 +13,45 @@ module NoteSanitizer "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)/, "<\\1") @@ -25,9 +64,13 @@ module NoteSanitizer "href" => ["http", "https", :relative] } }, - :css => Sanitize::Config::RELAXED[:css].merge({ - :protocols => [] - }) + :css => { + allow_comments: false, + allow_hacks: false, + at_rules: [], + protocols: [], + properties: ALLOWED_PROPERTIES, + } ) end end From 91ed7931202e085e2dc77e725ea853d60a8c22c9 Mon Sep 17 00:00:00 2001 From: evazion Date: Thu, 15 Jun 2017 20:37:32 -0500 Subject: [PATCH 6/6] notes: mark links as nofollow. --- app/logical/note_sanitizer.rb | 3 +++ test/unit/note_sanitizer_test.rb | 5 +++++ 2 files changed, 8 insertions(+) diff --git a/app/logical/note_sanitizer.rb b/app/logical/note_sanitizer.rb index e786dc1f2..27ca58345 100644 --- a/app/logical/note_sanitizer.rb +++ b/app/logical/note_sanitizer.rb @@ -59,6 +59,9 @@ module NoteSanitizer text, :elements => ALLOWED_ELEMENTS, :attributes => ALLOWED_ATTRIBUTES, + :add_attributes => { + "a" => { "rel" => "nofollow" }, + }, :protocols => { "a" => { "href" => ["http", "https", :relative] diff --git a/test/unit/note_sanitizer_test.rb b/test/unit/note_sanitizer_test.rb index 3394219ea..e87ed67d7 100644 --- a/test/unit/note_sanitizer_test.rb +++ b/test/unit/note_sanitizer_test.rb @@ -16,5 +16,10 @@ class NoteSanitizerTest < ActiveSupport::TestCase body = '

test

' assert_equal('

test

', NoteSanitizer.sanitize(body)) end + + should "mark links as nofollow" do + body = 'google' + assert_equal('google', NoteSanitizer.sanitize(body)) + end end end