Fix false positive/negative in Security/Eval cop

CHANGELOG.md

@@ -34,6 +34,8 @@
 * [#4304](https://github.com/bbatsov/rubocop/pull/4304): Allow enabling whole departments when `DisabledByDefault` is `true`. ([@jonas054][])
 * [#4264](https://github.com/bbatsov/rubocop/issues/4264): Prevent `Rails/SaveBang` from blowing up when using the assigned variable in a hash. ([@drenmi][])
 * [#4310](https://github.com/bbatsov/rubocop/pull/4310): Treat paths containing invalid byte sequences as non-matches. ([@mclark][])
+* [#4339](https://github.com/bbatsov/rubocop/pull/4339): Fix false positive in `Security/Eval` cop for multiline string lietral. ([@pocke][])
+* [#4339](https://github.com/bbatsov/rubocop/pull/4339): Fix false negative in `Security/Eval` cop for `Binding#eval`. ([@pocke][])
 
 ## 0.48.1 (2017-04-03)
 

lib/rubocop/cop/security/eval.rb

@@ -3,20 +3,26 @@
 module RuboCop
   module Cop
     module Security
-      # This cop checks for the use of *Kernel#eval*.
+      # This cop checks for the use of `Kernel#eval` and `Binding#eval`.
       #
       # @example
       #
       #   # bad
       #
       #   eval(something)
+      #   binding.eval(something)
       class Eval < Cop
         MSG = 'The use of `eval` is a serious security risk.'.freeze
 
-        def_node_matcher :eval?, '(send nil :eval $!str ...)'
+        def_node_matcher :eval?, <<-END
+          (send {nil (send nil :binding)} :eval $!str ...)
+        END
 
         def on_send(node)
-          eval?(node) { add_offense(node, :selector) }
+          eval?(node) do |code|
+            return if code.dstr_type? && code.recursive_literal?
+            add_offense(node, :selector)
+          end
         end
       end
     end

manual/cops_security.md

@@ -6,14 +6,15 @@ Enabled by default | Supports autocorrection
 --- | ---
 Enabled | No
 
-This cop checks for the use of *Kernel#eval*.
+This cop checks for the use of `Kernel#eval` and `Binding#eval`.
 
 ### Example
 
 ```ruby
 # bad
 
 eval(something)
+binding.eval(something)
 ```
 
 ## Security/JSONLoad

spec/rubocop/cop/security/eval_spec.rb

@@ -15,26 +15,51 @@
     expect(cop.highlights).to eq(['eval'])
   end
 
-  it 'does not register an offense for eval as variable' do
+  it 'registers an offense `Binding#eval`' do
+    inspect_source(cop, 'binding.eval something')
+    expect(cop.offenses.size).to eq(1)
+    expect(cop.highlights).to eq(['eval'])
+  end
+
+  it 'registers an offense for eval with string that has an interpolation' do
+    inspect_source(cop, 'eval "something#{foo}"')
+    expect(cop.offenses.size).to eq(1)
+    expect(cop.highlights).to eq(['eval'])
+  end
+
+  it 'accepts eval as variable' do
     inspect_source(cop, 'eval = something')
     expect(cop.offenses).to be_empty
   end
 
-  it 'does not register an offense for eval as method' do
+  it 'accepts eval as method' do
     inspect_source(cop, 'something.eval')
     expect(cop.offenses).to be_empty
   end
 
-  it 'does not register an offense for eval on a literal string' do
+  it 'accepts eval on a literal string' do
     inspect_source(cop, 'eval("puts 1")')
     expect(cop.offenses).to be_empty
   end
 
-  it 'does not register an offense for eval with no arguments' do
+  it 'accepts eval with no arguments' do
     inspect_source(cop, 'eval')
     expect(cop.offenses).to be_empty
   end
 
+  it 'accepts eval with a multiline string' do
+    inspect_source(cop, <<-END)
+      eval "something
+      something2"
+    END
+    expect(cop.offenses).to be_empty
+  end
+
+  it 'accepts eval with a string that is interpolated a literal' do
+    inspect_source(cop, 'eval "something#{2}"')
+    expect(cop.offenses).to be_empty
+  end
+
   context 'with an explicit binding, filename, and line number' do
     it 'registers an offense for eval as function' do
       inspect_source(cop, 'eval(something, binding, "test.rb", 1)')
@@ -48,7 +73,7 @@
       expect(cop.highlights).to eq(['eval'])
     end
 
-    it 'does not register an offense for eval on a literal string' do
+    it 'accepts eval on a literal string' do
       inspect_source(cop, 'eval("puts 1", binding, "test.rb", 1)')
       expect(cop.offenses).to be_empty
     end