diff --git a/app/helpers/html_helper.rb b/app/helpers/html_helper.rb index 218d58e78..a3d0bbab3 100644 --- a/app/helpers/html_helper.rb +++ b/app/helpers/html_helper.rb @@ -1,64 +1,5 @@ module HtmlHelper - include ERB::Util - - EXCLUDE_PUNCTUATION = %(.?,:!;"'<>) - EXCLUDE_PUNCTUATION_REGEX = /[#{Regexp.escape(EXCLUDE_PUNCTUATION)}]+\z/ - def format_html(html) - fragment = Nokogiri::HTML5.fragment(html) - - auto_link(fragment) - - fragment.to_html.html_safe + Loofah::HTML5::DocumentFragment.parse(html).scrub!(AutoLinkScrubber.new).to_html.html_safe end - - private - EXCLUDED_ELEMENTS = %w[ a figcaption pre code ] - EMAIL_AUTOLINK_REGEXP = /\b[a-zA-Z0-9.!#$%&'*+\/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*\b/ - URL_REGEXP = URI::DEFAULT_PARSER.make_regexp(%w[http https]) - - def auto_link(fragment) - fragment.traverse do |node| - next unless auto_linkable_node?(node) - - # Take care to escape the html text node, so that the subsequent Nokogiri re-parse doesn't - # create tags where there aren't any. - content = h(node.text) - linked_content = content.dup - - auto_link_urls(linked_content) - auto_link_emails(linked_content) - - if linked_content != content - node.replace(Nokogiri::HTML5.fragment(linked_content)) - end - end - end - - def auto_linkable_node?(node) - node.text? && node.ancestors.none? { |ancestor| EXCLUDED_ELEMENTS.include?(ancestor.name) } - end - - def auto_link_urls(linked_content) - linked_content.gsub!(URL_REGEXP) do |match| - url, trailing_punct = extract_url_and_punctuation(match) - %(#{url}#{trailing_punct}) - end - end - - def extract_url_and_punctuation(url_match) - url_match = CGI.unescapeHTML(url_match) - if match = url_match.match(EXCLUDE_PUNCTUATION_REGEX) - len = match[0].length - [ url_match[..-(len+1)], url_match[-len..] ] - else - [ url_match, "" ] - end - end - - def auto_link_emails(text) - text.gsub!(EMAIL_AUTOLINK_REGEXP) do |match| - %(#{match}) - end - end end diff --git a/lib/auto_link_scrubber.rb b/lib/auto_link_scrubber.rb new file mode 100644 index 000000000..5ac9f2f3d --- /dev/null +++ b/lib/auto_link_scrubber.rb @@ -0,0 +1,79 @@ +# Loofah scrubber to auto-link URLs and email addresses in HTML text nodes. +# +# This scrubber does not perform HTML sanitization; it's assumed that the input is already sanitized +# (for example, ActionText rich text). +class AutoLinkScrubber < Loofah::Scrubber + EXCLUDED_ELEMENTS = %w[a figcaption pre code].freeze + + # This regexp is similar to URI::MailTo::EMAIL_REGEXP but uses \b word boundaries instead of \A/\z + # anchors, allowing it to match email addresses embedded within longer strings. + # + # It's named EMAIL_AUTOLINK_REGEXP (not EMAIL_REGEXP) to avoid confusing Brakeman's imprecise + # constant lookup, which otherwise assumes Identity's email validation uses this \b-anchored pattern. + # See https://github.com/presidentbeef/brakeman/pull/1981 + EMAIL_AUTOLINK_REGEXP = /\b[a-zA-Z0-9.!#$%&'*+\/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*\b/ + URL_REGEXP = URI::DEFAULT_PARSER.make_regexp(%w[http https]) + AUTOLINK_REGEXP = /(?#{URL_REGEXP})|(?#{EMAIL_AUTOLINK_REGEXP})/ + + TRAILING_PUNCTUATION = %(.?,:!;"'<>) + TRAILING_PUNCTUATION_REGEXP = /[#{Regexp.escape(TRAILING_PUNCTUATION)}]+\z/ + + def initialize + @direction = :top_down + end + + def scrub(node) + return Loofah::Scrubber::STOP if EXCLUDED_ELEMENTS.include?(node.name) + + if node.text? + replacement = autolink_text_node(node) + node.replace(replacement) if replacement + end + + Loofah::Scrubber::CONTINUE + end + + private + def autolink_text_node(node) + text = node.text + links = find_links(text) + + return nil if links.empty? + + doc = node.document + nodes = Nokogiri::XML::NodeSet.new(doc) + pos = 0 + + links.each do |link| + nodes << doc.create_text_node(text[pos...link[:start]]) if link[:start] > pos + nodes << doc.create_element("a", link[:text], href: link[:href], rel: "noopener noreferrer") + pos = link[:start] + link[:length] + end + nodes << doc.create_text_node(text[pos..]) if pos < text.length + + nodes + end + + def find_links(text) + links = [] + + text.scan(AUTOLINK_REGEXP) do + match_data = Regexp.last_match + start_pos = match_data.begin(0) + + if match_data[:url] + url = clean_url(match_data[:url]) + links << { start: start_pos, length: url.length, text: url, href: url } + else + email = match_data[:email] + links << { start: start_pos, length: email.length, text: email, href: "mailto:#{email}" } + end + end + + links + end + + def clean_url(url) + url.sub(TRAILING_PUNCTUATION_REGEXP, "") + end +end diff --git a/test/helpers/html_helper_test.rb b/test/helpers/html_helper_test.rb index 4011cbf70..1af507ddf 100644 --- a/test/helpers/html_helper_test.rb +++ b/test/helpers/html_helper_test.rb @@ -3,71 +3,86 @@ require "test_helper" class HtmlHelperTest < ActionView::TestCase test "convert URLs into anchor tags" do assert_equal_html \ - %(

Check this: https://example.com

), + %(

Check this: https://example.com

), format_html("

Check this: https://example.com

") assert_equal_html \ - %(

Check this: https://example.com/

), + %(

Check this: https://example.com/

), format_html("

Check this: https://example.com/

") end + test "convert multiple URLs in the same string" do + assert_equal_html \ + %(Visit https://foo.com/. Also see https://bar.com/!), + format_html("Visit https://foo.com/. Also see https://bar.com/!") + end + test "don't include punctuation in URL autolinking" do assert_equal_html \ - %(

Check this: https://example.com/!

), + %(

Check this: https://example.com/!

), format_html("

Check this: https://example.com/!

") assert_equal_html \ - %(

Check this: https://example.com/.

), + %(

Check this: https://example.com/.

), format_html("

Check this: https://example.com/.

") assert_equal_html \ - %(

Check this: https://example.com/?

), + %(

Check this: https://example.com/?

), format_html("

Check this: https://example.com/?

") assert_equal_html \ - %(

Check this: https://example.com/,

), + %(

Check this: https://example.com/,

), format_html("

Check this: https://example.com/,

") assert_equal_html \ - %(

Check this: https://example.com/:

), + %(

Check this: https://example.com/:

), format_html("

Check this: https://example.com/:

") assert_equal_html \ - %(

Check this: https://example.com/;

), + %(

Check this: https://example.com/;

), format_html("

Check this: https://example.com/;

") assert_equal_html \ - %(

Check this: https://example.com/"

), + %(

Check this: https://example.com/"

), format_html("

Check this: https://example.com/\"

") assert_equal_html \ - %(

Check this: https://example.com/'

), + %(

Check this: https://example.com/'

), format_html("

Check this: https://example.com/'

") # trailing entities that decode to punctuation # use assert_equal and not assert_equal_html to make sure we're getting entities correct assert_equal \ - %(

Check this: https://example.com/<

), + %(

Check this: https://example.com/<

), format_html("

Check this: https://example.com/<

") assert_equal \ - %(

Check this: https://example.com/>

), + %(

Check this: https://example.com/>

), format_html("

Check this: https://example.com/>

") assert_equal \ - %(

Check this: https://example.com/"

), + %(

Check this: https://example.com/"

), format_html("

Check this: https://example.com/"

") # multiple punctuation characters including entities assert_equal_html \ - %(

Check this: https://example.com/!?;

), + %(

Check this: https://example.com/!?;

), format_html("

Check this: https://example.com/!?;

") assert_equal_html \ - %(<img src="https://example.com/">), + %(<img src="https://example.com/">), format_html(%(<img src="https://example.com/">)) assert_equal_html \ - %(<img src="https://example.com/"!>), + %(<img src="https://example.com/"!>), format_html(%(<img src="https://example.com/"!>)) end + test "make sure the linked content is properly sanitized" do + # https://hackerone.com/reports/3481093 + result = format_html(%(https://google.com/\">test</a><input></input>)) + assert_no_match(//i, result, "should not create an input element") + + result = format_html(%(https://google.com/\"><script>alert('xss')</script>)) + assert_no_match(/