Skip to content

Commit ad5355a

Browse files
committed
Pg Library, change note and Frameworks.qll
1 parent e33f3a6 commit ad5355a

File tree

4 files changed

+152
-0
lines changed

4 files changed

+152
-0
lines changed
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 an 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
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 statment 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+
}
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

0 commit comments

Comments
 (0)