Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Fixes incorrectly used for any* #73

Closed
wants to merge 6 commits into from

2 participants

@LTe

No description provided.

@dmajda

I think you should leave any(*) call here because there may be any number of arguments before the opions hash for some finder methods. For example, this is a valid (although unlikely) call:

User.find(1, 2, 3, 4, :conditions => { :admin => false })
@dmajda

Here any{1} is OK (both send_file and send_data take just 1 argument before the options) but you can write just any here (without the count).

@dmajda
Owner

Please fix the issues I pointed out in a new or rebased pull request. Otherwise the code looks OK.

@LTe

I think we should use any{1,}

Because with any* we also recognize

find(:limit => params[:input])
@dmajda
Owner

@LTe Yeah, you're right.

@LTe LTe closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
9 lib/scanny/checks/denial_of_service_check.rb
@@ -22,16 +22,15 @@ def pattern_find_with_like
SendWithArguments<
arguments = ActualArguments<
array = [
- any*,
+ any{odd},
HashLiteral<
array = [
- any*,
+ any{even},
SymbolLiteral<value = :limit | :conditions>,
StringLiteral<string *= 'LIKE'>,
- any*
+ any{even}
]
- >,
- any*
+ >
]
>,
name *= /^find/
View
9 lib/scanny/checks/sql_injection/find_method_check.rb
@@ -57,17 +57,16 @@ def pattern_find_by_with_conditions
SendWithArguments<
arguments = ActualArguments<
array = [
- any*,
+ any{1},
HashLiteral<
array = [
- any*,
+ any{even},
SymbolLiteral<
value = :conditions
>,
- any*
+ any{odd}
]
- >,
- any*
+ >
]
>,
name = :find
View
9 lib/scanny/checks/sql_injection/find_method_with_dynamic_string_check.rb
@@ -22,16 +22,15 @@ def pattern_find_by_with_conditions_dynamic_string
SendWithArguments<
arguments = ActualArguments<
array = [
- any*,
+ any{1},
HashLiteral<
array = [
- any*,
+ any{even},
SymbolLiteral<value = :conditions>,
DynamicString,
- any*
+ any{even}
]
- >,
- any*
+ >
]
>,
name = :find
View
10 lib/scanny/checks/sql_injection/find_method_with_params_check.rb
@@ -45,20 +45,18 @@ def pattern_find_with_conditions_and_params_or_limit
SendWithArguments<
arguments = ActualArguments<
array = [
- any*,
+ any{1},
HashLiteral<
array = [
- any*,
+ any{even},
SymbolLiteral<value = :limit | :conditions>,
- any*,
SendWithArguments<
name = :[],
receiver = Send<name = :params | :session>
>,
- any*
+ any{even}
]
- >,
- any*
+ >
]
>,
name = :find>
View
8 lib/scanny/checks/verify_check.rb
@@ -22,15 +22,13 @@ def pattern_verify
SendWithArguments<
arguments = ActualArguments<
array = [
- any*,
HashLiteral<
array = [
- any*,
+ any{even},
SymbolLiteral<value = :method>,
- any*
+ any{odd}
]
- >,
- any*
+ >
]
>,
name = :verify
View
1  lib/scanny/checks/xss/xss_send_check.rb
@@ -25,6 +25,7 @@ def pattern_send
name = :send_file | :send_data,
arguments = ActualArguments<
array = [
+ any{1},
HashLiteral<
array = [
SymbolLiteral<value = :disposition>,
View
8 spec/scanny/checks/denial_of_service_check_spec.rb
@@ -12,11 +12,17 @@ module Scanny::Checks
it "reports \"User.find(:first, :conditions => \"name LIKE '%bob%'\" )\" correctly" do
@runner.should check("User.find(:first, :conditions => \"name LIKE '%bob%'\" )").
with_issue(@issue)
+ @runner.should check("User.find(:conditions => \"name LIKE '%bob%'\")").
+ without_issues
end
it "reports \"User.find(:first, :limit => \"name LIKE '%bob%'\" )\" correctly" do
@runner.should check("User.find(:first, :limit => \"name LIKE '%bob%'\" )").
- with_issue(@issue)
+ with_issue(@issue)
+ @runner.should check("User.find(:limit => \"name LIKE '%bob%'\")").
+ without_issues
end
+
+
end
end
View
10 spec/scanny/checks/sql_injection/find_method_check_spec.rb
@@ -14,6 +14,16 @@ module Scanny::Checks::Sql
with_issue(@issue_low)
end
+ it "does not report \"find\" calls without first argument" do
+ @runner.should check("find(:conditions => { :id => 10 })").
+ without_issues
+ end
+
+ it "does not report \"find\" with wrong key" do
+ @runner.should check("find(:first, :hello => :conditions)").
+ without_issues
+ end
+
it "reports \"find\" calls with :conditions key and dynamic value correctly" do
@runner.should check('find(:first, :conditions => "#{method}")').
with_issue(@issue_low)
View
11 spec/scanny/checks/sql_injection/find_method_with_dynamic_string_check_spec.rb
@@ -11,8 +11,17 @@ module Scanny::Checks::Sql
it "reports \"find\" calls with :conditions key and dynamic value correctly" do
@runner.should check('find(:first, :conditions => "#{method}")').
- with_issue(@issue_medium)
+ with_issue(@issue_medium)
@runner.should check('find(:first, :conditions => "normal_string")').without_issues
end
+
+ it "does not report \"find\" calls without first argument" do
+ @runner.should check("find(:conditions => :value)").without_issues
+ end
+
+ it "does not report \"find\" calls with incorrect hash value" do
+ @runner.should check('find(:first, "#{conditions}" => :value)').
+ without_issues
+ end
end
end
View
10 spec/scanny/checks/sql_injection/find_method_with_params_check_spec.rb
@@ -37,6 +37,16 @@ module Scanny::Checks::Sql
with_issue(@issue_high)
end
+ it "does not report \"find\" calls when no first argument is given" do
+ @runner.should check("find(:limit => session[:password])").
+ without_issues
+ end
+
+ it "does not report \"find\" when hash keys are incorrect" do
+ @runner.should check("find(:first, :key => :limit, params[:hello] => :value)").
+ without_issues
+ end
+
it "reports \"execute\" calls on class with params correctly" do
@runner.should check('User.execute params[:password]').with_issue(@issue_high)
end
View
5 spec/scanny/checks/verify_check_spec.rb
@@ -18,5 +18,10 @@ module Scanny::Checks
@runner.should check("verify :params => 'user', :only => :update_password").
without_issues
end
+
+ it "does not report \"verify :argument, :method => :post\"" do
+ @runner.should check("verify :argument, :method => :post").
+ without_issues
+ end
end
end
View
8 spec/scanny/checks/xss/xss_send_check_spec.rb
@@ -9,13 +9,13 @@ module Scanny::Checks
end
it "reports \"send_file :disposition => 'inline'\" correctly" do
- @runner.should check("send_file :disposition => 'inline' ").with_issue(@issue)
- @runner.should check("send_file :disposition => 'attachment' ").without_issues
+ @runner.should check("send_file 'file', :disposition => 'inline' ").with_issue(@issue)
+ @runner.should check("send_file 'file', :disposition => 'attachment' ").without_issues
end
it "reports \"send_data :disposition => 'inline'\" correctly" do
- @runner.should check("send_data :disposition => 'inline' ").with_issue(@issue)
- @runner.should check("send_data :disposition => 'attachment' ").without_issues
+ @runner.should check("send_data 'file', :disposition => 'inline' ").with_issue(@issue)
+ @runner.should check("send_data 'file', :disposition => 'attachment' ").without_issues
end
end
end
Something went wrong with that request. Please try again.