hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-25 16:55:19 +02:00
Code Issues Packages Projects Releases Wiki Activity

Simplify guard in SQL injection tests

Browse Source 
We don't (yet) properly sanitize taint in cases like this

    foo = "A" unless foo == "B"

So for now, use a simpler guard in the SQL injection test.
We can resurrect the old, more idiomatic guard when we can support it.
This commit is contained in:
Harry Maclean
2021-09-10 16:27:57 +01:00
parent 56983565fe
commit 8f36b0d7fe
2 changed files with 14 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
ql/test/query-tests/security/cwe-089/ActiveRecordInjection.rb
View File

@@ -71,8 +71,15 @@ class BarController < ApplicationController
def safe_paths
dir = params[:order]
# GOOD: barrier guard prevents taint flow
dir = "DESC" unless dir == "ASC"
User.order("name #{dir}")
if dir == "ASC"
User.order("name #{dir}")
else
dir = "DESC"
User.order("name #{dir}")
end
# TODO: a more idiomatic form of this guard is the following:
# dir = "DESC" unless dir == "ASC"
# but our taint tracking can't (yet) handle that properly
name = params[:user_name]
# GOOD: barrier guard prevents taint flow

10
ql/test/query-tests/security/cwe-089/SqlInjection.expected
View File

@@ -12,8 +12,8 @@ edges
| ActiveRecordInjection.rb:56:38:56:43 | call to params : | ActiveRecordInjection.rb:56:38:56:50 | ...[...] : |
| ActiveRecordInjection.rb:56:38:56:50 | ...[...] : | ActiveRecordInjection.rb:8:31:8:34 | pass : |
| ActiveRecordInjection.rb:62:10:62:15 | call to params : | ActiveRecordInjection.rb:68:21:68:33 | ... + ... |
| ActiveRecordInjection.rb:94:22:94:27 | call to params : | ActiveRecordInjection.rb:94:22:94:45 | ...[...] : |
| ActiveRecordInjection.rb:94:22:94:45 | ...[...] : | ActiveRecordInjection.rb:20:23:20:31 | condition : |
| ActiveRecordInjection.rb:101:22:101:27 | call to params : | ActiveRecordInjection.rb:101:22:101:45 | ...[...] : |
| ActiveRecordInjection.rb:101:22:101:45 | ...[...] : | ActiveRecordInjection.rb:20:23:20:31 | condition : |
nodes
| ActiveRecordInjection.rb:8:25:8:28 | name : | semmle.label | name : |
| ActiveRecordInjection.rb:8:31:8:34 | pass : | semmle.label | pass : |
@@ -36,12 +36,12 @@ nodes
| ActiveRecordInjection.rb:56:38:56:50 | ...[...] : | semmle.label | ...[...] : |
| ActiveRecordInjection.rb:62:10:62:15 | call to params : | semmle.label | call to params : |
| ActiveRecordInjection.rb:68:21:68:33 | ... + ... | semmle.label | ... + ... |
| ActiveRecordInjection.rb:94:22:94:27 | call to params : | semmle.label | call to params : |
| ActiveRecordInjection.rb:94:22:94:45 | ...[...] : | semmle.label | ...[...] : |
| ActiveRecordInjection.rb:101:22:101:27 | call to params : | semmle.label | call to params : |
| ActiveRecordInjection.rb:101:22:101:45 | ...[...] : | semmle.label | ...[...] : |
#select
| ActiveRecordInjection.rb:10:33:10:67 | "name='#{...}' and pass='#{...}'" | ActiveRecordInjection.rb:56:23:56:28 | call to params : | ActiveRecordInjection.rb:10:33:10:67 | "name='#{...}' and pass='#{...}'" | This SQL query depends on $@. | ActiveRecordInjection.rb:56:23:56:28 | call to params | a user-provided value |
| ActiveRecordInjection.rb:10:33:10:67 | "name='#{...}' and pass='#{...}'" | ActiveRecordInjection.rb:56:38:56:43 | call to params : | ActiveRecordInjection.rb:10:33:10:67 | "name='#{...}' and pass='#{...}'" | This SQL query depends on $@. | ActiveRecordInjection.rb:56:38:56:43 | call to params | a user-provided value |
| ActiveRecordInjection.rb:23:17:23:25 | condition | ActiveRecordInjection.rb:94:22:94:27 | call to params : | ActiveRecordInjection.rb:23:17:23:25 | condition | This SQL query depends on $@. | ActiveRecordInjection.rb:94:22:94:27 | call to params | a user-provided value |
| ActiveRecordInjection.rb:23:17:23:25 | condition | ActiveRecordInjection.rb:101:22:101:27 | call to params : | ActiveRecordInjection.rb:23:17:23:25 | condition | This SQL query depends on $@. | ActiveRecordInjection.rb:101:22:101:27 | call to params | a user-provided value |
| ActiveRecordInjection.rb:35:30:35:44 | ...[...] | ActiveRecordInjection.rb:35:30:35:35 | call to params : | ActiveRecordInjection.rb:35:30:35:44 | ...[...] | This SQL query depends on $@. | ActiveRecordInjection.rb:35:30:35:35 | call to params | a user-provided value |
| ActiveRecordInjection.rb:39:21:39:43 | "id = '#{...}'" | ActiveRecordInjection.rb:39:30:39:35 | call to params : | ActiveRecordInjection.rb:39:21:39:43 | "id = '#{...}'" | This SQL query depends on $@. | ActiveRecordInjection.rb:39:30:39:35 | call to params | a user-provided value |
| ActiveRecordInjection.rb:43:23:43:45 | "id = '#{...}'" | ActiveRecordInjection.rb:43:32:43:37 | call to params : | ActiveRecordInjection.rb:43:23:43:45 | "id = '#{...}'" | This SQL query depends on $@. | ActiveRecordInjection.rb:43:32:43:37 | call to params | a user-provided value |