From 7a0c159330ea4f31ad9b01b7b81b5f41ca070921 Mon Sep 17 00:00:00 2001
From: Justin Collins <justin@presidentbeef.com>
Date: Thu, 9 Jul 2015 14:58:28 -0700
Subject: [PATCH] Fix String#<< with string target

---
 lib/brakeman/processors/alias_processor.rb     |  5 +++--
 .../app/controllers/another_controller.rb      | 18 ++++++++++++++++++
 test/tests/alias_processor.rb                  |  8 ++++++++
 test/tests/rails4.rb                           | 16 ++++++++++++++--
 4 files changed, 43 insertions(+), 4 deletions(-)

diff --git a/lib/brakeman/processors/alias_processor.rb b/lib/brakeman/processors/alias_processor.rb
index 06981397..23d976ae 100644
--- a/lib/brakeman/processors/alias_processor.rb
+++ b/lib/brakeman/processors/alias_processor.rb
@@ -77,6 +77,7 @@ class Brakeman::AliasProcessor < Brakeman::SexpProcessor
   #Process a method call.
   def process_call exp
     target_var = exp.target
+    target_var &&= target_var.deep_clone
     exp = process_default exp
 
     #In case it is replaced with something else
@@ -164,8 +165,8 @@ class Brakeman::AliasProcessor < Brakeman::SexpProcessor
         env[target_var] = target
         return target
       else
-        target = find_push_target exp
-        env[target] = exp unless target.nil? #Happens in TemplateAliasProcessor
+        target = find_push_target(target_var)
+        env[target] = exp unless target.nil? # Happens in TemplateAliasProcessor
       end
     end
 
diff --git a/test/apps/rails4/app/controllers/another_controller.rb b/test/apps/rails4/app/controllers/another_controller.rb
index 2cd01591..e53a66b4 100644
--- a/test/apps/rails4/app/controllers/another_controller.rb
+++ b/test/apps/rails4/app/controllers/another_controller.rb
@@ -37,4 +37,22 @@ class AnotherController < ApplicationController
   def use_params_in_regex
     @x = something.match /#{params[:x]}/
   end
+
+  def building_strings_for_sql
+    query = "SELECT * FROM users WHERE"
+
+    if params[:search].to_i == 1
+      query << " role = 'admin'"
+    else
+      query << " role = 'admin' " + params[:search]
+    end
+
+    begin
+      result = {:result => User.find_by_sql(query) }
+    rescue
+      result = {}
+    end
+
+    render json: result.as_json
+  end
 end
diff --git a/test/tests/alias_processor.rb b/test/tests/alias_processor.rb
index f5b6b003..6ef20f4c 100644
--- a/test/tests/alias_processor.rb
+++ b/test/tests/alias_processor.rb
@@ -50,6 +50,14 @@ class AliasProcessorTests < Test::Unit::TestCase
     RUBY
   end
 
+  def test_string_append_call
+    assert_alias "'hello ' << params[:x]", <<-RUBY
+    x = ""
+    x << 'hello ' << params[:x]
+    x
+    RUBY
+  end
+
   def test_array_index
     assert_alias "'cookie'", <<-RUBY
       dessert = ["fruit", "pie", "ice cream"]
diff --git a/test/tests/rails4.rb b/test/tests/rails4.rb
index 7fac2647..1dcade6f 100644
--- a/test/tests/rails4.rb
+++ b/test/tests/rails4.rb
@@ -14,7 +14,7 @@ class Rails4Tests < Test::Unit::TestCase
       :controller => 0,
       :model => 2,
       :template => 6,
-      :generic => 60
+      :generic => 61
     }
   end
 
@@ -702,7 +702,19 @@ class Rails4Tests < Test::Unit::TestCase
       :user_input => s(:lvar, :locale)
   end
 
- def test_no_sql_injection_from_arel_methods
+  def test_sql_injection_string_concat
+    assert_warning :type => :warning,
+      :warning_code => 0,
+      :fingerprint => "b92b1e8d755fb15bed56f8e6f872f81784813b0eae3682b68dd0601e4fb9c0a6",
+      :warning_type => "SQL Injection",
+      :line => 51,
+      :message => /^Possible\ SQL\ injection/,
+      :confidence => 0,
+      :relative_path => "app/controllers/another_controller.rb",
+      :user_input => s(:call, s(:params), :[], s(:lit, :search))
+  end
+
+  def test_no_sql_injection_from_arel_methods
     assert_no_warning :type => :warning,
       :warning_code => 0,
       :fingerprint => "61d957cdeca70a82f53d7ec72287fc21f67c67c6e8dbc9c3c4cb2d115f3a5602",
-- 
GitLab