From 7ccbaaf3136f28331d1ff12aded3628738f4d738 Mon Sep 17 00:00:00 2001 From: Justin Collins Date: Sat, 18 Feb 2023 22:17:36 -0800 Subject: [PATCH] Fix file/line location for EOL software warnings (#1761) * Store file and line for Ruby version * Fix EOL warning line numbers * Match rescanning order to scan order Gemfile comes before config files * Fix EOL Ruby tests --- lib/brakeman/checks/base_check.rb | 1 - lib/brakeman/checks/eol_check.rb | 4 ++-- lib/brakeman/processors/gem_processor.rb | 4 ++-- lib/brakeman/rescanner.rb | 4 +++- lib/brakeman/scanner.rb | 2 +- lib/brakeman/tracker/config.rb | 3 ++- test/apps/rails7/.ruby-version | 1 - test/test.rb | 2 +- test/tests/cves.rb | 10 ++++++---- test/tests/rails52.rb | 10 ++++++---- test/tests/rails6.rb | 2 +- test/tests/rails7.rb | 12 ++++++++++-- 12 files changed, 34 insertions(+), 21 deletions(-) delete mode 100644 test/apps/rails7/.ruby-version diff --git a/lib/brakeman/checks/base_check.rb b/lib/brakeman/checks/base_check.rb index 5aaa7acd86..78d3739b93 100644 --- a/lib/brakeman/checks/base_check.rb +++ b/lib/brakeman/checks/base_check.rb @@ -467,7 +467,6 @@ def lts_version? version version_between? version, "2.3.18.99", tracker.config.gem_version(:'railslts-version') end - def version_between? low_version, high_version, current_version = nil tracker.config.version_between? low_version, high_version, current_version end diff --git a/lib/brakeman/checks/eol_check.rb b/lib/brakeman/checks/eol_check.rb index d96b9bd51f..52245db462 100644 --- a/lib/brakeman/checks/eol_check.rb +++ b/lib/brakeman/checks/eol_check.rb @@ -34,7 +34,7 @@ def warn_about_soon_unsupported_version library, eol_date, version, confidence warning_code: :"pending_eol_#{library}", message: msg("Support for ", msg_version(version, library.capitalize), " ends on #{eol_date}"), confidence: confidence, - gem_info: gemfile_or_environment, + gem_info: gemfile_or_environment(library), :cwe_id => [1104] end @@ -43,7 +43,7 @@ def warn_about_unsupported_version library, eol_date, version warning_code: :"eol_#{library}", message: msg("Support for ", msg_version(version, library.capitalize), " ended on #{eol_date}"), confidence: :high, - gem_info: gemfile_or_environment, + gem_info: gemfile_or_environment(library), :cwe_id => [1104] end end diff --git a/lib/brakeman/processors/gem_processor.rb b/lib/brakeman/processors/gem_processor.rb index b423d0b8bf..a8f2831635 100644 --- a/lib/brakeman/processors/gem_processor.rb +++ b/lib/brakeman/processors/gem_processor.rb @@ -56,7 +56,7 @@ def process_call exp elsif exp.method == :ruby version = exp.first_arg if string? version - @tracker.config.set_ruby_version version.value + @tracker.config.set_ruby_version version.value, @gemfile, exp.line end end elsif @inside_gemspec and exp.method == :add_dependency @@ -97,7 +97,7 @@ def set_gem_version_and_file line, file, line_num if line =~ @gem_name_version @tracker.config.add_gem $1, $2, file, line_num elsif line =~ @ruby_version - @tracker.config.set_ruby_version $1 + @tracker.config.set_ruby_version $1, file, line_num end end end diff --git a/lib/brakeman/rescanner.rb b/lib/brakeman/rescanner.rb index 77ff06a6ee..bbcab1d0bc 100644 --- a/lib/brakeman/rescanner.rb +++ b/lib/brakeman/rescanner.rb @@ -6,7 +6,7 @@ class Brakeman::Rescanner < Brakeman::Scanner include Brakeman::Util KNOWN_TEMPLATE_EXTENSIONS = Brakeman::TemplateParser::KNOWN_TEMPLATE_EXTENSIONS - SCAN_ORDER = [:config, :gemfile, :initializer, :lib, :routes, :template, + SCAN_ORDER = [:gemfile, :config, :initializer, :lib, :routes, :template, :model, :controller] #Create new Rescanner to scan changed files @@ -332,6 +332,8 @@ def file_type path :routes when /\/config\/.+\.(rb|yml)/ :config + when /\.ruby-version/ + :config when /Gemfile|gems\./ :gemfile else diff --git a/lib/brakeman/scanner.rb b/lib/brakeman/scanner.rb index 3ce95d5cea..0b560d7214 100644 --- a/lib/brakeman/scanner.rb +++ b/lib/brakeman/scanner.rb @@ -138,7 +138,7 @@ def process_config if @app_tree.exists? ".ruby-version" if version = @app_tree.file_path(".ruby-version").read[/(\d\.\d.\d+)/] - tracker.config.set_ruby_version version + tracker.config.set_ruby_version version, @app_tree.file_path(".ruby-version"), 1 end end diff --git a/lib/brakeman/tracker/config.rb b/lib/brakeman/tracker/config.rb index e7d06d598d..5bab37ca21 100644 --- a/lib/brakeman/tracker/config.rb +++ b/lib/brakeman/tracker/config.rb @@ -129,8 +129,9 @@ def rails_version @rails_version end - def set_ruby_version version + def set_ruby_version version, file, line @ruby_version = extract_version(version) + add_gem :ruby, @ruby_version, file, line end def extract_version version diff --git a/test/apps/rails7/.ruby-version b/test/apps/rails7/.ruby-version deleted file mode 100644 index ccfb6efd96..0000000000 --- a/test/apps/rails7/.ruby-version +++ /dev/null @@ -1 +0,0 @@ -ruby-2.7.0 diff --git a/test/test.rb b/test/test.rb index f51e7961da..8a0d0dfa3d 100644 --- a/test/test.rb +++ b/test/test.rb @@ -214,7 +214,7 @@ def assert_fixed expected #Check how many new warnings were reported def assert_new expected - assert_equal expected, new.length, "Expected #{expected} new warnings, but found #{new.length}" + assert_equal expected, new.length, lambda { "Expected #{expected} new warnings, but found #{new.length}:\n#{new.map {|w| "\t#{w.message} #{w.file}" }.join("\n")}" } end #Check how many existing warnings were reported diff --git a/test/tests/cves.rb b/test/tests/cves.rb index 5113c3f244..7abc9bc5f3 100644 --- a/test/tests/cves.rb +++ b/test/tests/cves.rb @@ -252,7 +252,9 @@ def test_CVE_2016_6316_rails5 end def test_CVE_2018_3760_sprockets - before_rescan_of ["Gemfile.lock", "config/environments/production.rb"], "rails5.2" do + # Have to include `.ruby-version` otherwise it changes the EOL Ruby warning + # because the warning will point at Gemfile.lock instead of .ruby-version + before_rescan_of [".ruby-version", "Gemfile.lock", "config/environments/production.rb"], "rails5.2" do replace "Gemfile.lock", "sprockets (3.7.1)", "sprockets (4.0.0.beta2)" replace "config/environments/production.rb", "config.assets.compile = false", "config.assets.compile = true" end @@ -262,7 +264,7 @@ def test_CVE_2018_3760_sprockets end def test_CVE_2018_8048_exact_fix_version - before_rescan_of "Gemfile.lock", "rails5.2" do + before_rescan_of [".ruby-version", "Gemfile.lock"], "rails5.2" do replace "Gemfile.lock", "loofah (2.1.1)", "loofah (2.2.1)" end @@ -271,7 +273,7 @@ def test_CVE_2018_8048_exact_fix_version end def test_CVE_2018_8048_newer_version - before_rescan_of "Gemfile.lock", "rails5.2" do + before_rescan_of [".ruby-version", "Gemfile.lock"], "rails5.2" do replace "Gemfile.lock", "loofah (2.1.1)", "loofah (2.10.1)" end @@ -349,7 +351,7 @@ class TestCVEController < ApplicationController def test_CVE_2020_8166 Date.stub :today, Date.parse('2021-04-05') do - before_rescan_of "Gemfile.lock", "rails5.2" do + before_rescan_of [".ruby-version", "Gemfile.lock"], "rails5.2" do replace "Gemfile.lock", " rails (5.2.0.beta2)", " rails (5.2.4.3)" end end diff --git a/test/tests/rails52.rb b/test/tests/rails52.rb index 62b773b22f..86506a1e61 100644 --- a/test/tests/rails52.rb +++ b/test/tests/rails52.rb @@ -658,16 +658,18 @@ def test_command_injection_ignored_in_stdin :relative_path => "lib/shell.rb" end - def test_unmaintained_dependency_rails + def test_unmaintained_dependency_ruby assert_warning check_name: "EOLRuby", type: :warning, warning_code: 121, - fingerprint: "9a3951031616a07c8e02c86652f537e92c08685da97f5ec2b12d5d3602b55bb8", + fingerprint: "edf687f759ec9765bd5db185dbc615c80af77d6e7e19386fc42934e7a80307af", warning_type: "Unmaintained Dependency", - line: 109, + line: 1, message: /^Support\ for\ Ruby\ 2\.3\.1\ ended\ on\ 2019\-03\-/, confidence: 0, - relative_path: "Gemfile.lock" + relative_path: ".ruby-version", + code: nil, + user_input: nil end end diff --git a/test/tests/rails6.rb b/test/tests/rails6.rb index 07d3ea6a52..38e658a953 100644 --- a/test/tests/rails6.rb +++ b/test/tests/rails6.rb @@ -780,7 +780,7 @@ def test_unmaintained_dependency_ruby warning_code: 121, fingerprint: "81776f151be34b9c42a5fc3bec249507a2acd9b64338e6f544a68559976bc5d5", warning_type: "Unmaintained Dependency", - line: 7, + line: 4, message: /^Support\ for\ Ruby\ 2\.5\.3\ ended\ on\ 2021\-03\-/, confidence: 0, relative_path: "Gemfile" diff --git a/test/tests/rails7.rb b/test/tests/rails7.rb index 9ba73ce44a..915eb5911f 100644 --- a/test/tests/rails7.rb +++ b/test/tests/rails7.rb @@ -22,8 +22,16 @@ def expected def test_ruby_2_7_eol assert_warning check_name: "EOLRuby", - message: "Support for Ruby 2.7.0 ends on 2023-03-31 near line 140", - fingerprint: "425dcb3af9624f11f12d777d6f9fe05995719975a155c30012baa6b9dc3487df" + type: :warning, + warning_code: 123, + fingerprint: "425dcb3af9624f11f12d777d6f9fe05995719975a155c30012baa6b9dc3487df", + warning_type: "Unmaintained Dependency", + line: 230, + message: /^Support\ for\ Ruby\ 2\.7\.0\ ends\ on\ 2023\-03\-3/, + confidence: 2, + relative_path: "Gemfile.lock", + code: nil, + user_input: nil end def test_missing_encryption_1