From e5285b5ed8d1ca7c653819a1d1023d4affda4072 Mon Sep 17 00:00:00 2001 From: Bo Anderson Date: Wed, 14 Apr 2021 16:08:37 +0100 Subject: [PATCH] Add cop for IO.read usage --- Library/.rubocop.yml | 4 + Library/Homebrew/rubocops.rb | 1 + Library/Homebrew/rubocops/io_read.rb | 35 +++++++++ Library/Homebrew/rubocops/shell_commands.rb | 2 +- .../Homebrew/test/rubocops/io_read_spec.rb | 75 +++++++++++++++++++ .../test/rubocops/shell_commands_spec.rb | 2 +- 6 files changed, 117 insertions(+), 2 deletions(-) create mode 100644 Library/Homebrew/rubocops/io_read.rb create mode 100644 Library/Homebrew/test/rubocops/io_read_spec.rb diff --git a/Library/.rubocop.yml b/Library/.rubocop.yml index e15f7840a6fa3..98e4a63fc6186 100644 --- a/Library/.rubocop.yml +++ b/Library/.rubocop.yml @@ -54,6 +54,10 @@ FormulaAudit: FormulaAuditStrict: Enabled: true +# enable all Homebrew custom cops +Homebrew: + Enabled: true + # makes DSL usage ugly. Layout/SpaceBeforeBrackets: Exclude: diff --git a/Library/Homebrew/rubocops.rb b/Library/Homebrew/rubocops.rb index aeff5d68a2425..3190abc5863b2 100644 --- a/Library/Homebrew/rubocops.rb +++ b/Library/Homebrew/rubocops.rb @@ -12,6 +12,7 @@ require "rubocop-rspec" require "rubocop-sorbet" +require "rubocops/io_read" require "rubocops/shell_commands" require "rubocops/formula_desc" diff --git a/Library/Homebrew/rubocops/io_read.rb b/Library/Homebrew/rubocops/io_read.rb new file mode 100644 index 0000000000000..3db249031e469 --- /dev/null +++ b/Library/Homebrew/rubocops/io_read.rb @@ -0,0 +1,35 @@ +# typed: true +# frozen_string_literal: true + +module RuboCop + module Cop + module Homebrew + # This cop restricts usage of IO.read functions for security reasons. + # + # @api private + class IORead < Base + MSG = "The use of `IO.%s` is a security risk." + RESTRICT_ON_SEND = [:read, :readlines].freeze + + def on_send(node) + return if node.receiver != s(:const, nil, :IO) + return if safe?(node.arguments.first) + + add_offense(node, message: format(MSG, method: node.method_name)) + end + + private + + def safe?(node) + if node.str_type? + !node.str_content.empty? && !node.str_content.start_with?("|") + elsif node.dstr_type? || (node.send_type? && node.method?(:+)) + safe?(node.children.first) + else + false + end + end + end + end + end +end diff --git a/Library/Homebrew/rubocops/shell_commands.rb b/Library/Homebrew/rubocops/shell_commands.rb index 838c15ceef67b..21975a595199d 100644 --- a/Library/Homebrew/rubocops/shell_commands.rb +++ b/Library/Homebrew/rubocops/shell_commands.rb @@ -6,7 +6,7 @@ module RuboCop module Cop - module Style + module Homebrew # https://github.com/ruby/ruby/blob/v2_6_3/process.c#L2430-L2460 SHELL_BUILTINS = %w[ ! diff --git a/Library/Homebrew/test/rubocops/io_read_spec.rb b/Library/Homebrew/test/rubocops/io_read_spec.rb new file mode 100644 index 0000000000000..7817d34c3198f --- /dev/null +++ b/Library/Homebrew/test/rubocops/io_read_spec.rb @@ -0,0 +1,75 @@ +# typed: false +# frozen_string_literal: true + +require "rubocops/io_read" + +describe RuboCop::Cop::Homebrew::IORead do + subject(:cop) { described_class.new } + + it "reports an offense when `IO.read` is used with a pipe character" do + expect_offense(<<~RUBY) + IO.read("|echo test") + ^^^^^^^^^^^^^^^^^^^^^ The use of `IO.read` is a security risk. + RUBY + end + + it "does not report an offense when `IO.read` is used without a pipe character" do + expect_no_offenses(<<~RUBY) + IO.read("file.txt") + RUBY + end + + it "reports an offense when `IO.read` is used with untrustworthy input" do + expect_offense(<<~RUBY) + input = "input value from an unknown source" + IO.read(input) + ^^^^^^^^^^^^^^ The use of `IO.read` is a security risk. + RUBY + end + + it "reports an offense when `IO.read` is used with a dynamic string starting with a pipe character" do + expect_offense(<<~'RUBY') + input = "test" + IO.read("|echo #{input}") + ^^^^^^^^^^^^^^^^^^^^^^^^^ The use of `IO.read` is a security risk. + RUBY + end + + it "reports an offense when `IO.read` is used with a dynamic string at the start" do + expect_offense(<<~'RUBY') + input = "|echo test" + IO.read("#{input}.txt") + ^^^^^^^^^^^^^^^^^^^^^^^ The use of `IO.read` is a security risk. + RUBY + end + + it "does not report an offense when `IO.read` is used with a dynamic string safely" do + expect_no_offenses(<<~'RUBY') + input = "test" + IO.read("somefile#{input}.txt") + RUBY + end + + it "reports an offense when `IO.read` is used with a concatenated string starting with a pipe character" do + expect_offense(<<~'RUBY') + input = "|echo test" + IO.read("|echo " + input) + ^^^^^^^^^^^^^^^^^^^^^^^^^ The use of `IO.read` is a security risk. + RUBY + end + + it "reports an offense when `IO.read` is used with a concatenated string starting with untrustworthy input" do + expect_offense(<<~'RUBY') + input = "|echo test" + IO.read(input + ".txt") + ^^^^^^^^^^^^^^^^^^^^^^^ The use of `IO.read` is a security risk. + RUBY + end + + it "does not report an offense when `IO.read` is used with a concatenated string safely" do + expect_no_offenses(<<~'RUBY') + input = "test" + IO.read("somefile" + input + ".txt") + RUBY + end +end diff --git a/Library/Homebrew/test/rubocops/shell_commands_spec.rb b/Library/Homebrew/test/rubocops/shell_commands_spec.rb index 9e142b7207f4a..d13592cf9cde0 100644 --- a/Library/Homebrew/test/rubocops/shell_commands_spec.rb +++ b/Library/Homebrew/test/rubocops/shell_commands_spec.rb @@ -5,7 +5,7 @@ module RuboCop module Cop - module Style + module Homebrew describe ShellCommands do subject(:cop) { described_class.new }