diff --git a/rules/ruby/rails/sql_injection.yml b/rules/ruby/rails/sql_injection.yml index d6fe4dc4..db8edbbe 100644 --- a/rules/ruby/rails/sql_injection.yml +++ b/rules/ruby/rails/sql_injection.yml @@ -1,112 +1,126 @@ imports: - ruby_shared_common_external_input patterns: - - pattern: $<_>.$($<...>$$<...>) - filters: - - variable: METHOD - detection: ruby_rails_sql_injection_regular_method - scope: cursor - - variable: USER_INPUT - detection: ruby_rails_sql_injection_external_input - scope: result - - pattern: $($$<...>) - filters: - - variable: METHOD - detection: ruby_rails_sql_injection_regular_method - scope: cursor - - variable: USER_INPUT - detection: ruby_rails_sql_injection_external_input - scope: result - - pattern: ActiveRecord::Base.connection.$($$<...>) + - pattern: ActiveRecord::Base.connection.$($$<...>) filters: - variable: CONN_METHOD detection: ruby_rails_sql_injection_connection_method scope: cursor - - variable: USER_INPUT + - variable: EXTERNAL_INPUT detection: ruby_rails_sql_injection_external_input scope: result - - pattern: connection.$($$<...>) + - pattern: connection.$($$<...>) filters: - variable: CONN_METHOD detection: ruby_rails_sql_injection_connection_method scope: cursor - - variable: USER_INPUT + - variable: EXTERNAL_INPUT detection: ruby_rails_sql_injection_external_input scope: result - - pattern: $<_>.$($$<...>) + - pattern: $<_>.$($$<...>) filters: - variable: SPECIAL_METHOD detection: ruby_rails_sql_injection_special_arg_method scope: cursor - - variable: USER_INPUT + - variable: EXTERNAL_INPUT detection: ruby_rails_sql_injection_external_input scope: result - not: - variable: USER_INPUT + variable: EXTERNAL_INPUT detection: ruby_rails_sql_injection_safe_special_arg - - pattern: $($$<...>) + - pattern: $($$<...>) filters: - variable: SPECIAL_METHOD detection: ruby_rails_sql_injection_special_arg_method scope: cursor - - variable: USER_INPUT + - variable: EXTERNAL_INPUT detection: ruby_rails_sql_injection_external_input scope: result - not: - variable: USER_INPUT + variable: EXTERNAL_INPUT detection: ruby_rails_sql_injection_safe_special_arg + - pattern: $<_>.$($<...>$$<...>) + filters: + - variable: METHOD + detection: ruby_rails_sql_injection_regular_method + scope: cursor + - variable: EXTERNAL_INPUT + detection: ruby_rails_sql_injection_external_input + scope: result + - pattern: $($<...>$$<...>) + filters: + - variable: METHOD + detection: ruby_rails_sql_injection_regular_method + scope: cursor + - variable: EXTERNAL_INPUT + detection: ruby_rails_sql_injection_external_input + scope: result + - pattern: $.$($<...>$$<...>) + filters: + - variable: ACTIVE_RECORD + detection: ruby_rails_sql_injection_active_record + scope: cursor + - variable: SHARED_METHOD + detection: ruby_rails_sql_injection_shared_method + scope: cursor + - variable: EXTERNAL_INPUT + detection: ruby_rails_sql_injection_external_input + scope: result auxiliary: - id: ruby_rails_sql_injection_regular_method patterns: - - pattern: $ - filters: - - variable: METHOD - values: - - joins - - select - - reselect - - group - - having - - order - - reorder - - minimum - - maximum - - calculate - - count - - count_by_sql - - sum - - average + - joins + - group + - having + - order + - reorder + - minimum + - maximum + - count_by_sql + - average + - id: ruby_rails_sql_injection_shared_method + patterns: + - count + - select + - sum - id: ruby_rails_sql_injection_connection_method patterns: - - pattern: $ - filters: - - variable: CONN_METHOD - values: - - execute - - exec_delete - - exec_insert - - exec_query - - exec_update - - select_all - - select_one - - select_rows - - select_value - - select_values + - execute + - exec_delete + - exec_insert + - exec_query + - exec_update + - select_all + - select_one + - select_rows + - select_value + - select_values - id: ruby_rails_sql_injection_special_arg_method patterns: - - pattern: $ + - find_by + - find_by! + - find_by_sql + - find_sole_by + - from + - where + - delete_by + - destroy_by + - update_all + - id: ruby_rails_sql_injection_active_record + patterns: + - pattern: $.$<_>($<...>) filters: - - variable: SPECIAL_METHOD - values: - - find_by - - find_by! - - find_by_sql - - find_sole_by - - from - - where - - delete_by - - destroy_by - - update_all + - variable: ACTIVE_RECORD + detection: ruby_rails_sql_injection_active_record + scope: cursor + - pattern: $ + filters: + - variable: MODEL_NAME + regex: \A[A-Z][a-zA-Z]*[a-z]+\z + - pattern: $<_>.$ + filters: + - variable: ASSOCIATION + regex: \A\w*_?s\z - id: ruby_rails_sql_injection_external_input sanitizer: ruby_rails_sql_injection_sanitized patterns: diff --git a/tests/ruby/rails/sql_injection/test.js b/tests/ruby/rails/sql_injection/test.js index 04083059..962cb88b 100644 --- a/tests/ruby/rails/sql_injection/test.js +++ b/tests/ruby/rails/sql_injection/test.js @@ -7,7 +7,6 @@ const { ruleId, ruleFile, testBase } = getEnvironment(__dirname) describe(ruleId, () => { const invoke = createNewInvoker(ruleId, ruleFile, testBase) - test("injected_params", () => { const testCase = "injected_params.rb" @@ -16,7 +15,7 @@ describe(ruleId, () => { expect(results.Missing).toEqual([]) expect(results.Extra).toEqual([]) }) - + test("ok_sanitized", () => { const testCase = "ok_sanitized.rb" @@ -26,7 +25,7 @@ describe(ruleId, () => { expect(results.Missing).toEqual([]) expect(results.Extra).toEqual([]) }) - + test("ok_using_bind", () => { const testCase = "ok_using_bind.rb" @@ -36,5 +35,14 @@ describe(ruleId, () => { expect(results.Missing).toEqual([]) expect(results.Extra).toEqual([]) }) - + + test("shared methods", () => { + const testCase = "shared_methods.rb" + + const results = invoke(testCase) + + expect(results.Missing).toEqual([]) + expect(results.Extra).toEqual([]) + }) + }) \ No newline at end of file diff --git a/tests/ruby/rails/sql_injection/testdata/injected_params.rb b/tests/ruby/rails/sql_injection/testdata/injected_params.rb index 7a4c67db..56c534a1 100644 --- a/tests/ruby/rails/sql_injection/testdata/injected_params.rb +++ b/tests/ruby/rails/sql_injection/testdata/injected_params.rb @@ -4,8 +4,6 @@ find_by!("oops #{params[:oops]}") # bearer:expected ruby_rails_sql_injection User.joins("INNER JOIN #{params[:oops]}") -# bearer:expected ruby_rails_sql_injection -select("#{params[:oops]} AS oops") # chained case # bearer:expected ruby_rails_sql_injection diff --git a/tests/ruby/rails/sql_injection/testdata/shared_methods.rb b/tests/ruby/rails/sql_injection/testdata/shared_methods.rb new file mode 100644 index 00000000..ab1bcdcf --- /dev/null +++ b/tests/ruby/rails/sql_injection/testdata/shared_methods.rb @@ -0,0 +1,20 @@ + +# bearer:expected ruby_rails_sql_injection +User.where(username: "mish").count(params[:col]) + +# bearer:expected ruby_rails_sql_injection +user.posts.count(params[:col]) + +# safe uses of shared method +my_arr = [1, 2, 3, 2] +# ok +my_arr.count(params[:item]) + +ITEMS = [:apple, :carrot, "orange"] +ITEMS.count +ITEMS.count(:apple) + +# bearer:expected ruby_rails_sql_injection +User.select("#{params[:oops]} AS oops") +# bearer:expected ruby_rails_sql_injection +user.posts.select("#{params[:oops]} AS oops")