Add new Security/IoMethods cop

changelog/new_add_new_security_io_read_cop.md

@@ -0,0 +1 @@
+* [#10102](https://github.com/rubocop/rubocop/pull/10102): Add new `Security/IoMethods` cop. ([@koic][])

config/default.yml

@@ -2739,6 +2739,14 @@ Security/Eval:
   Enabled: true
   VersionAdded: '0.47'
 
+Security/IoMethods:
+  Description: >-
+                 Checks for the first argument to `IO.read`, `IO.binread`, `IO.write`, `IO.binwrite`,
+                 `IO.foreach`, and `IO.readlines`.
+  Enabled: pending
+  Safe: false
+  VersionAdded: '<<next>>'
+
 Security/JSONLoad:
   Description: >-
                  Prefer usage of `JSON.parse` over `JSON.load` due to potential

lib/rubocop.rb

@@ -639,6 +639,7 @@
 require_relative 'rubocop/cop/style/zero_length_predicate'
 
 require_relative 'rubocop/cop/security/eval'
+require_relative 'rubocop/cop/security/io_methods'
 require_relative 'rubocop/cop/security/json_load'
 require_relative 'rubocop/cop/security/marshal_load'
 require_relative 'rubocop/cop/security/open'

lib/rubocop/cop/security/io_methods.rb

@@ -0,0 +1,49 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Security
+      # Checks for the first argument to `IO.read`, `IO.binread`, `IO.write`, `IO.binwrite`,
+      # `IO.foreach`, and `IO.readlines`.
+      #
+      # If argument starts with a pipe character (`'|'`) and the receiver is the `IO` class,
+      # a subprocess is created in the same way as `Kernel#open`, and its output is returned.
+      # `Kernel#open` may allow unintentional command injection, which is the reason these
+      # `IO` methods are a security risk.
+      # Consider to use `File.read` to disable the behavior of subprocess invocation.
+      #
+      # @safety
+      #   This cop is unsafe because false positive will occur if the variable passed as
+      #   the first argument is a command that is not a file path.
+      #
+      # @example
+      #
+      #   # bad
+      #   IO.read(path)
+      #   IO.read('path')
+      #
+      #   # good
+      #   File.read(path)
+      #   File.read('path')
+      #   IO.read('| command') # Allow intentional command invocation.
+      #
+      class IoMethods < Base
+        extend AutoCorrector
+
+        MSG = '`File.%<method_name>s` is safer than `IO.%<method_name>s`.'
+        RESTRICT_ON_SEND = %i[read binread write binwrite foreach readlines].freeze
+
+        def on_send(node)
+          return unless (receiver = node.receiver) && receiver.source == 'IO'
+
+          argument = node.first_argument
+          return if argument.respond_to?(:value) && argument.value.strip.start_with?('|')
+
+          add_offense(node, message: format(MSG, method_name: node.method_name)) do |corrector|
+            corrector.replace(receiver, 'File')
+          end
+        end
+      end
+    end
+  end
+end

spec/rubocop/cli/options_spec.rb

@@ -507,6 +507,8 @@ class SomeCop < Base
                 Enabled: false
               Naming:
                 Enabled: false
+              Security:
+                Enabled: false
 
               Style/SomeCop:
                 Description: Something

spec/rubocop/cop/generator_spec.rb

@@ -183,7 +183,8 @@ def on_send(node)
     end
 
     before do
-      IO.write(path, <<~YAML)
+      # It is hacked to use `IO.write` to avoid mocking `File.write` for testing.
+      IO.write(path, <<~YAML) # rubocop:disable Security/IoMethods
         Style/Alias:
           Enabled: true
 

spec/rubocop/cop/security/io_methods_spec.rb

@@ -0,0 +1,92 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::Cop::Security::IoMethods, :config do
+  shared_examples 'offense' do |current, preferred, method_name|
+    it "registers and corrects an offense when using `#{method_name}`" do
+      expect_offense(<<~RUBY, current: current)
+        #{current}
+        ^{current} `File.#{method_name}` is safer than `IO.#{method_name}`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        #{preferred}
+      RUBY
+    end
+  end
+
+  shared_examples 'accepts' do |code|
+    it "does not register an offense when using `#{code}`" do
+      expect_no_offenses(<<~RUBY)
+        #{code}
+      RUBY
+    end
+  end
+
+  context 'when using `IO` receiver and variable argument' do
+    it_behaves_like 'offense', 'IO.read(path)', 'File.read(path)', 'read'
+    it_behaves_like 'offense', 'IO.write(path, "hi")', 'File.write(path, "hi")', 'write'
+    it_behaves_like 'offense', 'IO.binread(path)', 'File.binread(path)', 'binread'
+    it_behaves_like 'offense', 'IO.binwrite(path, "hi")', 'File.binwrite(path, "hi")', 'binwrite'
+    it_behaves_like 'offense', 'IO.readlines(path)', 'File.readlines(path)', 'readlines'
+    it 'registers and corrects an offense when using `foreach`' do
+      expect_offense(<<~RUBY)
+        IO.foreach(path) { |x| puts x }
+        ^^^^^^^^^^^^^^^^ `File.foreach` is safer than `IO.foreach`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        File.foreach(path) { |x| puts x }
+      RUBY
+    end
+  end
+
+  context 'when using `IO` receiver and string argument' do
+    it_behaves_like 'offense', 'IO.read("command")', 'File.read("command")', 'read'
+    it_behaves_like 'offense', 'IO.write("command", "hi")', 'File.write("command", "hi")', 'write'
+    it_behaves_like 'offense', 'IO.binwrite("command", "hi")', 'File.binwrite("command", "hi")', 'binwrite'
+    it_behaves_like 'offense', 'IO.binwrite(path, "hi")', 'File.binwrite(path, "hi")', 'binwrite'
+    it_behaves_like 'offense', 'IO.readlines("command")', 'File.readlines("command")', 'readlines'
+    it 'registers and corrects an offense when using `foreach`' do
+      expect_offense(<<~RUBY)
+        IO.foreach("command") { |x| puts x }
+        ^^^^^^^^^^^^^^^^^^^^^ `File.foreach` is safer than `IO.foreach`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        File.foreach("command") { |x| puts x }
+      RUBY
+    end
+  end
+
+  context 'when using `File` receiver' do
+    it_behaves_like 'accepts', 'File.read(path)'
+    it_behaves_like 'accepts', 'File.binread(path)'
+    it_behaves_like 'accepts', 'File.binwrite(path, "hi")'
+    it_behaves_like 'accepts', 'File.readlines(path)'
+    it_behaves_like 'accepts', 'File.foreach(path) { |x| puts x }'
+  end
+
+  context 'when using no receiver' do
+    it_behaves_like 'accepts', 'read("command")'
+    it_behaves_like 'accepts', 'write("command", "hi")'
+    it_behaves_like 'accepts', 'binwrite("command", "hi")'
+    it_behaves_like 'accepts', 'readlines("command")'
+    it_behaves_like 'accepts', 'foreach("command") { |x| puts x }'
+  end
+
+  context 'when using `IO` receiver and string argument that starts with a pipe character (`"| command"`)' do
+    it_behaves_like 'accepts', 'IO.read("| command")'
+    it_behaves_like 'accepts', 'IO.write("| command", "hi")'
+    it_behaves_like 'accepts', 'IO.binwrite("| command", "hi")'
+    it_behaves_like 'accepts', 'IO.readlines("| command")'
+    it_behaves_like 'accepts', 'IO.foreach("| command") { |x| puts x }'
+  end
+
+  context 'when using `IO` receiver and string argument that starts with a pipe character (`" | command"`)' do
+    it_behaves_like 'accepts', 'IO.read(" | command")'
+    it_behaves_like 'accepts', 'IO.write(" | command", "hi")'
+    it_behaves_like 'accepts', 'IO.binwrite(" | command", "hi")'
+    it_behaves_like 'accepts', 'IO.readlines(" | command")'
+    it_behaves_like 'accepts', 'IO.foreach(" | command") { |x| puts x }'
+  end
+end