From de6129b29ca5b3634291429a911cf188d68ea2ad Mon Sep 17 00:00:00 2001 From: rhymes Date: Fri, 20 Sep 2019 17:28:34 +0300 Subject: [PATCH] Replace ^ and $ with \A and \z in Ruby regexps (#4072) --- app/helpers/application_helper.rb | 2 +- app/liquid_tags/codepen_tag.rb | 4 ++-- app/liquid_tags/glitch_tag.rb | 6 ++++-- app/liquid_tags/jsfiddle_tag.rb | 6 ++++-- app/liquid_tags/kotlin_tag.rb | 3 ++- app/liquid_tags/stackblitz_tag.rb | 9 ++++++--- app/liquid_tags/stackexchange_tag.rb | 3 ++- app/liquid_tags/tweet_tag.rb | 4 ++-- spec/helpers/application_helper_spec.rb | 18 ++++++++++++++++++ spec/liquid_tags/codepen_tag_spec.rb | 9 +++++++++ spec/liquid_tags/kotlin_tag_spec.rb | 8 ++++---- spec/liquid_tags/stackexchange_tag_spec.rb | 2 ++ 12 files changed, 56 insertions(+), 18 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 315340946..63bf43987 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -121,7 +121,7 @@ module ApplicationHelper end def beautified_url(url) - url.sub(/^((http[s]?|ftp):\/)?\//, "").sub(/\?.*/, "").chomp("/") + url.sub(/\A((http[s]?|ftp):\/)?\//, "").sub(/\?.*/, "").chomp("/") rescue StandardError url end diff --git a/app/liquid_tags/codepen_tag.rb b/app/liquid_tags/codepen_tag.rb index dbf349bf3..ba36d2d3d 100644 --- a/app/liquid_tags/codepen_tag.rb +++ b/app/liquid_tags/codepen_tag.rb @@ -1,5 +1,6 @@ class CodepenTag < LiquidTagBase PARTIAL = "liquids/codepen".freeze + URL_REGEXP = /\A(http|https):\/\/(codepen\.io|codepen\.io\/team)\/[a-zA-Z0-9_\-]{1,30}\/pen\/([a-zA-Z]{5,7})\/{0,1}\z/.freeze def initialize(tag_name, link, tokens) super @@ -46,8 +47,7 @@ class CodepenTag < LiquidTagBase def valid_link?(link) link_no_space = link.delete(" ") - (link_no_space =~ - /^(http|https):\/\/(codepen\.io|codepen\.io\/team)\/[a-zA-Z0-9_\-]{1,30}\/pen\/([a-zA-Z]{5,7})\/{0,1}\z/)&.zero? + (link_no_space =~ URL_REGEXP)&.zero? end def raise_error diff --git a/app/liquid_tags/glitch_tag.rb b/app/liquid_tags/glitch_tag.rb index 7e099963d..2afc5d817 100644 --- a/app/liquid_tags/glitch_tag.rb +++ b/app/liquid_tags/glitch_tag.rb @@ -3,6 +3,8 @@ require "uri" class GlitchTag < LiquidTagBase attr_accessor :uri PARTIAL = "liquids/glitch".freeze + ID_REGEXP = /\A[a-zA-Z0-9\-]{1,110}\z/.freeze + OPTION_REGEXP = /(app|code|no-files|preview-first|no-attribution|file\=\w(\.\w)?)/.freeze def initialize(tag_name, id, tokens) super @@ -23,7 +25,7 @@ class GlitchTag < LiquidTagBase private def valid_id?(input) - (input =~ /^[a-zA-Z0-9\-]{1,110}$/)&.zero? + (input =~ ID_REGEXP)&.zero? end def parse_id(input) @@ -34,7 +36,7 @@ class GlitchTag < LiquidTagBase end def valid_option(option) - option.match(/(app|code|no-files|preview-first|no-attribution|file\=\w(\.\w)?)/) + option.match(OPTION_REGEXP) end def option_to_query_pair(option) diff --git a/app/liquid_tags/jsfiddle_tag.rb b/app/liquid_tags/jsfiddle_tag.rb index 1d76b8cb4..5b2a53084 100644 --- a/app/liquid_tags/jsfiddle_tag.rb +++ b/app/liquid_tags/jsfiddle_tag.rb @@ -1,5 +1,7 @@ class JSFiddleTag < LiquidTagBase PARTIAL = "liquids/jsfiddle".freeze + OPTION_REGEXP = /\A(js|html|css|result|,)*\z/.freeze + LINK_REGEXP = /\A(http|https):\/\/(jsfiddle\.net)\/[a-zA-Z0-9\-\/]*\z/.freeze def initialize(tag_name, link, tokens) super @@ -21,7 +23,7 @@ class JSFiddleTag < LiquidTagBase private def valid_option(option) - option.match(/^(js|html|css|result|,)*\z/) + option.match(OPTION_REGEXP) end def parse_options(input) @@ -45,7 +47,7 @@ class JSFiddleTag < LiquidTagBase def valid_link?(link) link_no_space = link.delete(" ") - (link_no_space =~ /^(http|https):\/\/(jsfiddle\.net)\/[a-zA-Z0-9\-\/]*\z/).zero? + (link_no_space =~ LINK_REGEXP).zero? end end diff --git a/app/liquid_tags/kotlin_tag.rb b/app/liquid_tags/kotlin_tag.rb index 47283fc4a..5e328d47e 100644 --- a/app/liquid_tags/kotlin_tag.rb +++ b/app/liquid_tags/kotlin_tag.rb @@ -1,5 +1,6 @@ class KotlinTag < LiquidTagBase PARTIAL = "liquids/kotlin".freeze + PARAM_REGEXP = /\A[a-zA-Z0-9]+\z/.freeze def initialize(tag_name, link, tokens) super @@ -44,7 +45,7 @@ class KotlinTag < LiquidTagBase end def self.valid_param?(value) - !value&.match(/^[a-zA-Z0-9]+$/)&.nil? + !value&.match(PARAM_REGEXP)&.nil? end def raise_error diff --git a/app/liquid_tags/stackblitz_tag.rb b/app/liquid_tags/stackblitz_tag.rb index 0dba89b41..0d59662a6 100644 --- a/app/liquid_tags/stackblitz_tag.rb +++ b/app/liquid_tags/stackblitz_tag.rb @@ -1,5 +1,8 @@ class StackblitzTag < LiquidTagBase PARTIAL = "liquids/stackblitz".freeze + ID_REGEXP = /\A[a-zA-Z0-9\-]{0,60}\z/.freeze + VIEW_OPTION_REGEXP = /\Aview=(preview|editor|both)\z/.freeze + FILE_OPTION_REGEXP = /\Afile=(.*)\z/.freeze def initialize(tag_name, id, tokens) super @@ -24,7 +27,7 @@ class StackblitzTag < LiquidTagBase private def valid_id?(id) - id =~ /\A[a-zA-Z0-9\-]{0,60}\Z/ + id =~ ID_REGEXP end def parse_id(input) @@ -45,11 +48,11 @@ class StackblitzTag < LiquidTagBase end def valid_view?(option) - option.match(/^view=(preview|editor|both)\z/) + option.match(VIEW_OPTION_REGEXP) end def valid_file?(option) - option.match(/^file=(.*)\z/) + option.match(FILE_OPTION_REGEXP) end end diff --git a/app/liquid_tags/stackexchange_tag.rb b/app/liquid_tags/stackexchange_tag.rb index dddf4cb4f..0fcb6bd84 100644 --- a/app/liquid_tags/stackexchange_tag.rb +++ b/app/liquid_tags/stackexchange_tag.rb @@ -9,6 +9,7 @@ class StackexchangeTag < LiquidTagBase "question" => "!*1SgQGDOL9bPBHULz9sKS.y6qv7V9fYNszvdhDuv5", "site" => "!mWxO_PNa4i" }.freeze + ID_REGEXP = /\A\d{1,20}\z/.freeze attr_reader :site, :post_type @@ -54,7 +55,7 @@ class StackexchangeTag < LiquidTagBase def valid_input?(input) return false if input.nil? - /^\d{1,20}$/.match?(input.split(" ")[0]) + ID_REGEXP.match?(input.split(" ")[0]) end def handle_response_error(response) diff --git a/app/liquid_tags/tweet_tag.rb b/app/liquid_tags/tweet_tag.rb index b7ae649e7..0596374ec 100644 --- a/app/liquid_tags/tweet_tag.rb +++ b/app/liquid_tags/tweet_tag.rb @@ -1,6 +1,7 @@ class TweetTag < LiquidTagBase include ActionView::Helpers::AssetTagHelper PARTIAL = "liquids/tweet".freeze + ID_REGEXP = /\A\d{10,20}\z/.freeze # id must be all numbers between 10 and 20 chars def initialize(tag_name, id, tokens) super @@ -53,8 +54,7 @@ class TweetTag < LiquidTagBase end def valid_id?(id) - # id must be all numbers under 20 characters - /^\d{10,20}$/.match?(id) + ID_REGEXP.match?(id) end end diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index b4e21a8f3..d60c6a660 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -7,4 +7,22 @@ RSpec.describe ApplicationHelper, type: :helper do expect(helper.community_qualified_name).to eq(expected_name) end end + + describe "#beautified_url" do + it "strips the protocol" do + expect(helper.beautified_url("https://github.com")).to eq("github.com") + end + + it "strips params" do + expect(helper.beautified_url("https://github.com?a=3")).to eq("github.com") + end + + it "strips the last forward slash" do + expect(helper.beautified_url("https://github.com/")).to eq("github.com") + end + + it "does not strip the path" do + expect(helper.beautified_url("https://github.com/rails")).to eq("github.com/rails") + end + end end diff --git a/spec/liquid_tags/codepen_tag_spec.rb b/spec/liquid_tags/codepen_tag_spec.rb index 965bec050..7670b08e3 100644 --- a/spec/liquid_tags/codepen_tag_spec.rb +++ b/spec/liquid_tags/codepen_tag_spec.rb @@ -68,5 +68,14 @@ RSpec.describe CodepenTag, type: :liquid_template do expect { generate_new_liquid(link) }.to raise_error(StandardError) end end + + it "rejects multiline XSS attempt" do + xss_multiline_link = <<~XSS + javascript:exploit_code();/* + #{codepen_link} + */ + XSS + expect { generate_new_liquid(xss_multiline_link) }.to raise_error(StandardError) + end end end diff --git a/spec/liquid_tags/kotlin_tag_spec.rb b/spec/liquid_tags/kotlin_tag_spec.rb index ad793efe1..e42093eb4 100644 --- a/spec/liquid_tags/kotlin_tag_spec.rb +++ b/spec/liquid_tags/kotlin_tag_spec.rb @@ -2,8 +2,7 @@ require "rails_helper" RSpec.describe KotlinTag, type: :liquid_template do describe "#link" do - input = "https://pl.kotl.in/owreUFFUG?theme=darcula&from=3&to=6&readOnly=true" - expected = "https://play.kotlinlang.org/embed?short=owreUFFUG&from=3&to=6&theme=darcula&readOnly=true" + let(:valid_link) { "https://pl.kotl.in/owreUFFUG?theme=darcula&from=3&to=6&readOnly=true" } def generate_new_liquid(link) Liquid::Template.register_tag("kotlin", KotlinTag) @@ -34,11 +33,12 @@ RSpec.describe KotlinTag, type: :liquid_template do end it "produces a correct final URL" do - expect(described_class.embedded_url(input)).to eq(expected) + expected = "https://play.kotlinlang.org/embed?short=owreUFFUG&from=3&to=6&theme=darcula&readOnly=true" + expect(described_class.embedded_url(valid_link)).to eq(expected) end it "renders correctly a Kotlin Playground link" do - liquid = generate_new_liquid(input) + liquid = generate_new_liquid(valid_link) rendered_kotlin_iframe = liquid.render Approvals.verify(rendered_kotlin_iframe, name: "kotlin_liquid_tag", format: :html) end diff --git a/spec/liquid_tags/stackexchange_tag_spec.rb b/spec/liquid_tags/stackexchange_tag_spec.rb index adc268391..cddab9b40 100644 --- a/spec/liquid_tags/stackexchange_tag_spec.rb +++ b/spec/liquid_tags/stackexchange_tag_spec.rb @@ -25,10 +25,12 @@ RSpec.describe StackexchangeTag, type: :liquid_template, vcr: vcr_option do liquid = generate_new_liquid(valid_id) expect(liquid.render).to include("ltag__stackexchange") end + it "renders basic exchange html" do liquid = generate_exchange_liquid(exchange_id) expect(liquid.render).to include("stackexchange-logo") end + it "rejects invalid id" do expect do liquid = generate_exchange_liquid(invalid_id)