diff --git a/rubocop-airbnb/lib/rubocop/cop/airbnb/default_scope.rb b/rubocop-airbnb/lib/rubocop/cop/airbnb/default_scope.rb index a6cbf4e..7cfb064 100644 --- a/rubocop-airbnb/lib/rubocop/cop/airbnb/default_scope.rb +++ b/rubocop-airbnb/lib/rubocop/cop/airbnb/default_scope.rb @@ -8,9 +8,10 @@ class DefaultScope < Base 'refactor data access patterns since the scope becomes part '\ 'of every query unless explicitly excluded, even when it is '\ 'unnecessary or incidental to the desired logic.'.freeze + RESTRICT_ON_SEND = %i(default_scope).freeze def on_send(node) - return unless node.command?(:default_scope) + return if node.receiver add_offense(node) end diff --git a/rubocop-airbnb/lib/rubocop/cop/airbnb/factory_class_use_string.rb b/rubocop-airbnb/lib/rubocop/cop/airbnb/factory_class_use_string.rb index 0b53cd1..3a1c6fa 100644 --- a/rubocop-airbnb/lib/rubocop/cop/airbnb/factory_class_use_string.rb +++ b/rubocop-airbnb/lib/rubocop/cop/airbnb/factory_class_use_string.rb @@ -6,9 +6,10 @@ module Airbnb class FactoryClassUseString < Base MSG = 'Instead of :class => MyClass, use :class => "MyClass". ' \ "This enables faster spec startup time and faster Zeus reload time.".freeze + RESTRICT_ON_SEND = %i(factory).freeze def on_send(node) - return unless node.command?(:factory) + return if node.receiver class_pair = class_node(node) diff --git a/rubocop-airbnb/lib/rubocop/cop/airbnb/mass_assignment_accessible_modifier.rb b/rubocop-airbnb/lib/rubocop/cop/airbnb/mass_assignment_accessible_modifier.rb index be982e1..233cf0f 100644 --- a/rubocop-airbnb/lib/rubocop/cop/airbnb/mass_assignment_accessible_modifier.rb +++ b/rubocop-airbnb/lib/rubocop/cop/airbnb/mass_assignment_accessible_modifier.rb @@ -5,11 +5,9 @@ module Airbnb # mass assignment. It's a lazy, potentially dangerous approach that should be discouraged. class MassAssignmentAccessibleModifier < Base MSG = 'Do no override and objects mass assignment restrictions.'.freeze + RESTRICT_ON_SEND = %i(accessible=).freeze def on_send(node) - _receiver, method_name, *_args = *node - - return unless method_name == :accessible= add_offense(node, message: MSG) end end diff --git a/rubocop-airbnb/lib/rubocop/cop/airbnb/no_timeout.rb b/rubocop-airbnb/lib/rubocop/cop/airbnb/no_timeout.rb index 66e929c..b2c452d 100644 --- a/rubocop-airbnb/lib/rubocop/cop/airbnb/no_timeout.rb +++ b/rubocop-airbnb/lib/rubocop/cop/airbnb/no_timeout.rb @@ -8,9 +8,14 @@ class NoTimeout < Base 'It can also cause logic errors since it can raise in ' \ 'any callee scope. Use client library timeouts and monitoring to ' \ 'ensure proper timing behavior for web requests.'.freeze + RESTRICT_ON_SEND = %i(timeout).freeze + + def_node_matcher :timeout_const?, <<~PATTERN + (const {cbase nil?} :Timeout) + PATTERN def on_send(node) - return unless node.source.start_with?('Timeout.timeout') + return unless timeout_const?(node.receiver) add_offense(node, message: MSG) end end diff --git a/rubocop-airbnb/lib/rubocop/cop/airbnb/phrase_bundle_keys.rb b/rubocop-airbnb/lib/rubocop/cop/airbnb/phrase_bundle_keys.rb index cec6ab4..069db77 100644 --- a/rubocop-airbnb/lib/rubocop/cop/airbnb/phrase_bundle_keys.rb +++ b/rubocop-airbnb/lib/rubocop/cop/airbnb/phrase_bundle_keys.rb @@ -27,10 +27,11 @@ module Airbnb class PhraseBundleKeys < Base MESSAGE = 'Phrase bundle keys should match their translation keys.'.freeze + RESTRICT_ON_SEND = %i(t).freeze def on_send(node) parent = node.parent - if t_call?(node) && in_phrase_bundle_class?(node) && parent.pair_type? + if in_phrase_bundle_class?(node) && parent.pair_type? hash_key = parent.children[0] unless hash_key.children[0] == node.children[2].children[0] add_offense(hash_key, message: MESSAGE) @@ -57,10 +58,6 @@ def const_phrase_bundle_children(node) e.children[1] == :PhraseBundle end end - - def t_call?(node) - node.children[1] == :t - end end end end diff --git a/rubocop-airbnb/lib/rubocop/cop/airbnb/risky_activerecord_invocation.rb b/rubocop-airbnb/lib/rubocop/cop/airbnb/risky_activerecord_invocation.rb index 0a65200..52903c9 100644 --- a/rubocop-airbnb/lib/rubocop/cop/airbnb/risky_activerecord_invocation.rb +++ b/rubocop-airbnb/lib/rubocop/cop/airbnb/risky_activerecord_invocation.rb @@ -3,7 +3,14 @@ module Cop module Airbnb # Disallow ActiveRecord calls that pass interpolated or added strings as an argument. class RiskyActiverecordInvocation < Base - VULNERABLE_AR_METHODS = [ + MSG = 'Passing a string computed by interpolation or addition to an ActiveRecord ' \ + 'method is likely to lead to SQL injection. Use hash or parameterized syntax. For ' \ + 'more information, see ' \ + 'http://guides.rubyonrails.org/security.html#sql-injection-countermeasures and ' \ + 'https://rails-sqli.org/rails3. If you have confirmed with Security that this is a ' \ + 'safe usage of this style, disable this alert with ' \ + '`# rubocop:disable Airbnb/RiskyActiverecordInvocation`.'.freeze + RESTRICT_ON_SEND = [ :delete_all, :destroy_all, :exists?, @@ -22,29 +29,15 @@ class RiskyActiverecordInvocation < Base :update_all, :where, ].freeze - MSG = 'Passing a string computed by interpolation or addition to an ActiveRecord ' \ - 'method is likely to lead to SQL injection. Use hash or parameterized syntax. For ' \ - 'more information, see ' \ - 'http://guides.rubyonrails.org/security.html#sql-injection-countermeasures and ' \ - 'https://rails-sqli.org/rails3. If you have confirmed with Security that this is a ' \ - 'safe usage of this style, disable this alert with ' \ - '`# rubocop:disable Airbnb/RiskyActiverecordInvocation`.'.freeze def on_send(node) - receiver, method_name, *_args = *node - - return if receiver.nil? - return unless vulnerable_ar_method?(method_name) - if !includes_interpolation?(_args) && !includes_sum?(_args) + return if node.receiver.nil? + if !includes_interpolation?(node.arguments) && !includes_sum?(node.arguments) return end add_offense(node) end - def vulnerable_ar_method?(method) - VULNERABLE_AR_METHODS.include?(method) - end - # Return true if the first arg is a :dstr that has non-:str components def includes_interpolation?(args) !args.first.nil? && diff --git a/rubocop-airbnb/lib/rubocop/cop/airbnb/rspec_environment_modification.rb b/rubocop-airbnb/lib/rubocop/cop/airbnb/rspec_environment_modification.rb index b4ffc09..55e0326 100644 --- a/rubocop-airbnb/lib/rubocop/cop/airbnb/rspec_environment_modification.rb +++ b/rubocop-airbnb/lib/rubocop/cop/airbnb/rspec_environment_modification.rb @@ -40,6 +40,7 @@ class RspecEnvironmentModification < Base def_node_matcher :rails_env_assignment, '(send (const nil? :Rails) :env= ...)' MESSAGE = "Do not stub or set Rails.env in specs. Use the `stub_env` method instead".freeze + RESTRICT_ON_SEND = %i(to stub env=).freeze def on_send(node) path = node.source_range.source_buffer.name diff --git a/rubocop-airbnb/lib/rubocop/cop/airbnb/unsafe_yaml_marshal.rb b/rubocop-airbnb/lib/rubocop/cop/airbnb/unsafe_yaml_marshal.rb index 0687379..34d5991 100644 --- a/rubocop-airbnb/lib/rubocop/cop/airbnb/unsafe_yaml_marshal.rb +++ b/rubocop-airbnb/lib/rubocop/cop/airbnb/unsafe_yaml_marshal.rb @@ -6,35 +6,34 @@ class UnsafeYamlMarshal < Base MSG = 'Using unsafe YAML parsing methods on untrusted input can lead ' \ 'to remote code execution. Use `safe_load`, `parse`, `parse_file`, or ' \ '`parse_stream` instead'.freeze + RESTRICT_ON_SEND = %i(load load_documents load_file load_stream).freeze def on_send(node) - receiver, method_name, *_args = *node + return if node.receiver.nil? + return unless node.receiver.const_type? - return if receiver.nil? - return unless receiver.const_type? - - check_yaml(node, receiver, method_name, *_args) - check_marshal(node, receiver, method_name, *_args) + check_yaml(node) + check_marshal(node) rescue => e puts e puts e.backtrace raise end - def check_yaml(node, receiver, method_name, *_args) - return unless ['YAML', 'Psych'].include?(receiver.const_name) - return unless [:load, :load_documents, :load_file, :load_stream].include?(method_name) + def check_yaml(node) + const_name = node.receiver.const_name + return unless ['YAML', 'Psych'].include?(const_name) - message = "Using `#{receiver.const_name}.#{method_name}` on untrusted input can lead " \ + message = "Using `#{const_name}.#{node.method_name}` on untrusted input can lead " \ "to remote code execution. Use `safe_load`, `parse`, `parse_file`, or " \ "`parse_stream` instead" add_offense(node, message: message) end - def check_marshal(node, receiver, method_name, *_args) - return unless receiver.const_name == 'Marshal' - return unless method_name == :load + def check_marshal(node) + return unless node.receiver.const_name == 'Marshal' + return unless node.method?(:load) message = 'Using `Marshal.load` on untrusted input can lead to remote code execution. ' \ 'Restructure your code to not use Marshal' diff --git a/rubocop-airbnb/spec/rubocop/cop/airbnb/no_timeout_spec.rb b/rubocop-airbnb/spec/rubocop/cop/airbnb/no_timeout_spec.rb index 730a6d3..631021a 100644 --- a/rubocop-airbnb/spec/rubocop/cop/airbnb/no_timeout_spec.rb +++ b/rubocop-airbnb/spec/rubocop/cop/airbnb/no_timeout_spec.rb @@ -11,6 +11,17 @@ def some_method(a) RUBY end + it 'rejects ::Timeout.timeout' do + expect_offense(<<~RUBY) + def some_method(a) + ::Timeout.timeout(5) do + ^^^^^^^^^^^^^^^^^^^^ Do not use Timeout.timeout. [...] + some_other_method(a) + end + end + RUBY + end + it 'accepts foo.timeout' do expect_no_offenses(<<~RUBY) def some_method(a)