Permalink
Browse files

Ehanced protection

  • Loading branch information...
1 parent 24a7d88 commit 33f192ea44166270ba02f5be1804c7c403c92349 @binarylogic committed Sep 2, 2008
View
@@ -14,14 +14,14 @@ Here's where you get aroused...
# app/controllers/users_controller.rb
def index
- @search = User.new_search(:conditions => params[:search])
+ @search = User.new_search(:conditions => params[:conditions])
@users, @users_count = @search.all, @search.count
end
Now your view:
# app/views/users/index.html.erb
- <%= form_for :search, @search.conditions, :url => users_path do |f| %>
+ <%= form_for :conditions, @search.conditions, :url => users_path do |f| %>
<%= f.text_field :first_name_contains %>
<%= f.calendar_date_select :created_after %> # nice rails plugin for replacing date_select
<% f.fields_for :orders, f.object.orders do |orders_f| %>
@@ -196,27 +196,24 @@ Not only can you use searchgasm when searching, but you can use it when setting
named_scope :sexy, :conditions => {:first_name => "Ben", email_ends_with => "binarylogic.com"}, :per_page => 20
end
-## Always use protection (searching with params)
+## Always use protection...against SQL injections
-If there is one thing we in sex ed. it's to always use protection. As I mentioned above the purpose of this plugin was to create a search object and use it in form\_for or fields\_for. What about receiving the params in the controller and protecting against SQL injection?
+If there is one thing we all know, it's to always use protection. That's why searchgasm protects you by default. The new\_search and new\_conditions methods are protected by default. This means that various checks are done to ensure it is not possible to perform any type of SQL injection. But this also limits how you can search, meaning you can't write raw SQL. If you want to be daring and search without protection prepare to accept the consequences. All that you have to do is add ! to the end of the methods: new\_search! and new\_conditions!. That was an awkward paragraph. Let's move on.
- accounts = Account.find_with_protection(params[:search])
- accounts = Account.all_with_protection(params[:search])
- account = Account.first_with_protection(params[:search])
+### Protected from SQL injections
-For the lazy programmer:
+ search = Account.new_search(params[:search])
+ conditions = Account.new_conditions(params[:conditions])
- accounts = Account.findwp(params[:search])
- accounts = Account.allwp(params[:search])
- account = Account.firstwp(params[:search])
+### *NOT* protected from SQL injections
-This performs various checks to ensure SQL injection is impossible. I'm sure you know this, but I have to say it: *DO NOT* pass params into the "find", "all", or "first" methods, otherwise you are opening yourself up to SQL injections.
-
- # DO NOT DO THIS!
+ accounts = Account.find(params[:search])
accounts = Account.all(params[:search])
-
- # OR THIS!
- accounts = Account.all(:conditions => params[:conditions])
+ account = Account.first(params[:search])
+ search = Account.new_search!(params[:search])
+ conditions = Account.new_conditions!(params[:conditions])
+
+Lesson learned: use new\_search and new\_conditions when passing in params as *ANY* of the options.
## Available Conditions
View
@@ -11,7 +11,7 @@ Echoe.new 'searchgasm' do |p|
p.summary = "Orgasmic ActiveRecord searching"
p.description = "Makes ActiveRecord searching easier, robust, and powerful. Automatic conditions, pagination support, object based searching, and more."
p.url = "http://github.com/binarylogic/searchgasm"
- p.dependencies = %w(activerecord)
+ p.dependencies = %w(activerecord activesupport)
p.include_rakefile = true
end
View
@@ -1,5 +1,4 @@
require "active_record"
-require "searchgasm/active_record/protection"
require "searchgasm/active_record/base"
require "searchgasm/active_record/associations"
require "searchgasm/version"
@@ -3,24 +3,34 @@ module Searchgasm
module ActiveRecord
module Associations
module AssociationCollection
- def self.included(klass)
- klass.class_eval do
- include Protection
- end
- end
-
def find_with_searchgasm(*args)
options = args.extract_options!
args << sanitize_options_with_searchgasm(options)
find_without_searchgasm(*args)
end
def build_conditions(options = {}, &block)
- @reflection.klass.build_conditions(options.merge(:scope => scope(:find)[:conditions]), &block)
+ conditions = @reflection.klass.build_conditions(options, &block)
+ conditions.scope = scope(:find)[:conditions]
+ conditions
+ end
+
+ def build_conditions!(options = {}, &block)
+ conditions = @reflection.klass.build_conditions!(options, &block)
+ conditions.scope = scope(:find)[:conditions]
+ conditions
end
def build_search(options = {}, &block)
- @reflection.klass.build_search(options.merge(:scope => scope(:find)[:conditions]), &block)
+ conditions = @reflection.klass.build_search(options, &block)
+ conditions.scope = scope(:find)[:conditions]
+ conditions
+ end
+
+ def build_search!(options = {}, &block)
+ conditions = @reflection.klass.build_search!(options, &block)
+ conditions.scope = scope(:find)[:conditions]
+ conditions
end
end
@@ -2,49 +2,62 @@ module BinaryLogic
module Searchgasm
module ActiveRecord
module Base
- def self.included(klass)
- klass.class_eval do
- alias_method :new_conditions, :build_conditions
- alias_method :new_search, :build_search
- include Protection
- end
- end
-
def calculate_with_searchgasm(*args)
options = args.extract_options!
options = sanitize_options_with_searchgasm(options)
args << options
calculate_without_searchgasm(*args)
end
-
+
def find_with_searchgasm(*args)
options = args.extract_options!
options = sanitize_options_with_searchgasm(options)
args << options
find_without_searchgasm(*args)
end
-
+
def scope_with_searchgasm(method, key = nil)
scope = scope_without_searchgasm(method, key)
- return sanitize_options_with_searchgasm(scope) if key.nil? && method == :find
+ return sanitize_options_with_searchgasm(scope) if key.nil? && method == :find && !scope.blank?
scope
end
-
- def build_conditions(options = {})
- BinaryLogic::Searchgasm::Search::Conditions(self, options)
+
+ def build_conditions(values = {}, &block)
+ conditions = searchgasm_conditions({})
+ conditions.protect = true
+ conditions.value = values
+ yield conditions if block_given?
+ conditions
end
-
- def build_search(options = {})
- searcher = searchgasm_searcher(options)
- yield searcher if block_given?
- searcher
+
+ def build_conditions!(values = {}, &block)
+ conditions = searchgasm_conditions(values)
+ yield conditions if block_given?
+ conditions
end
-
+
+ def build_search(options = {}, &block)
+ options[:protect] = true
+ search = searchgasm_searcher(options)
+ yield search if block_given?
+ search
+ end
+
+ def build_search!(options = {}, &block)
+ search = searchgasm_searcher(options)
+ yield search if block_given?
+ search
+ end
+
private
def sanitize_options_with_searchgasm(options)
searchgasm_searcher(options).sanitize
end
-
+
+ def searchgasm_conditions(options)
+ BinaryLogic::Searchgasm::Search::Conditions.new(self, options)
+ end
+
def searchgasm_searcher(options)
BinaryLogic::Searchgasm::Search::Base.new(self, options)
end
@@ -61,6 +74,10 @@ class << self
alias_method_chain :calculate, :searchgasm
alias_method_chain :find, :searchgasm
alias_method_chain :scope, :searchgasm
+ alias_method :new_conditions, :build_conditions
+ alias_method :new_conditions!, :build_conditions!
+ alias_method :new_search, :build_search
+ alias_method :new_search!, :build_search!
def valid_find_options
VALID_FIND_OPTIONS
@@ -1,37 +0,0 @@
-module BinaryLogic
- module Searchgasm
- module ActiveRecord
- module Protection
- def self.included(klass)
- klass.class_eval do
- alias_method :new_search, :build_search
- alias_method :findwp, :find_with_protection
- alias_method :allwp, :all_with_protection
- alias_method :firstwp, :first_with_protection
- end
- end
-
- def find_with_protection(*args)
- options = args.extract_options!
- options.merge!(:protect => true)
- args << options
- find(*args)
- end
-
- def all_with_protection(*args)
- options = args.extract_options!
- options.merge!(:protect => true)
- args << options
- all(*args)
- end
-
- def first_with_protection(*args)
- options = args.extract_options!
- options.merge!(:protect => true)
- args << options
- first(*args)
- end
- end
- end
- end
-end
@@ -1,14 +1,16 @@
module BinaryLogic
module Searchgasm
- module Search
+ module Search
class Base
include Utilities
SPECIAL_FIND_OPTIONS = [:order_by, :order_as, :page, :per_page]
VALID_FIND_OPTIONS = ::ActiveRecord::Base.valid_find_options + SPECIAL_FIND_OPTIONS
+ SAFE_OPTIONS = SPECIAL_FIND_OPTIONS + [:conditions, :limit, :offset]
+ VULNERABLE_OPTIONS = VALID_FIND_OPTIONS - SAFE_OPTIONS
attr_accessor :klass
- attr_reader :conditions
+ attr_reader :conditions, :protect
attr_writer :options
def initialize(klass, options = {})
@@ -18,10 +20,12 @@ def initialize(klass, options = {})
end
(::ActiveRecord::Base.valid_find_options - [:conditions]).each do |option|
- class_eval <<-SRC
+ src = <<-SRC
def #{option}(sanitize = false); options[:#{option}]; end
def #{option}=(value); self.options[:#{option}] = value; end
SRC
+
+ class_eval src
end
alias_method :per_page, :limit
@@ -61,6 +65,11 @@ def include(sanitize = false)
includes.blank? ? nil : (includes.size == 1 ? includes.first : includes)
end
+ def inspect
+ options_as_nice_string = ::ActiveRecord::Base.valid_find_options.collect { |name| "#{name}: #{send(name)}" }.join(", ")
+ "#<#{klass} #{options_as_nice_string}>"
+ end
+
def limit=(value)
return options[:limit] = nil if value.nil? || value == 0
@@ -75,9 +84,12 @@ def options
@options ||= {}
end
- def options=(options)
- return unless options.is_a?(Hash)
- options.each { |option, value| send("#{option}=", value) }
+ def options=(values)
+ return unless values.is_a?(Hash)
+ self.protect = values.delete(:protect) if values.has_key?(:protect) # make sure we do this first
+ values.symbolize_keys.assert_valid_keys(VALID_FIND_OPTIONS)
+ frisk!(values) if protect?
+ values.each { |option, value| send("#{option}=", value) }
end
def order_as
@@ -114,6 +126,15 @@ def page=(value)
value
end
+ def protect=(value)
+ conditions.protect = value
+ @protect = value
+ end
+
+ def protect?
+ protect == true
+ end
+
def reset!
conditions.reset!
self.options = {}
@@ -136,6 +157,31 @@ def scope
def scope=(value)
conditions.scope = value
end
+
+ private
+ def order_by_safe?(order_by)
+ return true if order_by.blank?
+
+ column_names = klass.column_names
+
+ [order_by].flatten.each do |column|
+ case column
+ when Hash
+ return false unless order_by_safe?(column.to_a)
+ when Array
+ return false unless order_by_safe?(column)
+ else
+ return false unless column_names.include?(column)
+ end
+ end
+
+ true
+ end
+
+ def frisk!(options)
+ options.symbolize_keys.assert_valid_keys(SAFE_OPTIONS)
+ raise(ArgumentError, ":order_by can only contain colum names in the string, hash, or array format") unless order_by_safe?(options[:order_by])
+ end
end
end
end
Oops, something went wrong.

0 comments on commit 33f192e

Please sign in to comment.