Skip to content

Commit 5a15558

Browse files
committed
Ruby: treat an Arel.sql call as a SqlConstruction
1 parent c9d3494 commit 5a15558

File tree

5 files changed

+32
-0
lines changed

5 files changed

+32
-0
lines changed

ruby/ql/lib/codeql/ruby/frameworks/Arel.qll

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66

77
private import codeql.ruby.ApiGraphs
88
private import codeql.ruby.dataflow.FlowSummary
9+
private import codeql.ruby.Concepts
10+
private import codeql.ruby.DataFlow
911

1012
/**
1113
* Provides modeling for Arel, a low level SQL library that powers ActiveRecord.
@@ -28,4 +30,14 @@ module Arel {
2830
input = "Argument[0]" and output = "ReturnValue" and preservesValue = false
2931
}
3032
}
33+
34+
/** A call to `Arel.sql`, considered as a SQL construction. */
35+
private class ArelSqlConstruction extends SqlConstruction::Range, DataFlow::CallNode {
36+
ArelSqlConstruction() {
37+
this = DataFlow::getConstant("Arel").getAMethodCall() and
38+
this.getMethodName() = "sql"
39+
}
40+
41+
override DataFlow::Node getSql() { result = this.getArgument(0) }
42+
}
3143
}
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
| arel.rb:3:8:3:18 | call to sql | arel.rb:3:17:3:17 | x |
2+
| arel.rb:8:8:8:18 | call to sql | arel.rb:8:17:8:17 | x |
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
import codeql.ruby.Concepts
2+
import codeql.ruby.DataFlow
3+
4+
query predicate sqlConstructions(SqlConstruction c, DataFlow::Node sql) { sql = c.getSql() }
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
2+
class PotatoController < ActionController::Base
3+
def unsafe_action
4+
name = params[:user_name]
5+
# BAD: SQL statement constructed from user input
6+
sql = Arel.sql("SELECT * FROM users WHERE name = #{name}")
7+
end
8+
end

ruby/ql/test/query-tests/security/cwe-089/SqlInjection.expected

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ edges
3333
| ActiveRecordInjection.rb:137:21:137:44 | ...[...] : | ActiveRecordInjection.rb:20:22:20:30 | condition : |
3434
| ActiveRecordInjection.rb:151:59:151:64 | call to params : | ActiveRecordInjection.rb:151:59:151:74 | ...[...] : |
3535
| ActiveRecordInjection.rb:151:59:151:74 | ...[...] : | ActiveRecordInjection.rb:151:27:151:76 | "this is an unsafe annotation:..." |
36+
| ArelInjection.rb:4:12:4:17 | call to params : | ArelInjection.rb:4:12:4:29 | ...[...] : |
37+
| ArelInjection.rb:4:12:4:29 | ...[...] : | ArelInjection.rb:6:20:6:61 | "SELECT * FROM users WHERE nam..." |
3638
nodes
3739
| ActiveRecordInjection.rb:8:25:8:28 | name : | semmle.label | name : |
3840
| ActiveRecordInjection.rb:8:31:8:34 | pass : | semmle.label | pass : |
@@ -85,6 +87,9 @@ nodes
8587
| ActiveRecordInjection.rb:151:27:151:76 | "this is an unsafe annotation:..." | semmle.label | "this is an unsafe annotation:..." |
8688
| ActiveRecordInjection.rb:151:59:151:64 | call to params : | semmle.label | call to params : |
8789
| ActiveRecordInjection.rb:151:59:151:74 | ...[...] : | semmle.label | ...[...] : |
90+
| ArelInjection.rb:4:12:4:17 | call to params : | semmle.label | call to params : |
91+
| ArelInjection.rb:4:12:4:29 | ...[...] : | semmle.label | ...[...] : |
92+
| ArelInjection.rb:6:20:6:61 | "SELECT * FROM users WHERE nam..." | semmle.label | "SELECT * FROM users WHERE nam..." |
8893
subpaths
8994
#select
9095
| ActiveRecordInjection.rb:10:33:10:67 | "name='#{...}' and pass='#{...}'" | ActiveRecordInjection.rb:70:23:70:28 | call to params : | ActiveRecordInjection.rb:10:33:10:67 | "name='#{...}' and pass='#{...}'" | This SQL query depends on a $@. | ActiveRecordInjection.rb:70:23:70:28 | call to params | user-provided value |
@@ -105,3 +110,4 @@ subpaths
105110
| ActiveRecordInjection.rb:92:21:92:35 | ...[...] | ActiveRecordInjection.rb:92:21:92:26 | call to params : | ActiveRecordInjection.rb:92:21:92:35 | ...[...] | This SQL query depends on a $@. | ActiveRecordInjection.rb:92:21:92:26 | call to params | user-provided value |
106111
| ActiveRecordInjection.rb:104:20:104:32 | ... + ... | ActiveRecordInjection.rb:98:10:98:15 | call to params : | ActiveRecordInjection.rb:104:20:104:32 | ... + ... | This SQL query depends on a $@. | ActiveRecordInjection.rb:98:10:98:15 | call to params | user-provided value |
107112
| ActiveRecordInjection.rb:151:27:151:76 | "this is an unsafe annotation:..." | ActiveRecordInjection.rb:151:59:151:64 | call to params : | ActiveRecordInjection.rb:151:27:151:76 | "this is an unsafe annotation:..." | This SQL query depends on a $@. | ActiveRecordInjection.rb:151:59:151:64 | call to params | user-provided value |
113+
| ArelInjection.rb:6:20:6:61 | "SELECT * FROM users WHERE nam..." | ArelInjection.rb:4:12:4:17 | call to params : | ArelInjection.rb:6:20:6:61 | "SELECT * FROM users WHERE nam..." | This SQL query depends on a $@. | ArelInjection.rb:4:12:4:17 | call to params | user-provided value |

0 commit comments

Comments
 (0)