CVE-2012-5648: Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection') (CWE-89)
value: Multiple SQL injection vulnerabilities in Foreman before 1.0.2 allow remote attackers to execute arbitrary SQL commands via unspecified parameters to (1) app/models/hostext/search.rb or (2) app/models/puppetclass.rb, related to the search mechanism.
CWE description: The product constructs all or part of an SQL command using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the intended SQL command when it is sent to a downstream component. Without sufficient removal or quoting of SQL syntax in user-controllable inputs, the generated SQL query can cause those inputs to be interpreted as SQL instead of ordinary user data.
Published: 2014-04-04T14:55Z
Repository: https://github.com/theforeman/foreman
Language: Ruby
Files changed: app/models/hostext/search.rb, app/models/puppetclass.rb
Commit date: 2012-12-19 12:38:18 +0200
Commit message: Fix Foreman SQL injection through search mechanism CVE-2012-5648

Diff: diff --git a/app/models/hostext/search.rb b/app/models/hostext/search.rb
index e4e18c44..50b5ee4a 100644
--- a/app/models/hostext/search.rb
+++ b/app/models/hostext/search.rb
@@ -49,19 +49,20 @@ module Hostext
           scoped_search :in => :search_users, :on => :mail,      :complete_value => true, :only_explicit => true, :rename => :'user.mail',     :operators => ['= ', '~ '], :ext_method => :search_by_user
         end
 
         def self.search_by_user(key, operator, value)
           key_name = key.sub(/^.*\./,'')
-          users = User.all(:conditions => "#{key_name} #{operator} '#{value_to_sql(operator, value)}'")
+          condition = sanitize_sql_for_conditions(["? #{operator} ?", key_name, value_to_sql(operator, value)])
+          users = User.all(:conditions => condition)
           hosts = users.map(&:hosts).flatten
-          opts  = hosts.empty? ? "= 'nil'" : "IN (#{hosts.map(&:id).join(',')})"
+          opts  = hosts.empty? ? "< 0" : "IN (#{hosts.map(&:id).join(',')})"
 
           return {:conditions => " hosts.id #{opts} " }
         end
 
         def self.search_by_puppetclass(key, operator, value)
-          conditions  = "puppetclasses.name #{operator} '#{value_to_sql(operator, value)}'"
+          conditions  = sanitize_sql_for_conditions(["puppetclasses.name #{operator} ?", value_to_sql(operator, value)])
           hosts       = Host.my_hosts.all(:conditions => conditions, :joins => :puppetclasses, :select => 'DISTINCT hosts.id').map(&:id)
           host_groups = Hostgroup.all(:conditions => conditions, :joins => :puppetclasses, :select => 'DISTINCT hostgroups.id').map(&:id)
 
           opts = ''
           opts += "hosts.id IN(#{hosts.join(',')})"             unless hosts.blank?
@@ -71,16 +72,18 @@ module Hostext
           return {:conditions => opts, :include => :hostgroup}
         end
 
         def self.search_by_params(key, operator, value)
           key_name = key.sub(/^.*\./,'')
-          opts     = {:conditions => "name = '#{key_name}' and value #{operator} '#{value_to_sql(operator, value)}'", :order => :priority}
+          condition = sanitize_sql_for_conditions(["name = ? and value #{operator} ?", key_name, value_to_sql(operator, value)])
+          opts     = {:conditions => condition, :order => :priority}
           p        = Parameter.all(opts)
           return {:conditions => '1 = 0'} if p.blank?
 
           max         = p.first.priority
-          negate_opts = {:conditions => "name = '#{key_name}' and NOT(value #{operator} '#{value_to_sql(operator, value)}') and priority > #{max}", :order => :priority}
+          condition   = sanitize_sql_for_conditions(["name = ? and NOT(value #{operator} ?) and priority > ?",key_name,value_to_sql(operator, value), max])
+          negate_opts = {:conditions => condition, :order => :priority}
           n           = Parameter.all(negate_opts)
 
           conditions = param_conditions(p)
           negate = param_conditions(n)
 
diff --git a/app/models/puppetclass.rb b/app/models/puppetclass.rb
index 10cc1d53..e7744655 100644
--- a/app/models/puppetclass.rb
+++ b/app/models/puppetclass.rb
@@ -149,12 +149,12 @@ class Puppetclass < ActiveRecord::Base
   def as_json(options={})
     super({:only => [:name, :id], :include => [:lookup_keys]})
   end
 
   def self.search_by_host(key, operator, value)
-    conditions = "hosts.name #{operator} '#{value_to_sql(operator, value)}'"
-    direct     = Puppetclass.all(:conditions => conditions, :joins => :hosts, :select => 'puppetclasses.id').map(&:id).uniq
+    conditions = sanitize_sql_for_conditions(["hosts.name #{operator} ?", value_to_sql(operator, value)])
+    direct     = Puppetclass.joins(:hosts).where(conditions).select('puppetclasses.id').map(&:id).uniq
     hostgroup  = Hostgroup.joins(:hosts).where(conditions).first
     indirect   = HostgroupClass.where(:hostgroup_id => hostgroup.path_ids).pluck(:puppetclass_id).uniq
     return { :conditions => "1=0" } if direct.blank? && indirect.blank?
 
     puppet_classes = (direct + indirect).uniq

Vulnerable code:           users = User.all(:conditions => "#{key_name} #{operator} '#{value_to_sql(operator, value)}'")
          opts  = hosts.empty? ? "= 'nil'" : "IN (#{hosts.map(&:id).join(',')})"
          conditions  = "puppetclasses.name #{operator} '#{value_to_sql(operator, value)}'"
          opts     = {:conditions => "name = '#{key_name}' and value #{operator} '#{value_to_sql(operator, value)}'", :order => :priority}
          negate_opts = {:conditions => "name = '#{key_name}' and NOT(value #{operator} '#{value_to_sql(operator, value)}') and priority > #{max}", :order => :priority}
    conditions = "hosts.name #{operator} '#{value_to_sql(operator, value)}'"
    direct     = Puppetclass.all(:conditions => conditions, :joins => :hosts, :select => 'puppetclasses.id').map(&:id).uniq
Fixed code:           condition = sanitize_sql_for_conditions(["? #{operator} ?", key_name, value_to_sql(operator, value)])
          users = User.all(:conditions => condition)
          opts  = hosts.empty? ? "< 0" : "IN (#{hosts.map(&:id).join(',')})"
          conditions  = sanitize_sql_for_conditions(["puppetclasses.name #{operator} ?", value_to_sql(operator, value)])
          condition = sanitize_sql_for_conditions(["name = ? and value #{operator} ?", key_name, value_to_sql(operator, value)])
          opts     = {:conditions => condition, :order => :priority}
          condition   = sanitize_sql_for_conditions(["name = ? and NOT(value #{operator} ?) and priority > ?",key_name,value_to_sql(operator, value), max])
          negate_opts = {:conditions => condition, :order => :priority}
    conditions = sanitize_sql_for_conditions(["hosts.name #{operator} ?", value_to_sql(operator, value)])
    direct     = Puppetclass.joins(:hosts).where(conditions).select('puppetclasses.id').map(&:id).uniq