Commit 99c09876b4462346155288ff5c2cc57f21e70599
Exists in
theme-brasil-digital-from-staging
and in
9 other branches
Merge branch 'api' into production
Showing
2 changed files
with
54 additions
and
3 deletions
Show diff stats
lib/noosfero/api/helpers.rb
... | ... | @@ -160,8 +160,27 @@ require 'grape' |
160 | 160 | conditions |
161 | 161 | end |
162 | 162 | |
163 | - def make_order_with_parameters(params) | |
164 | - params[:order] || "created_at DESC" | |
163 | + # changing make_order_with_parameters to avoid sql injection | |
164 | + def make_order_with_parameters(object, method, params) | |
165 | + order = "created_at DESC" | |
166 | + unless params[:order].blank? | |
167 | + if params[:order].include? '\'' or params[:order].include? '"' | |
168 | + order = "created_at DESC" | |
169 | + elsif ['RANDOM()', 'RANDOM'].include? params[:order].upcase | |
170 | + order = 'RANDOM()' | |
171 | + else | |
172 | + field_name, direction = params[:order].split(' ') | |
173 | + assoc = object.class.reflect_on_association(method.to_sym) | |
174 | + if !field_name.blank? and assoc | |
175 | + if assoc.klass.attribute_names.include? field_name | |
176 | + if direction.present? and ['ASC','DESC'].include? direction.upcase | |
177 | + order = "#{field_name} #{direction.upcase}" | |
178 | + end | |
179 | + end | |
180 | + end | |
181 | + end | |
182 | + end | |
183 | + return order | |
165 | 184 | end |
166 | 185 | |
167 | 186 | def by_reference(scope, params) |
... | ... | @@ -176,7 +195,7 @@ require 'grape' |
176 | 195 | |
177 | 196 | def select_filtered_collection_of(object, method, params) |
178 | 197 | conditions = make_conditions_with_parameter(params) |
179 | - order = make_order_with_parameters(params) | |
198 | + order = make_order_with_parameters(object,method,params) | |
180 | 199 | |
181 | 200 | objects = object.send(method) |
182 | 201 | objects = by_reference(objects, params) | ... | ... |
test/unit/api/helpers_test.rb
... | ... | @@ -161,6 +161,38 @@ class APIHelpersTest < ActiveSupport::TestCase |
161 | 161 | assert_nil make_conditions_with_parameter[:type] |
162 | 162 | end |
163 | 163 | |
164 | + #test_should_make_order_with_parameters_return_order_if attribute_is_found_at_object_association | |
165 | + should 'make_order_with_parameters return order if attribute is found at object association' do | |
166 | + environment = Environment.new | |
167 | + params = {:order => "name ASC"} | |
168 | + assert_equal "name ASC", make_order_with_parameters(environment, "articles", params) | |
169 | + end | |
170 | + | |
171 | + # test added to check for eventual sql injection vunerabillity | |
172 | + #test_should_make_order_with_parameters_return_default_order_if_attributes_not_exists | |
173 | + should 'make_order_with_parameters return default order if attributes not exists' do | |
174 | + environment = Environment.new | |
175 | + params = {:order => "CRAZY_FIELD ASC"} # quote used to check sql injection vunerabillity | |
176 | + assert_equal "created_at DESC", make_order_with_parameters(environment, "articles", params) | |
177 | + end | |
178 | + | |
179 | + should 'make_order_with_parameters return default order if sql injection detected' do | |
180 | + environment = Environment.new | |
181 | + params = {:order => "name' ASC"} # quote used to check sql injection vunerabillity | |
182 | + assert_equal "created_at DESC", make_order_with_parameters(environment, "articles", params) | |
183 | + end | |
184 | + | |
185 | + should 'make_order_with_parameters return RANDOM() if random is passed' do | |
186 | + environment = Environment.new | |
187 | + params = {:order => "random"} # quote used to check sql injection vunerabillity | |
188 | + assert_equal "RANDOM()", make_order_with_parameters(environment, "articles", params) | |
189 | + end | |
190 | + | |
191 | + should 'make_order_with_parameters return RANDOM() if random function is passed' do | |
192 | + environment = Environment.new | |
193 | + params = {:order => "random()"} # quote used to check sql injection vunerabillity | |
194 | + assert_equal "RANDOM()", make_order_with_parameters(environment, "articles", params) | |
195 | + end | |
164 | 196 | |
165 | 197 | should 'render not_found if endpoint is unavailable' do |
166 | 198 | Noosfero::API::API.stubs(:endpoint_unavailable?).returns(true) | ... | ... |