fix style-sanitizing regex

in commit 8ae4ba8e, the regex was modified to accept negative
values (e.g., -1.0em), making the \w-\w clause redundant and
causing extremely slow matching in certain pathological cases.

test plan:
 - import the Angel package attached to the ticket
 - it should complete in a reasonable time (a few minutes,
   probably) and definitely should not max out your CPU
   for three hours while making no visible progress

fixes CNVS-10820

Change-Id: I582386c25d4d99f362ce2411b0c387bc958a71c5
Reviewed-on: https://gerrit.instructure.com/30074
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Bracken Mosbacker <bracken@instructure.com>
Product-Review: Bracken Mosbacker <bracken@instructure.com>
QA-Review: Nathan Rogowski <nathan@instructure.com>
This commit is contained in:
Jeremy Stanley 2014-02-12 16:30:26 -07:00
parent f38037b4b1
commit 6dbbd8d9f8
2 changed files with 7 additions and 1 deletions

View File

@ -32,7 +32,7 @@ module CanvasSanitize #:nodoc:
style = node['style'] || ""
# taken from https://github.com/flavorjones/loofah/blob/master/lib/loofah/html5/scrub.rb
# the gauntlet
style = '' unless style =~ /\A([-:,\;#%.\(\)\/\sa-zA-Z0-9!]|\w-\w|\'[\s\w]+\'|\"[\s\w]+\"|\([\d,\s]+\))*\z/
style = '' unless style =~ /\A([-:,\;#%.\(\)\/\sa-zA-Z0-9!]|\'[\s\w]+\'|\"[\s\w]+\"|\([\d,\s]+\))*\z/
style = '' unless style =~ /\A\s*([-\w]+\s*:[^\;]*(\;\s*|$))*\z/
config = env[:config]

View File

@ -106,6 +106,12 @@ describe Sanitize do
res.should == "<p>but not me</p>"
end
it "should not be extremely slow with long, weird microsoft styles" do
str = %{<span lang="EN" style="font-family: 'Times New Roman','serif'; color: #17375e; font-size: 12pt; mso-fareast-font-family: 'Times New Roman'; mso-themecolor: text2; mso-themeshade: 191; mso-style-textfill-fill-color: #17375E; mso-style-textfill-fill-themecolor: text2; mso-style-textfill-fill-alpha: 100.0%; mso-ansi-language: EN; mso-style-textfill-fill-colortransforms: lumm=75000"><p></p></span>}
# the above string took over a minute to sanitize as of 8ae4ba8e
Timeout.timeout(1) { Sanitize.clean(str, CanvasSanitize::SANITIZE) }
end
Dir.glob(Rails.root.join('spec', 'fixtures', 'xss', '*.xss')) do |filename|
name = File.split(filename).last
it "should sanitize xss attempts for #{name}" do