Spree 0.60 Vulnerability Fix

gistfile1.diff

From 6e07361e1dc08ace0fd38d39067d0d7e3da11e9c Mon Sep 17 00:00:00 2001
From: Ryan Bigg <radarlistener@gmail.com>
Date: Thu, 14 Jun 2012 11:10:01 +1000
Subject: [PATCH] Explicitly define scopes as being searchable This stops
 people using class methods or scopes that shouldn't be used
 for searching.

Props to joernchen from Phenoelit for informing us about this!

Remove conditions_any scope and use ARel query building instead
---
 core/app/models/product.rb       |   35 ++++++++--
 core/app/models/product_group.rb |    2 +-
 core/lib/product_filters.rb      |   26 +++++--
 core/lib/scopes/product.rb       |  143 ++++++++++++++++++-------------------
 4 files changed, 117 insertions(+), 89 deletions(-)

diff --git a/core/app/models/product.rb b/core/app/models/product.rb
index cdb906f..808d8ce 100644
--- a/core/app/models/product.rb
+++ b/core/app/models/product.rb
@@ -67,18 +67,34 @@ class Product < ActiveRecord::Base
 
   alias :options :product_option_types
 
+  cattr_accessor :search_scopes do
+    []
+  end
+
+  def self.add_search_scope(name, &block)
+    define_singleton_method name.intern, &block
+    search_scopes << name.intern
+  end
+
   include ::Scopes::Product
 
   #RAILS3 TODO -  scopes are duplicated here and in scopres/product.rb - can we DRY it up?
   # default product scope only lists available and non-deleted products
-  scope :not_deleted,     where("products.deleted_at is NULL")
+  add_search_scope :not_deleted do
+    where("products.deleted_at is NULL")
+  end
 
-  scope :available,       lambda { |*on| where("products.available_on <= ?", on.first || Time.zone.now ) }
+  add_search_scope :available do |*on|
+    where("products.available_on <= ?", on.first || Time.zone.now)
+  end
 
-  #RAILS 3 TODO - this scope doesn't match the original 2.3.x version, needs attention (but it works)
-  scope :active,          lambda{ not_deleted.available }
+  add_search_scope :active do
+    not_deleted.available
+  end
 
-  scope :on_hand,         where("products.count_on_hand > 0")
+  add_search_scope :on_hand do
+    where("products.count_on_hand > 0")
+  end
 
   if (ActiveRecord::Base.connection.adapter_name == 'PostgreSQL')
     if ActiveRecord::Base.connection.tables.include?("products")
@@ -87,11 +103,16 @@ class Product < ActiveRecord::Base
   else
     scope :group_by_products_id, { :group => "products.id" }
   end
+  search_scopes << :group_by_products_id
   search_methods :group_by_products_id
 
-  scope :id_equals, lambda { |input_id| where("products.id = ?", input_id) }
+  add_search_scope :id_equals do |input_id|
+    where("products.id = ?", input_id)
+  end
 
-  scope :taxons_name_eq, lambda { |name| joins(:taxons).where("taxons.name = ?", name) }
+  add_search_scope :taxons_name_eq do |name|
+    joins(:taxons).where("taxons.name = ?", name)
+  end
 
   # ----------------------------------------------------------------------------------------------------------
   #
diff --git a/core/app/models/product_group.rb b/core/app/models/product_group.rb
index 3127d07..bf537d0 100644
--- a/core/app/models/product_group.rb
+++ b/core/app/models/product_group.rb
@@ -90,7 +90,7 @@ class ProductGroup < ActiveRecord::Base
   end
 
   def add_scope(scope_name, arguments=[])
-    if scope_name.to_s !~ /eval|send|system|[^a-z0-9_!?]/
+    if Product.search_scopes.include?(scope_name)
       self.product_scopes << ProductScope.new({
           :name => scope_name.to_s,
           :arguments => [*arguments]
diff --git a/core/lib/product_filters.rb b/core/lib/product_filters.rb
index 047b231..5641f44 100644
--- a/core/lib/product_filters.rb
+++ b/core/lib/product_filters.rb
@@ -50,15 +50,20 @@ module ProductFilters
   Product.scope :price_range_any,
     lambda {|*opts|
       conds = opts.map {|o| ProductFilters.price_filter[:conds][o]}.reject {|c| c.nil?}
-      Product.scoped(:joins => :master).conditions_any(conds)
+      scope = conds.shift
+      conds.each do |new_scope|
+        scope = scope.or(new_scope)
+      end
+      Product.scoped(:joins => :master).where(scope)
     }
 
   def ProductFilters.price_filter
-    conds = [ [ "Under $10",    "price             <= 10" ],
-              [ "$10 - $15",    "price between 10 and 15" ],
-              [ "$15 - $18",    "price between 15 and 18" ],
-              [ "$18 - $20",    "price between 18 and 20" ],
-              [ "$20 or over",  "price             >= 20" ] ]
+    v = Spree::Variant.arel_table
+    conds = [ [ I18n.t(:under_price, :price => format_price(10))   , v[:price].lteq(10)],
+              [ "#{format_price(10)} - #{format_price(15)}"        , v[:price].in(10..15)],
+              [ "#{format_price(15)} - #{format_price(18)}"        , v[:price].in(15..18)],
+              [ "#{format_price(18)} - #{format_price(20)}"        , v[:price].in(18..20)],
+              [ I18n.t(:or_over_price, :price => format_price(20)) , v[:price].gteq(20)]]
     { :name   => "Price Range",
       :scope  => :price_range_any,
       :conds  => Hash[*conds.flatten],
@@ -84,12 +89,17 @@ module ProductFilters
     Product.scope :brand_any,
       lambda {|*opts|
         conds = opts.map {|o| ProductFilters.brand_filter[:conds][o]}.reject {|c| c.nil?}
-        Product.with_property("brand").conditions_any(conds)
+        scope = conds.shift
+        conds.each do |new_scope|
+          scope = scope.or(new_scope)
+        end
+        Product.with_property("brand").where(scope)
       }
 
     def ProductFilters.brand_filter
       brands = ProductProperty.where(:property_id => @@brand_property).map(&:value).compact.uniq
-      conds  = Hash[*brands.map {|b| [b, "product_properties.value = '#{b}'"]}.flatten]
+      pp = Spree::ProductProperty.arel_table
+      conds  = Hash[*brands.map {|b| [b, pp.value.eq(b)]}.flatten]
       { :name   => "Brands",
         :scope  => :brand_any,
         :conds  => conds,
diff --git a/core/lib/scopes/product.rb b/core/lib/scopes/product.rb
index 5717f92..89681bb 100644
--- a/core/lib/scopes/product.rb
+++ b/core/lib/scopes/product.rb
@@ -48,11 +48,16 @@ module Scopes::Product
     order_text << ((r[1] == 'ascend') ?  "asc" : "desc")
 
     Product.send(:scope, name.to_s, Product.send(:relation).order(order_text) )
+    Product.search_scopes << name.intern
   end
 
-  ::Product.scope :ascend_by_master_price, Product.joins(:variants_with_only_master).order('variants.price asc')
+  ::Product.add_search_scope :ascend_by_master_price do
+    joins(:variants_with_only_master).order('variants.price asc')
+  end
 
-  ::Product.scope :descend_by_master_price, Product.joins(:variants_with_only_master).order('variants.price desc')
+  ::Product.add_search_scope :descend_by_master_price do
+    joins(:variants_with_only_master).order('variants.price desc')
+  end
 
   ATTRIBUTE_HELPER_METHODS = {
     :with_ids => :product_picker_field
@@ -60,30 +65,33 @@ module Scopes::Product
 
   # Ryan Bates - http://railscasts.com/episodes/112
   # general merging of conditions, names following the searchlogic pattern
-  ::Product.scope :conditions, lambda { |*args| {:conditions => args}}
+  ::Product.add_search_scope :conditions do |*args|
+    where(args)
+  end
 
-  # conditions_all is a more descriptively named enhancement of the above
-  ::Product.scope :conditions_all, lambda { |*args| {:conditions => [args].flatten}}
+  ::Product.add_search_scope :conditions_all do |*args|
+    where([args].flatten)
+  end
 
   # forming the disjunction of a list of conditions (as strings)
-  ::Product.scope :conditions_any, lambda { |*args|
+  ::Product.add_search_scope :conditions_any do |*args|
     args = [args].flatten
     raise "non-strings in conditions_any" unless args.all? {|s| s.is_a? String}
-    {:conditions => args.map {|c| "(#{c})"}.join(" OR ")}
-  }
+    where(args.map {|c| "(#{c})"}.join(" OR "))
+  end
 
 
-  ::Product.scope :price_between, lambda { |low, high|
-    { :joins => :master, :conditions => ["variants.price BETWEEN ? AND ?", low, high] }
-  }
+  ::Product.add_search_scope :price_between do |low, high|
+    joins(:master).where("variants.price" => (low.to_f)..(high.to_f))
+  end
 
-  ::Product.scope :master_price_lte, lambda { |price|
-    { :joins => :master, :conditions => ["variants.price <= ?", price] }
-  }
+  ::Product.add_search_scope :master_price_lte do |price|
+    joins(:master).where("variants.price <= ?", price)
+  end
 
-  ::Product.scope :master_price_gte, lambda { |price|
-    { :joins => :master, :conditions => ["variants.price >= ?", price] }
-  }
+  ::Product.add_search_scope :master_price_gte do |price|
+    joins(:master).where("variants.price >= ?", price)
+  end
 
 
   # This scope selects products in taxon AND all its descendants
@@ -91,74 +99,66 @@ module Scopes::Product
   #
   #   Product.taxons_id_eq(x)
   #
-  ::Product.scope :in_taxon, lambda {|taxon|
-    { :joins => :taxons, :conditions => ["taxons.id IN (?) ", taxon.self_and_descendants.map(&:id)]}
-  }
+  ::Product.add_search_scope :in_taxon do |taxon|
+    joins(:taxons).where("taxons.id" => taxon.self_and_descendants.map(&:id))
+  end
 
   # This scope selects products in all taxons AND all its descendants
   # If you need products only within one taxon use
   #
   #   Product.taxons_id_eq([x,y])
   #
-  Product.scope :in_taxons, lambda {|*taxons|
+  ::Product.add_search_scope :in_taxons do |*taxons|
     taxons = get_taxons(taxons)
-    taxons.first ? prepare_taxon_conditions(taxons) : {}
-  }
+    taxons.first ? prepare_taxon_conditions(taxons) : scoped
+  end
 
   # for quick access to products in a group, WITHOUT using the association mechanism
-  Product.scope :in_cached_group, lambda {|product_group|
-    { :joins => "JOIN product_groups_products ON products.id = product_groups_products.product_id",
-      :conditions => ["product_groups_products.product_group_id = ?", product_group]
-    }
-  }
+  Product.add_search_scope :in_cached_group do |product_group|
+    joins("JOIN product_groups_products ON products.id = product_groups_products.product_id").
+    where(["product_groups_products.product_group_id = ?", product_group])
+  end
 
 
   # a scope that finds all products having property specified by name, object or id
-  Product.scope :with_property, lambda {|property|
+  Product.add_search_scope :with_property do |property|
     conditions = case property
     when String   then ["properties.name = ?", property]
     when Property then ["properties.id = ?", property.id]
     else               ["properties.id = ?", property.to_i]
     end
 
-    {
-      :joins => :properties,
-      :conditions => conditions
-    }
-  }
+    joins(:properties).
+    where(conditions)
+  end
 
   # a scope that finds all products having an option_type specified by name, object or id
-  Product.scope :with_option, lambda {|option|
+  Product.add_search_scope :with_option do |option|
     conditions = case option
     when String     then ["option_types.name = ?", option]
     when OptionType then ["option_types.id = ?",   option.id]
     else                 ["option_types.id = ?",   option.to_i]
     end
 
-    {
-      :joins => :option_types,
-      :conditions => conditions
-    }
-  }
+    joins(:option_types).
+    where(conditions)
+  end
 
   # a simple test for product with a certain property-value pairing
   # note that it can test for properties with NULL values, but not for absent values
-  Product.scope :with_property_value, lambda { |property, value|
+  Product.add_search_scope :with_property_value do |property, value|
     conditions = case property
     when String   then ["properties.name = ?", property]
     when Property then ["properties.id = ?", property.id]
     else               ["properties.id = ?", property.to_i]
     end
     conditions = ["product_properties.value = ? AND #{conditions[0]}", value, conditions[1]]
-
-    {
-      :joins => :properties,
-      :conditions => conditions
-    }
-  }
+    joins(:properties).
+    where(conditions)
+  end
 
   # a scope that finds all products having an option value specified by name, object or id
-  Product.scope :with_option_value, lambda {|option, value|
+  Product.add_search_scope :with_option_value do |option, value|
     option_type_id = case option
     when String
       option_type = OptionType.find_by_name(option) || option.to_i
@@ -172,36 +172,32 @@ module Scopes::Product
       value, option_type_id
     ]
 
-    {
-      :joins => {:variants => :option_values},
-      :conditions => conditions
-    }
-  }
+    joins(:variants => :option_values).
+    where(conditions)
+  end
 
   # finds product having option value OR product_property
-  Product.scope :with, lambda{|value|
-    {
-      :conditions => ["option_values.name = ? OR product_properties.value = ?", value, value],
-      :joins => {:variants => :option_values, :product_properties => []}
-    }
-  }
+  Product.add_search_scope :with do |value|
+    joins(:product_properties, :variants => :option_values).
+    where("option_values.name = ? OR product_properties.value = ?", value, value)
+  end
 
-  Product.scope :in_name, lambda{|words|
+  Product.add_search_scope :in_name do |words|
     Product.like_any([:name], prepare_words(words))
-  }
+  end
 
-  Product.scope :in_name_or_keywords, lambda{|words|
+  Product.add_search_scope :in_name_or_keywords do |words|
     Product.like_any([:name, :meta_keywords], prepare_words(words))
-  }
+  end
 
-  Product.scope :in_name_or_description, lambda{|words|
+  Product.add_search_scope :in_name_or_description do |words|
     Product.like_any([:name, :description, :meta_description, :meta_keywords], prepare_words(words))
-  }
+  end
 
-  Product.scope :with_ids, lambda{|ids|
+  Product.add_search_scope :with_ids do |ids|
     ids = ids.split(',') if ids.is_a?(String)
-    { :conditions => {:id => ids} }
-  }
+    where(:id => ids)
+  end
 
   # Sorts products from most popular (poularity is extracted from how many
   # times use has put product in cart, not completed orders)
@@ -211,7 +207,7 @@ module Scopes::Product
   #
   # :joins => "LEFT OUTER JOIN (SELECT line_items.variant_id as vid, COUNT(*) as cnt FROM line_items GROUP BY line_items.variant_id) AS popularity_count ON variants.id = vid",
   # :order => 'COALESCE(cnt, 0) DESC'
-  Product.scope :descend_by_popularity,
+  Product.add_search_scope :descend_by_popularity do
     {
       :joins => :master,
       :order => <<SQL
@@ -229,10 +225,11 @@ module Scopes::Product
          ), 0) DESC
 SQL
     }
+  end
 
   # Produce an array of keywords for use in scopes.
   # Always return array with at least an empty string to avoid SQL errors
-  def self.prepare_words(words)
+  def Product.prepare_words(words)
     return [''] if words.blank?
     a = words.split(/[,\s]/).map(&:strip)
     a.any? ? a : ['']
@@ -244,7 +241,7 @@ SQL
     end
   end
 
-  def self.get_taxons(*ids_or_records_or_names)
+  def Product.get_taxons(*ids_or_records_or_names)
     ids_or_records_or_names.flatten.map { |t|
       case t
       when Integer then Taxon.find_by_id(t)
@@ -259,8 +256,8 @@ SQL
   end
 
   # specifically avoid having an order for taxon search (conflicts with main order)
-  def self.prepare_taxon_conditions(taxons)
+  def Product.prepare_taxon_conditions(taxons)
     ids = taxons.map{|taxon| taxon.self_and_descendants.map(&:id)}.flatten.uniq
-    { :joins => :taxons, :conditions => ["taxons.id IN (?)", ids] }
+    joins(:taxons).where("taxons.id" => ids)
   end
 end
-- 
1.7.8.3