Class: Brakeman::CheckSQL

Inherits:
BaseCheck show all
Defined in:
lib/brakeman/checks/check_sql.rb

Overview

This check tests for find calls which do not use Rails’ auto SQL escaping

For example: Project.find(:all, :conditions => “name = ‘” + params + “’”)

Project.find(:all, :conditions => “name = ‘#:name’”)

User.find_by_sql(“SELECT * FROM projects WHERE name = ‘#:name’”)

Constant Summary collapse

STRING_METHODS = 

      
        
IGNORE_METHODS_IN_SQL =
          
        

        

      
    

  



  
  
  
Constants inherited
     from BaseCheck

  
BaseCheck::CONFIDENCE


  
  
  
Constants included
     from Util

  
Util::ALL_PARAMETERS, Util::COOKIES, Util::PARAMETERS, Util::PATH_PARAMETERS, Util::QUERY_PARAMETERS, Util::REQUEST_ENV, Util::REQUEST_PARAMETERS, Util::REQUEST_PARAMS, Util::SESSION


  
  
  
Constants inherited
     from SexpProcessor

  
SexpProcessor::VERSION





  
Instance Attribute Summary

  
  
Attributes inherited from BaseCheck

  
#tracker, #warnings


  
  
  
Attributes inherited from SexpProcessor

  
#context, #env, #expected



  
    

      Instance Method Summary
      collapse
    


    
  


  
  
  
  
  
  
  
  
  
Methods inherited from BaseCheck

  
#add_result, #initialize, #process_call, #process_cookies, #process_default, #process_if, #process_params


  
  
  
  
  
  
  
  
  
Methods included from Util

  
#array?, #call?, #camelize, #contains_class?, #context_for, #cookies?, #false?, #file_by_name, #file_for, #hash?, #hash_access, #hash_insert, #hash_iterate, #integer?, #node_type?, #number?, #params?, #pluralize, #regexp?, #request_env?, #request_value?, #result?, #set_env_defaults, #sexp?, #string?, #symbol?, #table_to_csv, #true?, #truncate_table, #underscore


  
  
  
  
  
  
  
  
  
Methods included from ProcessorHelper

  
#class_name, #process_all, #process_module


  
  
  
  
  
  
  
  
  
Methods inherited from SexpProcessor

  
#error_handler, #in_context, #initialize, #process, #process_dummy, #scope



  
Constructor Details

  
    
This class inherits a constructor from Brakeman::BaseCheck

  




  

    
Instance Method Details


    
      

  

  
    #check_by_sql_arguments(arg)  ⇒ Object 
  

  

  




  

    

find_by_sql and count_by_sql can take either a straight SQL string or an array with values to bind.



  





  



  

    
      


319
320
321
322
323
324
325
326
327
328
329

    		
    
      
# File 'lib/brakeman/checks/check_sql.rb', line 319

def check_by_sql_arguments arg
  return unless sexp? arg

  #This is kind of necessary, because unsafe_sql? will handle an array
  #correctly, but might be better to be explicit.
  if array? arg
    unsafe_sql? arg[1]
  else
    unsafe_sql? arg
  end
end

    		
  





    
      

  

  
    #check_call(exp)  ⇒ Object 
  

  

  




  

    

Check call for string building



  





  



  

    
      


525
526
527
528
529
530
531
532
533
534
535

    		
    
      
# File 'lib/brakeman/checks/check_sql.rb', line 525

def check_call exp
  return unless call? exp

  if unsafe = check_for_string_building(exp)
    unsafe
  elsif call? exp.target
    check_call exp.target
  else
    nil
  end
end

    		
  





    
      

  

  
    #check_find_arguments(arg)  ⇒ Object 
  

  

  




  

    

The ‘find’ methods accept a number of different types of parameters:


  • 

    The first argument might be :all, :first, or :last
    

  • 

    The first argument might be an integer ID or an array of IDs
    

  • 

    The second argument might be a hash of options, some of which are dangerous and some of which are not
    

  • 

    The second argument might contain SQL fragments as values
    

  • 

    The second argument might contain properly parameterized SQL fragments in arrays
    

  • 

    The second argument might contain improperly parameterized SQL fragments in arrays
    




This method should only be passed the second argument.



  





  



  

    
      


254
255
256
257
258
259
260

    		
    
      
# File 'lib/brakeman/checks/check_sql.rb', line 254

def check_find_arguments arg
  if not sexp? arg or node_type? arg, :lit, :string, :str, :true, :false, :nil
    return nil
  end

  unsafe_sql? arg
end

    		
  





    
      

  

  
    #check_for_limit_or_offset_vulnerability(options)  ⇒ Object 
  

  

  




  

    

Prior to Rails 2.1.1, the :offset and :limit parameters were not escaping input properly.



www.rorsecurity.info/2008/09/08/sql-injection-issue-in-limit-and-offset-parameter/



  





  



  

    
      


541
542
543
544
545
546
547
548
549

    		
    
      
# File 'lib/brakeman/checks/check_sql.rb', line 541

def check_for_limit_or_offset_vulnerability options
  return false if @rails_version.nil? or @rails_version >= "2.1.1" or not hash? options

  if hash_access(options, :limit) or hash_access(options, :offset)
    return true
  end

  false
end

    		
  





    
      

  

  
    #check_for_string_building(exp)  ⇒ Object 
  

  

  




  

    
      


484
485
486
487
488
489
490
491
492
493
494
495
496
497
498

    		
    
      
# File 'lib/brakeman/checks/check_sql.rb', line 484

def check_for_string_building exp
  return unless call? exp

  target = exp.target
  method = exp.method
  args = exp.args

  if string? target or string? args.first
    if STRING_METHODS.include? method
      return exp
    end
  elsif STRING_METHODS.include? method and call? target
    unsafe_sql? target
  end
end

    		
  





    
      

  

  
    #check_hash_keys(exp)  ⇒ Object 
  

  

  




  

    

Check hash keys for user input. (Seems unlikely, but if a user can control the column names queried, that could be bad)



  





  



  

    
      


362
363
364
365
366
367
368
369
370
371
372

    		
    
      
# File 'lib/brakeman/checks/check_sql.rb', line 362

def check_hash_keys exp
  hash_iterate(exp) do |key, value|
    unless symbol? key
      if unsafe_key = unsafe_sql?(value)
        return unsafe_key
      end
    end
  end

  false
end

    		
  





    
      

  

  
    #check_hash_values(exp)  ⇒ Object 
  

  

  




  

    

Checks hash values associated with these keys:


  • 

    conditions
    

  • 

    order
    

  • 

    having
    

  • 

    joins
    

  • 

    select
    

  • 

    from
    

  • 

    lock
    




  





  



  

    
      


457
458
459
460
461
462
463
464
465
466
467
468
469
470
471
472
473
474
475
476
477
478
479
480

    		
    
      
# File 'lib/brakeman/checks/check_sql.rb', line 457

def check_hash_values exp
  hash_iterate(exp) do |key, value|
    if symbol? key
      unsafe = case key[1]
               when :conditions, :having, :select
                 check_query_arguments value
               when :order, :group
                 check_order_arguments value
               when :joins
                 check_joins_arguments value
               when :lock
                 check_lock_arguments value
               when :from
                 unsafe_sql? value
               else
                 nil
               end

      return unsafe if unsafe
    end
  end

  false
end

    		
  





    
      

  

  
    #check_joins_arguments(arg)  ⇒ Object 
  

  

  




  

    

joins can take a string, hash of associations, or an array of both(?) We only care about the possible string values.



  





  



  

    
      


333
334
335
336
337
338
339
340
341
342
343
344
345
346
347

    		
    
      
# File 'lib/brakeman/checks/check_sql.rb', line 333

def check_joins_arguments arg
  return unless sexp? arg and not node_type? arg, :hash, :string, :str

  if array? arg
    arg.each do |a|
      if unsafe = check_joins_arguments(a)
        return unsafe
      end
    end

    nil
  else
    unsafe_sql? arg
  end
end

    		
  





    
      

  

  
    #check_lock_arguments(arg)  ⇒ Object 
  

  

  




  

    

Model#lock essentially only cares about strings. But those strings can be any SQL fragment. This does not apply to all databases. (For those who do not support it, the lock method does nothing).



  





  



  

    
      


352
353
354
355
356

    		
    
      
# File 'lib/brakeman/checks/check_sql.rb', line 352

def check_lock_arguments arg
  return unless sexp? arg and not node_type? arg, :hash, :array, :string, :str

  unsafe_sql? arg, :ignore_hash
end

    		
  





    
      

  

  
    #check_order_arguments(args)  ⇒ Object 
  

  

  




  

    

Checks each argument to order/reorder/group for possible SQL. Anything used with these methods is passed in verbatim.



  





  



  

    
      


301
302
303
304
305
306
307
308
309
310
311
312
313
314
315

    		
    
      
# File 'lib/brakeman/checks/check_sql.rb', line 301

def check_order_arguments args
  return unless sexp? args

  if node_type? args, :arglist
    args.each do |arg|
      if unsafe_arg = unsafe_sql?(arg)
        return unsafe_arg
      end
    end

    nil
  else
    unsafe_sql? args
  end
end

    		
  





    
      

  

  
    #check_query_arguments(arg)  ⇒ Object 
  

  

  




  

    
      


272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297

    		
    
      
# File 'lib/brakeman/checks/check_sql.rb', line 272

def check_query_arguments arg
  return unless sexp? arg

  if node_type? arg, :arglist
    if arg.length > 2 and node_type? arg[1], :string_interp, :dstr
      # Model.where("blah = ?", blah)
      return check_string_interp arg[1]
    else
      arg = arg[1]
    end
  end

  if request_value? arg
    # Model.where(params[:where])
    arg
  elsif hash? arg
    #This is generally going to be a hash of column names and values, which
    #would escape the values. But the keys _could_ be user input.
    check_hash_keys arg
  elsif node_type? arg, :lit, :str
    nil
  else
    #Hashes are safe...but we check above for hash, so...?
    unsafe_sql? arg, :ignore_hash
  end
end

    		
  





    
      

  

  
    #check_rails_version_for_cve_2012_2660  ⇒ Object 
  

  

  




  

    
      


94
95
96
97
98
99
100
101
102

    		
    
      
# File 'lib/brakeman/checks/check_sql.rb', line 94

def check_rails_version_for_cve_2012_2660
  if version_between?("2.0.0", "2.3.14") || version_between?("3.0.0", "3.0.12") || version_between?("3.1.0", "3.1.4") || version_between?("3.2.0", "3.2.3")
    warn :warning_type => 'SQL Injection',
      :message => 'All versions of Rails before 3.0.13, 3.1.5, and 3.2.5 contain a SQL Query Generation Vulnerability: CVE-2012-2660; Upgrade to 3.2.5, 3.1.5, 3.0.13',
      :confidence => CONFIDENCE[:high],
      :file => gemfile_or_environment,
      :link_path => "https://groups.google.com/d/topic/rubyonrails-security/8SA-M3as7A8/discussion"
  end
end

    		
  





    
      

  

  
    #check_rails_version_for_cve_2012_2661  ⇒ Object 
  

  

  




  

    
      


104
105
106
107
108
109
110
111
112

    		
    
      
# File 'lib/brakeman/checks/check_sql.rb', line 104

def check_rails_version_for_cve_2012_2661
  if version_between?("3.0.0", "3.0.12") || version_between?("3.1.0", "3.1.4") || version_between?("3.2.0", "3.2.3")
    warn :warning_type => 'SQL Injection',
      :message => 'All versions of Rails before 3.0.13, 3.1.5, and 3.2.5 contain a SQL Injection Vulnerability: CVE-2012-2661; Upgrade to 3.2.5, 3.1.5, 3.0.13',
      :confidence => CONFIDENCE[:high],
      :file => gemfile_or_environment,
      :link_path => "https://groups.google.com/d/topic/rubyonrails-security/dUaiOOGWL1k/discussion"
  end
end

    		
  





    
      

  

  
    #check_rails_version_for_cve_2012_2695  ⇒ Object 
  

  

  




  

    
      


114
115
116
117
118
119
120
121
122

    		
    
      
# File 'lib/brakeman/checks/check_sql.rb', line 114

def check_rails_version_for_cve_2012_2695
  if version_between?("2.0.0", "2.3.14") || version_between?("3.0.0", "3.0.13") || version_between?("3.1.0", "3.1.5") || version_between?("3.2.0", "3.2.5")
    warn :warning_type => 'SQL Injection',
      :message => 'All versions of Rails before 3.0.14, 3.1.6, and 3.2.6 contain SQL Injection Vulnerabilities: CVE-2012-2694 and CVE-2012-2695; Upgrade to 3.2.6, 3.1.6, 3.0.14',
      :confidence => CONFIDENCE[:high],
      :file => gemfile_or_environment,
      :link_path => "https://groups.google.com/d/topic/rubyonrails-security/l4L0TEVAz1k/discussion"
  end
end

    		
  





    
      

  

  
    #check_scope_arguments(args)  ⇒ Object 
  

  

  




  

    
      


262
263
264
265
266
267
268
269
270

    		
    
      
# File 'lib/brakeman/checks/check_sql.rb', line 262

def check_scope_arguments args
  return unless node_type? args, :arglist

  if node_type? args[2], :iter
    unsafe_sql? args[2][-1]
  else
    unsafe_sql? args[2]
  end
end

    		
  





    
      

  

  
    #check_string_interp(arg)  ⇒ Object 
  

  

  




  

    

Check an interpolated string for dangerous values.



This method assumes values interpolated into strings are unsafe by default, unless safe_value? explicitly returns true.



  





  



  

    
      


378
379
380
381
382
383
384
385
386

    		
    
      
# File 'lib/brakeman/checks/check_sql.rb', line 378

def check_string_interp arg
  arg.each do |exp|
    if node_type?(exp, :string_eval, :evstr) and not safe_value? exp.value
      return exp.value
    end
  end

  nil
end

    		
  





    
      

  

  
    #constantize_call?(result)  ⇒ Boolean 
  

  

  




  

    

Look for something like this:



params.constantize.find(‘something’)



s(:call,



s(:call,
  s(:call,
    s(:call, nil, :params, s(:arglist)),
    :[],
    s(:arglist, s(:lit, :x))),
  :constantize,
  s(:arglist)),
:find,
s(:arglist, s(:str, "something")))




  





  

Returns:


    
  
    
  • 
      
      
        (Boolean)
      
      
      
    
    • 
  





  

    
      


565
566
567
568

    		
    
      
# File 'lib/brakeman/checks/check_sql.rb', line 565

def constantize_call? result
  call = result[:call]
  call? call.target and call.target.method == :constantize
end

    		
  





    
      

  

  
    #find_dangerous_value(exp, ignore_hash)  ⇒ Object 
  

  

  




  

    

Check exp for dangerous values. Used by unsafe_sql?



  





  



  

    
      


405
406
407
408
409
410
411
412
413
414
415
416
417
418
419
420
421
422
423
424
425
426
427
428
429
430
431
432
433
434
435
436
437
438
439
440
441
442
443
444
445
446

    		
    
      
# File 'lib/brakeman/checks/check_sql.rb', line 405

def find_dangerous_value exp, ignore_hash
  case exp.node_type
  when :lit, :str, :const, :colon2, :true, :false, :nil
    nil
  when :array
    #Assume this is an array like
    #
    #  ["blah = ? AND thing = ?", ...]
    #
    #and check first value

    unsafe_sql? exp[1]
  when :string_interp, :dstr
    check_string_interp exp
  when :hash
    check_hash_values exp unless ignore_hash
  when :if
    unsafe_sql? exp.then_clause or unsafe_sql? exp.else_clause
  when :call
    unless IGNORE_METHODS_IN_SQL.include? exp.method
      if has_immediate_user_input? exp or has_immediate_model? exp
        exp
      else
        check_call exp
      end
    end
  when :or
    if unsafe = (unsafe_sql?(exp.lhs) || unsafe_sql?(exp.rhs))
      return unsafe
    else
      nil
    end
  when :block, :rlist
    unsafe_sql? exp.last
  else
    if has_immediate_user_input? exp or has_immediate_model? exp
      exp
    else
      nil
    end
  end
end

    		
  





    
      

  

  
    #find_scope_calls  ⇒ Object 
  

  

  




  

    

Find calls to named_scope() or scope() in models RP 3 TODO



  





  



  

    
      


57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92

    		
    
      
# File 'lib/brakeman/checks/check_sql.rb', line 57

def find_scope_calls
  scope_calls = []

  if version_between? "2.1.0", "3.0.9"
    active_record_models.each do |name, model|
      if model[:options][:named_scope]
        model[:options][:named_scope].each do |args|
          call = Sexp.new(:call, nil, :named_scope, args).line(args.line)
          scope_calls << { :call => call, :location => [:class, name ], :method => :named_scope }
        end
      end
     end
  elsif version_between? "3.1.0", "3.9.9"
    active_record_models.each do |name, model|
      if model[:options][:scope]
        model[:options][:scope].each do |args|
          second_arg = args[2]

          next unless sexp? second_arg

          if second_arg.node_type == :iter and node_type? second_arg.block, :block, :call
            process_scope_with_block name, args
          elsif second_arg.node_type == :call
            call = second_arg
            scope_calls << { :call => call, :location => [:class, name ], :method => call.method }
          else
            call = Sexp.new(:call, nil, :scope, args).line(args.line)
            scope_calls << { :call => call, :location => [:class, name ], :method => :scope }
          end
        end
      end
    end
  end

  scope_calls
end

    		
  





    
      

  

  
    #process_result(result)  ⇒ Object 
  

  

  




  

    

Process possible SQL injection sites:



Model#find



Model#(named_)scope



Model#(find|count)_by_sql



Model#all



Rails 3



Model#(where|having) Model#(order|group)



Find Options Hash



Dangerous keys that accept SQL:


  • 

    conditions
    

  • 

    order
    

  • 

    having
    

  • 

    joins
    

  • 

    select
    

  • 

    from
    

  • 

    lock
    




  





  



  

    
      


171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241

    		
    
      
# File 'lib/brakeman/checks/check_sql.rb', line 171

def process_result result
  return if duplicate? result or result[:call].original_line

  call = result[:call]
  method = call.method
  args = call.args

  dangerous_value = case method
                    when :find
                      check_find_arguments args.second
                    when :exists?
                      check_find_arguments args.first
                    when :named_scope, :scope
                      check_scope_arguments call.arglist
                    when :find_by_sql, :count_by_sql
                      check_by_sql_arguments args.first
                    when :calculate
                      check_find_arguments args[2]
                    when :last, :first, :all
                      check_find_arguments args.first
                    when :average, :count, :maximum, :minimum, :sum
                      if args.length > 2
                        unsafe_sql?(args.first) or check_find_arguments(args.last)
                      else
                        check_find_arguments args.last
                      end
                    when :where, :having
                      check_query_arguments call.arglist
                    when :order, :group, :reorder
                      check_order_arguments call.arglist
                    when :joins
                      check_joins_arguments args.first
                    when :from, :select
                      unsafe_sql? args.first
                    when :lock
                      check_lock_arguments args.first
                    else
                      Brakeman.debug "Unhandled SQL method: #{method}"
                    end

  if dangerous_value
    add_result result

    if input = include_user_input?(dangerous_value)
      confidence = CONFIDENCE[:high]
      user_input = input.match
    else
      confidence = CONFIDENCE[:med]
      user_input = dangerous_value
    end

    warn :result => result,
      :warning_type => "SQL Injection",
      :message => "Possible SQL injection",
      :user_input => user_input,
      :confidence => confidence
  end

  if check_for_limit_or_offset_vulnerability args[-1]
    if include_user_input? args[-1]
      confidence = CONFIDENCE[:high]
    else
      confidence = CONFIDENCE[:low]
    end

    warn :result => result,
      :warning_type => "SQL Injection",
      :message => "Upgrade to Rails >= 2.1.2 to escape :limit and :offset. Possible SQL injection",
      :confidence => confidence
  end
end

    		
  





    
      

  

  
    #process_scope_with_block(model_name, args)  ⇒ Object 
  

  

  




  

    
      


124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142

    		
    
      
# File 'lib/brakeman/checks/check_sql.rb', line 124

def process_scope_with_block model_name, args
  scope_name = args[1][1]
  block = args[-1][-1]

  #Search lambda for calls to query methods
  if block.node_type == :block
    find_calls = Brakeman::FindAllCalls.new tracker

    find_calls.process_source block, model_name, scope_name

    find_calls.calls.each do |call|
      if @sql_targets.include? call[:method]
        process_result call
      end
    end
  elsif block.node_type == :call
    process_result :target => block.target, :method => block.method, :call => block, :location => [:class, model_name, scope_name]
  end
end

    		
  





    
      

  

  
    #run_check  ⇒ Object 
  

  

  




  

    
      


16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53

    		
    
      
# File 'lib/brakeman/checks/check_sql.rb', line 16

def run_check
  @rails_version = tracker.config[:rails_version]

  @sql_targets = [:all, :average, :calculate, :count, :count_by_sql, :exists?,
    :find, :find_by_sql, :first, :last, :maximum, :minimum, :sum]

  if tracker.options[:rails3]
    @sql_targets.concat [:from, :group, :having, :joins, :lock, :order, :reorder, :select, :where]
  end

  Brakeman.debug "Finding possible SQL calls on models"
  calls = tracker.find_call :targets => active_record_models.keys,
    :methods => @sql_targets,
    :chained => true

  Brakeman.debug "Finding possible SQL calls with no target"
  calls.concat tracker.find_call(:target => nil, :method => @sql_targets)

  Brakeman.debug "Finding possible SQL calls using constantized()"
  calls.concat tracker.find_call(:method => @sql_targets).select { |result| constantize_call? result }

  Brakeman.debug "Finding calls to named_scope or scope"
  calls.concat find_scope_calls

  Brakeman.debug "Checking version of Rails for CVE-2012-2660"
  check_rails_version_for_cve_2012_2660

  Brakeman.debug "Checking version of Rails for CVE-2012-2661"
  check_rails_version_for_cve_2012_2661

  Brakeman.debug "Checking version of Rails for CVE-2012-2695"
  check_rails_version_for_cve_2012_2695

  Brakeman.debug "Processing possible SQL calls"
  calls.each do |c|
    process_result c
  end
end

    		
  





    
      

  

  
    #safe_value?(exp)  ⇒ Boolean 
  

  

  




  

    

  





  

Returns:


    
  
    
  • 
      
      
        (Boolean)
      
      
      
    
    • 
  





  

    
      


505
506
507
508
509
510
511
512
513
514
515
516
517
518
519
520
521
522

    		
    
      
# File 'lib/brakeman/checks/check_sql.rb', line 505

def safe_value? exp
  return true unless sexp? exp

  case exp.node_type
  when :str, :lit, :const, :colon2, :nil, :true, :false
    true
  when :call
    IGNORE_METHODS_IN_SQL.include? exp.method
  when :if
    safe_value? exp.then_clause and safe_value? exp.else_clause
  when :block, :rlist
    safe_value? exp.last
  when :or
    safe_value? exp.lhs and safe_value? exp.rhs
  else
    false
  end
end

    		
  





    
      

  

  
    #unsafe_sql?(exp, ignore_hash = false)  ⇒ Boolean 
  

  

  




  

    

Checks the given expression for unsafe SQL values. If an unsafe value is found, returns that value (may be the given exp or a subexpression).



Otherwise, returns false/nil.



  





  

Returns:


    
  
    
  • 
      
      
        (Boolean)
      
      
      
    
    • 
  





  

    
      


392
393
394
395
396
397
398
399
400
401
402

    		
    
      
# File 'lib/brakeman/checks/check_sql.rb', line 392

def unsafe_sql? exp, ignore_hash = false
  return unless sexp? exp

  dangerous_value = find_dangerous_value exp, ignore_hash

  if safe_value? dangerous_value
    false
  else
    dangerous_value
  end
end