Skip to content

Commit baabd2d

Browse files
alexrfordalexrford
authored
Merge pull request github#12832 from maikypedia/maikypedia/pg-sqli
Ruby: Add SQL Injection Sinks
2 parents a6e21da + 609319d commit baabd2d

File tree

6 files changed

+184
-0
lines changed

6 files changed

+184
-0
lines changed

‎ruby/ql/lib/change-notes/2023-05-06-pg.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Support for the `pg` gem has been added. Method calls that execute queries against a PostgreSQL database that may be vulnerable to injection attacks will now be recognized.

‎ruby/ql/lib/codeql/ruby/Frameworks.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,3 +32,4 @@ private import codeql.ruby.frameworks.Slim
3232
private import codeql.ruby.frameworks.Sinatra
3333
private import codeql.ruby.frameworks.Twirp
3434
private import codeql.ruby.frameworks.Sqlite3
35+
private import codeql.ruby.frameworks.Pg

‎ruby/ql/lib/codeql/ruby/frameworks/Pg.qll

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
/**
2+
* Provides modeling for Pg, a Ruby library (gem) for interacting with PostgreSQL databases.
3+
*/
4+
5+
private import codeql.ruby.ApiGraphs
6+
private import codeql.ruby.dataflow.FlowSummary
7+
private import codeql.ruby.Concepts
8+
9+
/**
10+
* Provides modeling for Pg, a Ruby library (gem) for interacting with PostgreSQL databases.
11+
*/
12+
module Pg {
13+
/**
14+
* Flow summary for `PG.new()`. This method initializes a database connection.
15+
*/
16+
private class SqlSummary extends SummarizedCallable {
17+
SqlSummary() { this = "PG.new()" }
18+
19+
override MethodCall getACall() { result = any(PgConnection c).asExpr().getExpr() }
20+
21+
override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
22+
input = "Argument[0]" and output = "ReturnValue" and preservesValue = false
23+
}
24+
}
25+
26+
/** A call to PG::Connection.open() is used to establish a connection to a PostgreSQL database. */
27+
private class PgConnection extends DataFlow::CallNode {
28+
PgConnection() {
29+
this =
30+
API::getTopLevelMember("PG")
31+
.getMember("Connection")
32+
.getAMethodCall(["open", "new", "connect_start"])
33+
or
34+
this = API::getTopLevelMember("PG").getAnInstantiation()
35+
}
36+
}
37+
38+
/** A call that prepares an SQL statement to be executed later. */
39+
private class PgPrepareCall extends SqlConstruction::Range, DataFlow::CallNode {
40+
private DataFlow::Node query;
41+
private PgConnection pgConnection;
42+
private string queryName;
43+
44+
PgPrepareCall() {
45+
this = pgConnection.getAMethodCall("prepare") and
46+
queryName = this.getArgument(0).getConstantValue().getStringlikeValue() and
47+
query = this.getArgument(1)
48+
}
49+
50+
PgConnection getConnection() { result = pgConnection }
51+
52+
string getQueryName() { result = queryName }
53+
54+
override DataFlow::Node getSql() { result = query }
55+
}
56+
57+
/** A call that executes SQL statements against a PostgreSQL database. */
58+
private class PgExecution extends SqlExecution::Range, DataFlow::CallNode {
59+
private DataFlow::Node query;
60+
61+
PgExecution() {
62+
exists(PgConnection pgConnection |
63+
this =
64+
pgConnection.getAMethodCall(["exec", "async_exec", "exec_params", "async_exec_params"]) and
65+
query = this.getArgument(0)
66+
or
67+
exists(PgPrepareCall prepareCall |
68+
pgConnection = prepareCall.getConnection() and
69+
this.getArgument(0).getConstantValue().isStringlikeValue(prepareCall.getQueryName()) and
70+
query = prepareCall.getSql()
71+
)
72+
)
73+
}
74+
75+
override DataFlow::Node getSql() { result = query }
76+
}
77+
}

‎ruby/ql/test/library-tests/dataflow/local/TaintStep.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2814,6 +2814,7 @@
28142814
| file://:0:0:0:0 | parameter position 0 of File.realdirpath | file://:0:0:0:0 | [summary] to write: return (return) in File.realdirpath |
28152815
| file://:0:0:0:0 | parameter position 0 of File.realpath | file://:0:0:0:0 | [summary] to write: return (return) in File.realpath |
28162816
| file://:0:0:0:0 | parameter position 0 of Hash[] | file://:0:0:0:0 | [summary] read: argument position 0.any element in Hash[] |
2817+
| file://:0:0:0:0 | parameter position 0 of PG.new() | file://:0:0:0:0 | [summary] to write: return (return) in PG.new() |
28172818
| file://:0:0:0:0 | parameter position 0 of String.try_convert | file://:0:0:0:0 | [summary] to write: return (return) in String.try_convert |
28182819
| file://:0:0:0:0 | parameter position 0 of \| | file://:0:0:0:0 | [summary] read: argument position 0.any element in \| |
28192820
| file://:0:0:0:0 | parameter position 1.. of File.join | file://:0:0:0:0 | [summary] to write: return (return) in File.join |

‎ruby/ql/test/query-tests/security/cwe-089/PgInjection.rb

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
class FooController < ActionController::Base
2+
3+
def some_request_handler
4+
# A string tainted by user input is inserted into a query
5+
# (i.e a remote flow source)
6+
name = params[:name]
7+
8+
# Establish a connection to a PostgreSQL database
9+
conn = PG::Connection.open(:dbname => 'postgresql', :user => 'user', :password => 'pass', :host => 'localhost', :port => '5432')
10+
11+
# .exec() and .async_exec()
12+
# BAD: SQL statement constructed from user input
13+
qry1 = "SELECT * FROM users WHERE username = '#{name}';"
14+
conn.exec(qry1)
15+
conn.async_exec(qry1)
16+
17+
# .exec_params() and .async_exec_params()
18+
# BAD: SQL statement constructed from user input
19+
qry2 = "SELECT * FROM users WHERE username = '#{name}';"
20+
conn.exec_params(qry2)
21+
conn.async_exec_params(qry2)
22+
23+
# .exec_params() and .async_exec_params()
24+
# GOOD: SQL statement constructed from sanitized user input
25+
qry2 = "SELECT * FROM users WHERE username = $1;"
26+
conn.exec_params(qry2, [name])
27+
conn.async_exec_params(qry2, [name])
28+
29+
# .prepare() and .exec_prepared()
30+
# BAD: SQL statement constructed from user input
31+
qry3 = "SELECT * FROM users WHERE username = '#{name}';"
32+
conn.prepare("query_1", qry3)
33+
conn.exec_prepared('query_1')
34+
35+
# .prepare() and .exec_prepared()
36+
# GOOD: SQL statement constructed from sanitized user input
37+
qry3 = "SELECT * FROM users WHERE username = $1;"
38+
conn.prepare("query_2", qry3)
39+
conn.exec_prepared('query_2', [name])
40+
41+
# .prepare() and .exec_prepared()
42+
# NOT EXECUTED: SQL statement constructed from user input but not executed
43+
qry3 = "SELECT * FROM users WHERE username = '#{name}';"
44+
conn.prepare("query_3", qry3)
45+
end
46+
end
47+
48+
class BarController < ApplicationController
49+
def safe_paths
50+
name1 = params["name1"]
51+
# GOOD: barrier guard prevents taint flow
52+
if name == "admin"
53+
qry_bar1 = "SELECT * FROM users WHERE username = '%s';" % name
54+
else
55+
qry_bar1 = "SELECT * FROM users WHERE username = 'none';"
56+
end
57+
conn.exec_params(qry_bar1)
58+
59+
60+
name2 = params["name2"]
61+
# GOOD: barrier guard prevents taint flow
62+
name2 = if ["admin", "guest"].include? name2
63+
name2
64+
else
65+
name2 = "none"
66+
end
67+
qry_bar2 = "SELECT * FROM users WHERE username = '%s';" % name
68+
conn.exec_params(qry_bar2)
69+
end
70+
end

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,18 @@ edges
5555
| ArelInjection.rb:4:5:4:8 | name | ArelInjection.rb:6:20:6:61 | "SELECT * FROM users WHERE nam..." |
5656
| ArelInjection.rb:4:12:4:17 | call to params | ArelInjection.rb:4:12:4:29 | ...[...] |
5757
| ArelInjection.rb:4:12:4:29 | ...[...] | ArelInjection.rb:4:5:4:8 | name |
58+
| PgInjection.rb:6:5:6:8 | name | PgInjection.rb:13:5:13:8 | qry1 |
59+
| PgInjection.rb:6:5:6:8 | name | PgInjection.rb:19:5:19:8 | qry2 |
60+
| PgInjection.rb:6:5:6:8 | name | PgInjection.rb:31:5:31:8 | qry3 |
61+
| PgInjection.rb:6:5:6:8 | name | PgInjection.rb:43:5:43:8 | qry3 |
62+
| PgInjection.rb:6:12:6:17 | call to params | PgInjection.rb:6:12:6:24 | ...[...] |
63+
| PgInjection.rb:6:12:6:24 | ...[...] | PgInjection.rb:6:5:6:8 | name |
64+
| PgInjection.rb:13:5:13:8 | qry1 | PgInjection.rb:14:15:14:18 | qry1 |
65+
| PgInjection.rb:13:5:13:8 | qry1 | PgInjection.rb:15:21:15:24 | qry1 |
66+
| PgInjection.rb:19:5:19:8 | qry2 | PgInjection.rb:20:22:20:25 | qry2 |
67+
| PgInjection.rb:19:5:19:8 | qry2 | PgInjection.rb:21:28:21:31 | qry2 |
68+
| PgInjection.rb:31:5:31:8 | qry3 | PgInjection.rb:32:29:32:32 | qry3 |
69+
| PgInjection.rb:43:5:43:8 | qry3 | PgInjection.rb:44:29:44:32 | qry3 |
5870
nodes
5971
| ActiveRecordInjection.rb:8:25:8:28 | name | semmle.label | name |
6072
| ActiveRecordInjection.rb:8:31:8:34 | pass | semmle.label | pass |
@@ -133,6 +145,19 @@ nodes
133145
| ArelInjection.rb:4:12:4:17 | call to params | semmle.label | call to params |
134146
| ArelInjection.rb:4:12:4:29 | ...[...] | semmle.label | ...[...] |
135147
| ArelInjection.rb:6:20:6:61 | "SELECT * FROM users WHERE nam..." | semmle.label | "SELECT * FROM users WHERE nam..." |
148+
| PgInjection.rb:6:5:6:8 | name | semmle.label | name |
149+
| PgInjection.rb:6:12:6:17 | call to params | semmle.label | call to params |
150+
| PgInjection.rb:6:12:6:24 | ...[...] | semmle.label | ...[...] |
151+
| PgInjection.rb:13:5:13:8 | qry1 | semmle.label | qry1 |
152+
| PgInjection.rb:14:15:14:18 | qry1 | semmle.label | qry1 |
153+
| PgInjection.rb:15:21:15:24 | qry1 | semmle.label | qry1 |
154+
| PgInjection.rb:19:5:19:8 | qry2 | semmle.label | qry2 |
155+
| PgInjection.rb:20:22:20:25 | qry2 | semmle.label | qry2 |
156+
| PgInjection.rb:21:28:21:31 | qry2 | semmle.label | qry2 |
157+
| PgInjection.rb:31:5:31:8 | qry3 | semmle.label | qry3 |
158+
| PgInjection.rb:32:29:32:32 | qry3 | semmle.label | qry3 |
159+
| PgInjection.rb:43:5:43:8 | qry3 | semmle.label | qry3 |
160+
| PgInjection.rb:44:29:44:32 | qry3 | semmle.label | qry3 |
136161
subpaths
137162
#select
138163
| 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 |
@@ -159,3 +184,9 @@ subpaths
159184
| ActiveRecordInjection.rb:177:43:177:104 | "SELECT * FROM users WHERE id ..." | ActiveRecordInjection.rb:173:5:173:10 | call to params | ActiveRecordInjection.rb:177:43:177:104 | "SELECT * FROM users WHERE id ..." | This SQL query depends on a $@. | ActiveRecordInjection.rb:173:5:173:10 | call to params | user-provided value |
160185
| ActiveRecordInjection.rb:178:35:178:96 | "SELECT * FROM users WHERE id ..." | ActiveRecordInjection.rb:173:5:173:10 | call to params | ActiveRecordInjection.rb:178:35:178:96 | "SELECT * FROM users WHERE id ..." | This SQL query depends on a $@. | ActiveRecordInjection.rb:173:5:173:10 | call to params | user-provided value |
161186
| 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 |
187+
| PgInjection.rb:14:15:14:18 | qry1 | PgInjection.rb:6:12:6:17 | call to params | PgInjection.rb:14:15:14:18 | qry1 | This SQL query depends on a $@. | PgInjection.rb:6:12:6:17 | call to params | user-provided value |
188+
| PgInjection.rb:15:21:15:24 | qry1 | PgInjection.rb:6:12:6:17 | call to params | PgInjection.rb:15:21:15:24 | qry1 | This SQL query depends on a $@. | PgInjection.rb:6:12:6:17 | call to params | user-provided value |
189+
| PgInjection.rb:20:22:20:25 | qry2 | PgInjection.rb:6:12:6:17 | call to params | PgInjection.rb:20:22:20:25 | qry2 | This SQL query depends on a $@. | PgInjection.rb:6:12:6:17 | call to params | user-provided value |
190+
| PgInjection.rb:21:28:21:31 | qry2 | PgInjection.rb:6:12:6:17 | call to params | PgInjection.rb:21:28:21:31 | qry2 | This SQL query depends on a $@. | PgInjection.rb:6:12:6:17 | call to params | user-provided value |
191+
| PgInjection.rb:32:29:32:32 | qry3 | PgInjection.rb:6:12:6:17 | call to params | PgInjection.rb:32:29:32:32 | qry3 | This SQL query depends on a $@. | PgInjection.rb:6:12:6:17 | call to params | user-provided value |
192+
| PgInjection.rb:44:29:44:32 | qry3 | PgInjection.rb:6:12:6:17 | call to params | PgInjection.rb:44:29:44:32 | qry3 | This SQL query depends on a $@. | PgInjection.rb:6:12:6:17 | call to params | user-provided value |

0 commit comments

Comments
 (0)