From 37e0059adeea881f5c4bba2e6646777c29d67b53 Mon Sep 17 00:00:00 2001 From: Yechiel Kalmenson Date: Wed, 11 Jul 2018 15:04:03 -0400 Subject: [PATCH] open markdown links in new tab (take 2) (#512) * open links in chats in new tabs * fixed target_blank vulnerability * add test to make sur links are being generated with target _blank * add test for target blank vulnerability * add rel and target to the whitelisted attributes * add the link attributes to HTMLRouge#link --- app/labor/markdown_parser.rb | 4 ++-- app/lib/redcarpet/render/html_rouge.rb | 6 +++++- spec/labor/markdown_parser_spec.rb | 13 +++++++++++++ 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/app/labor/markdown_parser.rb b/app/labor/markdown_parser.rb index 389609214..796698f36 100644 --- a/app/labor/markdown_parser.rb +++ b/app/labor/markdown_parser.rb @@ -37,12 +37,12 @@ class MarkdownParser renderer_options = { hard_wrap: true, filter_html: false, - link_attributes: { rel: 'noopener noreferrer', target: "_blank" }, + link_attributes: { rel: "noopener noreferrer", target: "_blank" }, } renderer = Redcarpet::Render::HTMLRouge.new(renderer_options) markdown = Redcarpet::Markdown.new(renderer, REDCARPET_CONFIG) ActionController::Base.helpers.sanitize(markdown.render(@content).html_safe, - tags: %w(strong i u b em code a), attributes: ["href"]) + tags: %w(strong i u b em code a), attributes: %w(href rel target)) end def tags_used diff --git a/app/lib/redcarpet/render/html_rouge.rb b/app/lib/redcarpet/render/html_rouge.rb index 4bc9ecfa9..a5706f7c9 100644 --- a/app/lib/redcarpet/render/html_rouge.rb +++ b/app/lib/redcarpet/render/html_rouge.rb @@ -9,7 +9,11 @@ module Redcarpet # Probably not the best fix but it does it's job of preventing # a nested links. return nil if //.match?(content) - %(#{content}) + link_attributes = "" + @options[:link_attributes]&.each do |attribute, value| + link_attributes += %( #{attribute}="#{value}") + end + %(#{content}) end end end diff --git a/spec/labor/markdown_parser_spec.rb b/spec/labor/markdown_parser_spec.rb index 7155a94e1..6c5e4e71e 100644 --- a/spec/labor/markdown_parser_spec.rb +++ b/spec/labor/markdown_parser_spec.rb @@ -22,6 +22,19 @@ RSpec.describe MarkdownParser do expect(generate_and_parse_markdown(inline_code)).to include(inline_code[1..-2]) end + context "when provided with a link in inline code" do + inline_code = "[dev.to](https://dev.to)" + let(:evaluated_markdown) { described_class.new(inline_code).evaluate_inline_markdown } + + it "renders with target _blank" do + expect(evaluated_markdown).to include("target=\"_blank\"") + end + + it "avoids the traget _blank vulnerability" do + expect(evaluated_markdown).to include("noopener", "noreferrer") + end + end + context "when provided with an @username" do it "links to a user if user exist" do username = create(:user).username