Merge remote-tracking branch 'origin/main' into regex

2026-04-28 10:15:14 +02:00 · 2021-06-25 15:00:26 +01:00
parent bee94757dd fa5e7cb9cc
commit ba7021086b
24 changed files with 494 additions and 47 deletions
--- a/ql/test/library-tests/frameworks/ActiveRecord.expected
+++ b/ql/test/library-tests/frameworks/ActiveRecord.expected
@@ -3,11 +3,15 @@ activeRecordModelClasses
 | ActiveRecordInjection.rb:5:1:7:3 | User |
 | ActiveRecordInjection.rb:9:1:10:3 | Admin |
 activeRecordSqlExecutionRanges
+| ActiveRecordInjection.rb:19:30:19:44 | ...[...] |
 | ActiveRecordInjection.rb:22:21:22:41 | "id = #{...}" |
+| ActiveRecordInjection.rb:25:23:25:43 | "id = #{...}" |
 | ActiveRecordInjection.rb:28:16:28:21 | <<-SQL |
 | ActiveRecordInjection.rb:32:35:32:60 | "user.id = #{...}" |
 | ActiveRecordInjection.rb:45:21:45:33 | ... + ... |
 activeRecordModelClassMethodCalls
+| ActiveRecordInjection.rb:2:3:2:17 | call to has_many |
+| ActiveRecordInjection.rb:6:3:6:24 | call to belongs_to |
 | ActiveRecordInjection.rb:19:5:19:45 | call to calculate |
 | ActiveRecordInjection.rb:22:5:22:42 | call to delete_all |
 | ActiveRecordInjection.rb:25:5:25:45 | call to destroy_all |
@@ -16,7 +20,9 @@ activeRecordModelClassMethodCalls
 | ActiveRecordInjection.rb:32:5:32:61 | call to where |
 | ActiveRecordInjection.rb:45:5:45:34 | call to delete_all |
 potentiallyUnsafeSqlExecutingMethodCall
+| ActiveRecordInjection.rb:19:5:19:45 | call to calculate |
 | ActiveRecordInjection.rb:22:5:22:42 | call to delete_all |
+| ActiveRecordInjection.rb:25:5:25:45 | call to destroy_all |
 | ActiveRecordInjection.rb:28:5:28:35 | call to where |
 | ActiveRecordInjection.rb:32:5:32:61 | call to where |
 | ActiveRecordInjection.rb:45:5:45:34 | call to delete_all |
--- a/ql/test/query-tests/security/cwe-089/ActiveRecordInjection.rb
+++ b/ql/test/query-tests/security/cwe-089/ActiveRecordInjection.rb
@@ -0,0 +1,83 @@
+class UserGroup < ActiveRecord::Base
+  has_many :users
+end
+
+class User < ApplicationRecord
+  belongs_to :user_group
+
+  def self.authenticate(name, pass)
+    # BAD: possible untrusted input interpolated into SQL fragment
+    find(:first, :conditions => "name='#{name}' and pass='#{pass}'")
+  end
+end
+
+class Admin < User
+end
+
+class FooController < ActionController::Base
+
+  MAX_USER_ID = 100_000
+
+  # A string tainted by user input is inserted into an SQL query
+  def some_request_handler
+    # BAD: executes `SELECT AVG(#{params[:column]}) FROM "users"`
+    # where `params[:column]` is unsanitized
+    User.calculate(:average, params[:column])
+
+    # BAD: executes `DELETE FROM "users" WHERE (id = '#{params[:id]}')`
+    # where `params[:id]` is unsanitized
+    User.delete_all("id = '#{params[:id]}'")
+
+    # BAD: executes `SELECT "users".* FROM "users" WHERE (id = '#{params[:id]}')`
+    # where `params[:id]` is unsanitized
+    User.destroy_all(["id = '#{params[:id]}'"])
+
+    # BAD: executes `SELECT "users".* FROM "users" WHERE id BETWEEN '#{params[:min_id]}' AND 100000`
+    # where `params[:min_id]` is unsanitized
+    User.where(<<-SQL, MAX_USER_ID)
+      id BETWEEN '#{params[:min_id]}' AND ?
+    SQL
+
+    # BAD: chained method case
+    # executes `SELECT "users".* FROM "users" WHERE (NOT (user_id = 'params[:id]'))`
+    # where `params[:id]` is unsanitized
+    User.where.not("user.id = '#{params[:id]}'")
+
+    User.authenticate(params[:name], params[:pass])
+  end
+end
+
+
+class BarController < ApplicationController
+
+  def some_other_request_handler
+    ps = params
+    uid = ps[:id]
+    uidEq = "= '#{uid}'"
+
+    # BAD: executes `DELETE FROM "users" WHERE (id = #{uid})`
+    # where `uid` is unsantized
+    User.delete_all("id " + uidEq)
+  end
+
+  def sanitized_paths
+
+    dir = params[:order]
+    # GOOD: barrier guard prevents taint flow
+    dir = "DESC" unless dir == "ASC"
+    User.order("name #{dir}")
+
+    name = params[:user_name]
+    # GOOD: barrier guard prevents taint flow
+    if %w(alice bob charlie).include? name
+      User.find_by("username = #{name}")
+    end
+
+    name = params[:user_name]
+    # GOOD: hash arguments are sanitized by ActiveRecord
+    User.find_by(user_name: name)
+  end
+end
+
+class BazController < BarController
+end
--- a/ql/test/query-tests/security/cwe-089/SqlInjection.expected
+++ b/ql/test/query-tests/security/cwe-089/SqlInjection.expected
@@ -0,0 +1,42 @@
+edges
+| ActiveRecordInjection.rb:8:25:8:28 | name :  | ActiveRecordInjection.rb:10:33:10:67 | "name='#{...}' and pass='#{...}'" |
+| ActiveRecordInjection.rb:8:31:8:34 | pass :  | ActiveRecordInjection.rb:10:33:10:67 | "name='#{...}' and pass='#{...}'" |
+| ActiveRecordInjection.rb:25:30:25:35 | call to params :  | ActiveRecordInjection.rb:25:30:25:44 | ...[...] |
+| ActiveRecordInjection.rb:29:30:29:35 | call to params :  | ActiveRecordInjection.rb:29:21:29:43 | "id = '#{...}'" |
+| ActiveRecordInjection.rb:33:32:33:37 | call to params :  | ActiveRecordInjection.rb:33:23:33:45 | "id = '#{...}'" |
+| ActiveRecordInjection.rb:38:21:38:26 | call to params :  | ActiveRecordInjection.rb:37:16:37:21 | <<-SQL |
+| ActiveRecordInjection.rb:44:34:44:39 | call to params :  | ActiveRecordInjection.rb:44:20:44:47 | "user.id = '#{...}'" |
+| ActiveRecordInjection.rb:46:23:46:28 | call to params :  | ActiveRecordInjection.rb:46:23:46:35 | ...[...] :  |
+| ActiveRecordInjection.rb:46:23:46:35 | ...[...] :  | ActiveRecordInjection.rb:8:25:8:28 | name :  |
+| ActiveRecordInjection.rb:46:38:46:43 | call to params :  | ActiveRecordInjection.rb:46:38:46:50 | ...[...] :  |
+| ActiveRecordInjection.rb:46:38:46:50 | ...[...] :  | ActiveRecordInjection.rb:8:31:8:34 | pass :  |
+| ActiveRecordInjection.rb:54:10:54:15 | call to params :  | ActiveRecordInjection.rb:60:21:60:33 | ... + ... |
+nodes
+| ActiveRecordInjection.rb:8:25:8:28 | name :  | semmle.label | name :  |
+| ActiveRecordInjection.rb:8:31:8:34 | pass :  | semmle.label | pass :  |
+| ActiveRecordInjection.rb:10:33:10:67 | "name='#{...}' and pass='#{...}'" | semmle.label | "name='#{...}' and pass='#{...}'" |
+| ActiveRecordInjection.rb:25:30:25:35 | call to params :  | semmle.label | call to params :  |
+| ActiveRecordInjection.rb:25:30:25:44 | ...[...] | semmle.label | ...[...] |
+| ActiveRecordInjection.rb:29:21:29:43 | "id = '#{...}'" | semmle.label | "id = '#{...}'" |
+| ActiveRecordInjection.rb:29:30:29:35 | call to params :  | semmle.label | call to params :  |
+| ActiveRecordInjection.rb:33:23:33:45 | "id = '#{...}'" | semmle.label | "id = '#{...}'" |
+| ActiveRecordInjection.rb:33:32:33:37 | call to params :  | semmle.label | call to params :  |
+| ActiveRecordInjection.rb:37:16:37:21 | <<-SQL | semmle.label | <<-SQL |
+| ActiveRecordInjection.rb:38:21:38:26 | call to params :  | semmle.label | call to params :  |
+| ActiveRecordInjection.rb:44:20:44:47 | "user.id = '#{...}'" | semmle.label | "user.id = '#{...}'" |
+| ActiveRecordInjection.rb:44:34:44:39 | call to params :  | semmle.label | call to params :  |
+| ActiveRecordInjection.rb:46:23:46:28 | call to params :  | semmle.label | call to params :  |
+| ActiveRecordInjection.rb:46:23:46:35 | ...[...] :  | semmle.label | ...[...] :  |
+| ActiveRecordInjection.rb:46:38:46:43 | call to params :  | semmle.label | call to params :  |
+| ActiveRecordInjection.rb:46:38:46:50 | ...[...] :  | semmle.label | ...[...] :  |
+| ActiveRecordInjection.rb:54:10:54:15 | call to params :  | semmle.label | call to params :  |
+| ActiveRecordInjection.rb:60:21:60:33 | ... + ... | semmle.label | ... + ... |
+#select
+| ActiveRecordInjection.rb:10:33:10:67 | "name='#{...}' and pass='#{...}'" | ActiveRecordInjection.rb:46:23:46:28 | call to params :  | ActiveRecordInjection.rb:10:33:10:67 | "name='#{...}' and pass='#{...}'" | This SQL query depends on $@. | ActiveRecordInjection.rb:46:23:46:28 | call to params | a user-provided value |
+| ActiveRecordInjection.rb:10:33:10:67 | "name='#{...}' and pass='#{...}'" | ActiveRecordInjection.rb:46:38:46:43 | call to params :  | ActiveRecordInjection.rb:10:33:10:67 | "name='#{...}' and pass='#{...}'" | This SQL query depends on $@. | ActiveRecordInjection.rb:46:38:46:43 | call to params | a user-provided value |
+| ActiveRecordInjection.rb:25:30:25:44 | ...[...] | ActiveRecordInjection.rb:25:30:25:35 | call to params :  | ActiveRecordInjection.rb:25:30:25:44 | ...[...] | This SQL query depends on $@. | ActiveRecordInjection.rb:25:30:25:35 | call to params | a user-provided value |
+| ActiveRecordInjection.rb:29:21:29:43 | "id = '#{...}'" | ActiveRecordInjection.rb:29:30:29:35 | call to params :  | ActiveRecordInjection.rb:29:21:29:43 | "id = '#{...}'" | This SQL query depends on $@. | ActiveRecordInjection.rb:29:30:29:35 | call to params | a user-provided value |
+| ActiveRecordInjection.rb:33:23:33:45 | "id = '#{...}'" | ActiveRecordInjection.rb:33:32:33:37 | call to params :  | ActiveRecordInjection.rb:33:23:33:45 | "id = '#{...}'" | This SQL query depends on $@. | ActiveRecordInjection.rb:33:32:33:37 | call to params | a user-provided value |
+| ActiveRecordInjection.rb:37:16:37:21 | <<-SQL | ActiveRecordInjection.rb:38:21:38:26 | call to params :  | ActiveRecordInjection.rb:37:16:37:21 | <<-SQL | This SQL query depends on $@. | ActiveRecordInjection.rb:38:21:38:26 | call to params | a user-provided value |
+| ActiveRecordInjection.rb:44:20:44:47 | "user.id = '#{...}'" | ActiveRecordInjection.rb:44:34:44:39 | call to params :  | ActiveRecordInjection.rb:44:20:44:47 | "user.id = '#{...}'" | This SQL query depends on $@. | ActiveRecordInjection.rb:44:34:44:39 | call to params | a user-provided value |
+| ActiveRecordInjection.rb:60:21:60:33 | ... + ... | ActiveRecordInjection.rb:54:10:54:15 | call to params :  | ActiveRecordInjection.rb:60:21:60:33 | ... + ... | This SQL query depends on $@. | ActiveRecordInjection.rb:54:10:54:15 | call to params | a user-provided value |
--- a/ql/test/query-tests/security/cwe-089/SqlInjection.qlref
+++ b/ql/test/query-tests/security/cwe-089/SqlInjection.qlref
@@ -0,0 +1 @@
+queries/security/cwe-089/SqlInjection.ql