Skip to content

Commit 2257df5

Browse files
Model Arel::Nodes::SqlLiteral.new
1 parent 2ebb80b commit 2257df5

File tree

4 files changed

+44
-0
lines changed

4 files changed

+44
-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+
Calls to `Arel::Nodes::SqlLiteral.new` are now modeled as instances of the `SqlConstruction` concept, as well as propagating taint from their argument.

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

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,4 +39,40 @@ module Arel {
3939

4040
override DataFlow::Node getSql() { result = this.getArgument(0) }
4141
}
42+
43+
/**
44+
* Flow summary for `Arel::Nodes::SqlLiteral.new`. This method wraps a SQL string, marking it as
45+
* safe.
46+
*/
47+
private class SqlLiteralNewSummary extends SummarizedCallable {
48+
SqlLiteralNewSummary() { this = "Arel::Nodes::SqlLiteral.new" }
49+
50+
override MethodCall getACall() {
51+
result =
52+
API::getTopLevelMember("Arel")
53+
.getMember("Nodes")
54+
.getMember("SqlLiteral")
55+
.getAMethodCall("new")
56+
.asExpr()
57+
.getExpr()
58+
}
59+
60+
override predicate propagatesFlow(string input, string output, boolean preservesValue) {
61+
input = "Argument[0]" and output = "ReturnValue" and preservesValue = false
62+
}
63+
}
64+
65+
/** A call to `Arel::Nodes::SqlLiteral.new`, considered as a SQL construction. */
66+
private class ArelSqlLiteralNewConstruction extends SqlConstruction::Range, DataFlow::CallNode {
67+
ArelSqlLiteralNewConstruction() {
68+
this.asExpr() =
69+
API::getTopLevelMember("Arel")
70+
.getMember("Nodes")
71+
.getMember("SqlLiteral")
72+
.getAMethodCall("new")
73+
.asExpr()
74+
}
75+
76+
override DataFlow::Node getSql() { result = this.getArgument(0) }
77+
}
4278
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,5 +4,6 @@ def unsafe_action
44
name = params[:user_name]
55
# BAD: SQL statement constructed from user input
66
sql = Arel.sql("SELECT * FROM users WHERE name = #{name}")
7+
sql = Arel::Nodes::SqlLiteral.new("SELECT * FROM users WHERE name = #{name}")
78
end
89
end

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ edges
7373
| ActiveRecordInjection.rb:198:69:198:84 | call to permitted_params | ActiveRecordInjection.rb:198:69:198:94 | ...[...] | provenance | |
7474
| ActiveRecordInjection.rb:198:69:198:94 | ...[...] | ActiveRecordInjection.rb:198:35:198:96 | "SELECT * FROM users WHERE id ..." | provenance | |
7575
| ArelInjection.rb:4:5:4:8 | name | ArelInjection.rb:6:20:6:61 | "SELECT * FROM users WHERE nam..." | provenance | |
76+
| ArelInjection.rb:4:5:4:8 | name | ArelInjection.rb:7:39:7:80 | "SELECT * FROM users WHERE nam..." | provenance | |
7677
| ArelInjection.rb:4:12:4:17 | call to params | ArelInjection.rb:4:12:4:29 | ...[...] | provenance | |
7778
| ArelInjection.rb:4:12:4:29 | ...[...] | ArelInjection.rb:4:5:4:8 | name | provenance | |
7879
| PgInjection.rb:6:5:6:8 | name | PgInjection.rb:13:5:13:8 | qry1 | provenance | |
@@ -194,6 +195,7 @@ nodes
194195
| ArelInjection.rb:4:12:4:17 | call to params | semmle.label | call to params |
195196
| ArelInjection.rb:4:12:4:29 | ...[...] | semmle.label | ...[...] |
196197
| ArelInjection.rb:6:20:6:61 | "SELECT * FROM users WHERE nam..." | semmle.label | "SELECT * FROM users WHERE nam..." |
198+
| ArelInjection.rb:7:39:7:80 | "SELECT * FROM users WHERE nam..." | semmle.label | "SELECT * FROM users WHERE nam..." |
197199
| PgInjection.rb:6:5:6:8 | name | semmle.label | name |
198200
| PgInjection.rb:6:12:6:17 | call to params | semmle.label | call to params |
199201
| PgInjection.rb:6:12:6:24 | ...[...] | semmle.label | ...[...] |
@@ -244,6 +246,7 @@ subpaths
244246
| ActiveRecordInjection.rb:197:43:197:104 | "SELECT * FROM users WHERE id ..." | ActiveRecordInjection.rb:193:5:193:10 | call to params | ActiveRecordInjection.rb:197:43:197:104 | "SELECT * FROM users WHERE id ..." | This SQL query depends on a $@. | ActiveRecordInjection.rb:193:5:193:10 | call to params | user-provided value |
245247
| ActiveRecordInjection.rb:198:35:198:96 | "SELECT * FROM users WHERE id ..." | ActiveRecordInjection.rb:193:5:193:10 | call to params | ActiveRecordInjection.rb:198:35:198:96 | "SELECT * FROM users WHERE id ..." | This SQL query depends on a $@. | ActiveRecordInjection.rb:193:5:193:10 | call to params | user-provided value |
246248
| 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 |
249+
| ArelInjection.rb:7:39:7:80 | "SELECT * FROM users WHERE nam..." | ArelInjection.rb:4:12:4:17 | call to params | ArelInjection.rb:7:39:7:80 | "SELECT * FROM users WHERE nam..." | This SQL query depends on a $@. | ArelInjection.rb:4:12:4:17 | call to params | user-provided value |
247250
| 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 |
248251
| 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 |
249252
| 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 |

0 commit comments

Comments
 (0)