hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-27 09:45:15 +02:00
Code Issues Packages Projects Releases Wiki Activity

improve some rails framework tests

Browse Source
This commit is contained in:
Alex Ford
2021-06-29 13:56:28 +01:00
parent 31cbf818ab
commit 3f76075fe6
3 changed files with 126 additions and 49 deletions
Download Patch File Download Diff File Expand all files Collapse all files

44
ql/test/library-tests/frameworks/ActionController.expected
View File

@@ -1,18 +1,32 @@
actionControllerControllerClasses
| ActiveRecordInjection.rb:12:1:34:3 | FooController |
| ActiveRecordInjection.rb:37:1:48:3 | BarController |
| ActiveRecordInjection.rb:50:1:51:3 | BazController |
| ActiveRecordInjection.rb:27:1:58:3 | FooController |
| ActiveRecordInjection.rb:60:1:90:3 | BarController |
| ActiveRecordInjection.rb:92:1:96:3 | BazController |
actionControllerParamsCalls
| ActiveRecordInjection.rb:19:30:19:35 | call to params |
| ActiveRecordInjection.rb:22:29:22:34 | call to params |
| ActiveRecordInjection.rb:25:31:25:36 | call to params |
| ActiveRecordInjection.rb:29:20:29:25 | call to params |
| ActiveRecordInjection.rb:32:48:32:53 | call to params |
| ActiveRecordInjection.rb:40:10:40:15 | call to params |
| ActiveRecordInjection.rb:35:30:35:35 | call to params |
| ActiveRecordInjection.rb:39:30:39:35 | call to params |
| ActiveRecordInjection.rb:43:32:43:37 | call to params |
| ActiveRecordInjection.rb:48:21:48:26 | call to params |
| ActiveRecordInjection.rb:54:34:54:39 | call to params |
| ActiveRecordInjection.rb:56:23:56:28 | call to params |
| ActiveRecordInjection.rb:56:38:56:43 | call to params |
| ActiveRecordInjection.rb:62:10:62:15 | call to params |
| ActiveRecordInjection.rb:72:11:72:16 | call to params |
| ActiveRecordInjection.rb:77:12:77:17 | call to params |
| ActiveRecordInjection.rb:83:12:83:17 | call to params |
| ActiveRecordInjection.rb:88:15:88:20 | call to params |
| ActiveRecordInjection.rb:94:22:94:27 | call to params |
actionControllerParamsSources
| ActiveRecordInjection.rb:19:30:19:35 | call to params |
| ActiveRecordInjection.rb:22:29:22:34 | call to params |
| ActiveRecordInjection.rb:25:31:25:36 | call to params |
| ActiveRecordInjection.rb:29:20:29:25 | call to params |
| ActiveRecordInjection.rb:32:48:32:53 | call to params |
| ActiveRecordInjection.rb:40:10:40:15 | call to params |
| ActiveRecordInjection.rb:35:30:35:35 | call to params |
| ActiveRecordInjection.rb:39:30:39:35 | call to params |
| ActiveRecordInjection.rb:43:32:43:37 | call to params |
| ActiveRecordInjection.rb:48:21:48:26 | call to params |
| ActiveRecordInjection.rb:54:34:54:39 | call to params |
| ActiveRecordInjection.rb:56:23:56:28 | call to params |
| ActiveRecordInjection.rb:56:38:56:43 | call to params |
| ActiveRecordInjection.rb:62:10:62:15 | call to params |
| ActiveRecordInjection.rb:72:11:72:16 | call to params |
| ActiveRecordInjection.rb:77:12:77:17 | call to params |
| ActiveRecordInjection.rb:83:12:83:17 | call to params |
| ActiveRecordInjection.rb:88:15:88:20 | call to params |
| ActiveRecordInjection.rb:94:22:94:27 | call to params |

60
ql/test/library-tests/frameworks/ActiveRecord.expected
View File

@@ -1,28 +1,46 @@
activeRecordModelClasses
| ActiveRecordInjection.rb:1:1:3:3 | UserGroup |
| ActiveRecordInjection.rb:5:1:7:3 | User |
| ActiveRecordInjection.rb:9:1:10:3 | Admin |
| ActiveRecordInjection.rb:5:1:17:3 | User |
| ActiveRecordInjection.rb:19:1:25: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 | ... + ... |
| ActiveRecordInjection.rb:10:33:10:67 | "name='#{...}' and pass='#{...}'" |
| ActiveRecordInjection.rb:23:17:23:25 | condition |
| ActiveRecordInjection.rb:35:30:35:44 | ...[...] |
| ActiveRecordInjection.rb:39:21:39:43 | "id = '#{...}'" |
| ActiveRecordInjection.rb:43:23:43:45 | "id = '#{...}'" |
| ActiveRecordInjection.rb:47:16:47:21 | <<-SQL |
| ActiveRecordInjection.rb:54:20:54:47 | "user.id = '#{...}'" |
| ActiveRecordInjection.rb:68:21:68:33 | ... + ... |
| ActiveRecordInjection.rb:75:16:75:28 | "name #{...}" |
| ActiveRecordInjection.rb:80:20:80:39 | "username = #{...}" |
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 |
| ActiveRecordInjection.rb:28:5:28:35 | call to where |
| ActiveRecordInjection.rb:32:5:32:27 | call to joins |
| ActiveRecordInjection.rb:32:5:32:61 | call to where |
| ActiveRecordInjection.rb:45:5:45:34 | call to delete_all |
| ActiveRecordInjection.rb:10:5:10:68 | call to find |
| ActiveRecordInjection.rb:15:5:15:40 | call to find_by |
| ActiveRecordInjection.rb:15:5:15:46 | call to users |
| ActiveRecordInjection.rb:23:5:23:26 | call to destroy_all |
| ActiveRecordInjection.rb:35:5:35:45 | call to calculate |
| ActiveRecordInjection.rb:39:5:39:44 | call to delete_all |
| ActiveRecordInjection.rb:43:5:43:47 | call to destroy_all |
| ActiveRecordInjection.rb:47:5:47:35 | call to where |
| ActiveRecordInjection.rb:54:5:54:14 | call to where |
| ActiveRecordInjection.rb:54:5:54:48 | call to not |
| ActiveRecordInjection.rb:56:5:56:51 | call to authenticate |
| ActiveRecordInjection.rb:68:5:68:34 | call to delete_all |
| ActiveRecordInjection.rb:75:5:75:29 | call to order |
| ActiveRecordInjection.rb:80:7:80:40 | call to find_by |
| ActiveRecordInjection.rb:85:5:85:33 | call to find_by |
| ActiveRecordInjection.rb:88:5:88:34 | call to find |
| ActiveRecordInjection.rb:94:5:94:46 | 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 |
| ActiveRecordInjection.rb:10:5:10:68 | call to find |
| ActiveRecordInjection.rb:23:5:23:26 | call to destroy_all |
| ActiveRecordInjection.rb:35:5:35:45 | call to calculate |
| ActiveRecordInjection.rb:39:5:39:44 | call to delete_all |
| ActiveRecordInjection.rb:43:5:43:47 | call to destroy_all |
| ActiveRecordInjection.rb:47:5:47:35 | call to where |
| ActiveRecordInjection.rb:54:5:54:48 | call to not |
| ActiveRecordInjection.rb:68:5:68:34 | call to delete_all |
| ActiveRecordInjection.rb:75:5:75:29 | call to order |
| ActiveRecordInjection.rb:80:7:80:40 | call to find_by |

71
ql/test/library-tests/frameworks/ActiveRecordInjection.rb
View File

@@ -4,9 +4,24 @@ 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
def self.from(user_group_id)
# GOOD: `find_by` with hash argument
UserGroup.find_by(id: user_group_id).users
end
end
class Admin < User
def self.delete_all(condition = nil)
# BAD: `delete_all` overrides an ActiveRecord method, but doesn't perform
# any validation before passing its arguments on to another ActiveRecord method
destroy_all(condition)
end
end
class FooController < ActionController::Base
@@ -15,37 +30,67 @@ class FooController < ActionController::Base
# A string tainted by user input is inserted into an SQL query
def some_request_handler
# SELECT AVG(#{params[:column]}) FROM "users"
# BAD: executes `SELECT AVG(#{params[:column]}) FROM "users"`
# where `params[:column]` is unsanitized
User.calculate(:average, params[:column])
# DELETE FROM "users" WHERE (id = #{params[:id]})
User.delete_all("id = #{params[:id]}")
# BAD: executes `DELETE FROM "users" WHERE (id = '#{params[:id]}')`
# where `params[:id]` is unsanitized
User.delete_all("id = '#{params[:id]}'")
# SELECT "users".* FROM "users" WHERE (id = #{params[:id]})
User.destroy_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]}'"])
# SELECT "users".* FROM "users" WHERE id BETWEEN #{params[:min_id]} AND 100000
# 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 ?
id BETWEEN '#{params[:min_id]}' AND ?
SQL
UserGroup.joins(:users).where("user.id = #{params[:id]}")
# 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}'"
# DELETE FROM "users" WHERE (id = #{uid})
User.delete_all("id = " + uid)
# BAD: executes `DELETE FROM "users" WHERE (id = #{uid})`
# where `uid` is unsantized
User.delete_all("id " + uidEq)
end
def safe_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)
# OK: `find` method is overridden in `User`
User.find(params[:user_group])
end
end
class BazController < BarController
def yet_another_handler
Admin.delete_all(params[:admin_condition])
end
end