Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add cop for IO.read usage #11140

Merged
merged 1 commit into from
May 12, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Library/.rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ FormulaAudit:
FormulaAuditStrict:
Enabled: true

# enable all Homebrew custom cops
Homebrew:
Enabled: true

# makes DSL usage ugly.
Layout/SpaceBeforeBrackets:
Exclude:
Expand Down
1 change: 1 addition & 0 deletions Library/Homebrew/rubocops.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
require "rubocop-rspec"
require "rubocop-sorbet"

require "rubocops/io_read"
require "rubocops/shell_commands"

require "rubocops/formula_desc"
Expand Down
35 changes: 35 additions & 0 deletions Library/Homebrew/rubocops/io_read.rb
Original file line number Diff line number Diff line change
@@ -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.%<method>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
2 changes: 1 addition & 1 deletion Library/Homebrew/rubocops/shell_commands.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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[
!
Expand Down
75 changes: 75 additions & 0 deletions Library/Homebrew/test/rubocops/io_read_spec.rb
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion Library/Homebrew/test/rubocops/shell_commands_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

module RuboCop
module Cop
module Style
module Homebrew
describe ShellCommands do
subject(:cop) { described_class.new }

Expand Down