From 37812b4a16ce2a4a061775a3fe2f828dd91e9efb Mon Sep 17 00:00:00 2001 From: Motoaki Sasaki Date: Tue, 27 Aug 2024 10:05:09 +0000 Subject: [PATCH 1/2] add NoLinkToBack linter --- lib/erb_lint/linters/no_link_to_back.rb | 99 +++++++++++++++++++ spec/erb_lint/linters/no_link_to_back_spec.rb | 74 ++++++++++++++ 2 files changed, 173 insertions(+) create mode 100644 lib/erb_lint/linters/no_link_to_back.rb create mode 100644 spec/erb_lint/linters/no_link_to_back_spec.rb diff --git a/lib/erb_lint/linters/no_link_to_back.rb b/lib/erb_lint/linters/no_link_to_back.rb new file mode 100644 index 0000000..c3c0060 --- /dev/null +++ b/lib/erb_lint/linters/no_link_to_back.rb @@ -0,0 +1,99 @@ +# frozen_string_literal: true + +require "better_html" + +module ERBLint + module Linters + # Disallow `:back` option in `link_to` like methods. + # It potentially causes XSS attack by HTTP Referer pollution. + class NoLinkToBack < Linter + include LinterRegistry + + class UnKnownMethodNameError < StandardError; end + + def run(processed_source) + processed_source + .parser + .ast + .descendants(:erb) + .flat_map { |erb| erb.descendants(:code).to_a } + .map do |code| + { + loc: code.loc, + node: BetterHtml::TestHelper::RubyNode.parse(code.loc.source), + block: code.loc.source.rstrip.end_with?("do"), + } + end + .select { |code| link_to_like_method?(code) } + .map do |code| + { + **code, + method_name: extract_method_name(code), + } + end + .select { |code| link_to_with_back?(code) } + .each do |code| + add_offense( + processed_source.to_source_range( + code[:loc].begin_pos..code[:loc].end_pos, + ), + "Don't use :back option in #{code[:node].method_name} method. It potentially causes XSS attack by HTTP Referer pollution.", + ) + end + end + + private + + def link_to_like_method?(code) + link_to?(code[:node]) || link_to_if?(code[:node]) || link_to_unless?(code[:node]) + end + + def extract_method_name(code) + if link_to?(code[:node]) + :link_to + elsif link_to_if?(code[:node]) + :link_to_if + elsif link_to_unless?(code[:node]) + :link_to_unless + else + raise UnKnownMethodNameError, "Unknown method name: #{code[:node].method_name}" + end + end + + def link_to_with_back?(code) + method_index = code[:node].children.find_index { |c| c == code[:method_name] } + arguments = code[:node].children[method_index + 1..-1] + index = option_index_in_signature(code) + return false if arguments[index].nil? + + arguments[index].is_a?(BetterHtml::TestHelper::RubyNode) && + arguments[index].type?(:sym) && + arguments[index].children.first.to_sym == :back + end + + def option_index_in_signature(code) + if code[:method_name] == :link_to + if code[:block] + 0 + else + 1 + end + else + 2 + end + end + + def link_to?(node) + !node.nil? && node.type == :send && node.method_name == :link_to + end + + def link_to_if?(node) + !node.nil? && node.type == :send && node.method_name == :link_to_if + end + + def link_to_unless?(node) + !node.nil? && node.type == :send && node.method_name == :link_to_unless + end + end + end +end diff --git a/spec/erb_lint/linters/no_link_to_back_spec.rb b/spec/erb_lint/linters/no_link_to_back_spec.rb new file mode 100644 index 0000000..277cb2b --- /dev/null +++ b/spec/erb_lint/linters/no_link_to_back_spec.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +require "spec_helper" +require "spec_utils" + +describe ERBLint::Linters::NoLinkToBack do + let(:linter_config) { described_class.config_schema.new } + + let(:file_loader) { ERBLint::FileLoader.new(".") } + let(:linter) { described_class.new(file_loader, linter_config) } + let(:processed_source) { ERBLint::ProcessedSource.new("file.rb", file) } + let(:offenses) { linter.offenses } + + describe "offenses" do + subject { offenses } + + context "when file has link_to :back" do + let(:file) do + <<~ERB + <%= link_to '戻る', :back %> + ERB + end + before { linter.run(processed_source) } + it do + expect(subject.size).to(eq(1)) + expect(subject.first.message).to( + eq("Don't use :back option in link_to method. It potentially causes XSS attack by HTTP Referer pollution."), + ) + end + end + + context "when file has link_to :back with block" do + let(:file) do + <<~ERB + <%= link_to :back do %> + test + <% end %> + ERB + end + before { linter.run(processed_source) } + it do + expect(subject.size).to(eq(1)) + expect(subject.first.message).to( + eq("Don't use :back option in link_to method. It potentially causes XSS attack by HTTP Referer pollution."), + ) + end + end + + context "when file has link_to_unless :back" do + let(:file) { "<%= link_to_unless true, '戻る', :back %>" } + before { linter.run(processed_source) } + + it do + expect(subject.size).to(eq(1)) + expect(subject.first.message).to(eq( + "Don't use :back option in link_to_unless method. " \ + "It potentially causes XSS attack by HTTP Referer pollution.", + )) + end + end + + context ":back is used as name" do + let(:file) do + <<~ERB + <%= link_to :back, '#' %> + ERB + end + before { linter.run(processed_source) } + it do + expect(subject.size).to(eq(0)) + end + end + end +end From 6cc333112cf73bd38c39866c4a510602f082ab70 Mon Sep 17 00:00:00 2001 From: Motoaki Sasaki Date: Wed, 28 Aug 2024 03:59:53 +0000 Subject: [PATCH 2/2] fix for rubocop --- lib/erb_lint/linters/no_link_to_back.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/erb_lint/linters/no_link_to_back.rb b/lib/erb_lint/linters/no_link_to_back.rb index c3c0060..cffe61f 100644 --- a/lib/erb_lint/linters/no_link_to_back.rb +++ b/lib/erb_lint/linters/no_link_to_back.rb @@ -37,7 +37,8 @@ def run(processed_source) processed_source.to_source_range( code[:loc].begin_pos..code[:loc].end_pos, ), - "Don't use :back option in #{code[:node].method_name} method. It potentially causes XSS attack by HTTP Referer pollution.", + "Don't use :back option in #{code[:node].method_name} method. " \ + "It potentially causes XSS attack by HTTP Referer pollution.", ) end end