Skip to content

Commit 22bdcad

Browse files
authored
Merge pull request #14302 from amammad/amammad-js-SQLI
JS: extend DatabaseAccess by `TypeORM` and `sqlite` and `better-sqlite3` packages
2 parents cca05e0 + abb8d65 commit 22bdcad

File tree

13 files changed

+621
-14
lines changed

13 files changed

+621
-14
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+
* Added models for the `sqlite` and `better-sqlite3` npm packages.

javascript/ql/lib/semmle/javascript/frameworks/SQL.qll

Lines changed: 69 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ private module Postgres {
104104
API::Node clientOrPool() { result = API::Node::ofType("pg", ["Client", "PoolClient", "Pool"]) }
105105

106106
/** A call to the Postgres `query` method. */
107-
private class QueryCall extends DatabaseAccess, DataFlow::MethodCallNode {
107+
private class QueryCall extends DatabaseAccess, API::CallNode {
108108
QueryCall() { this = clientOrPool().getMember(["execute", "query"]).getACall() }
109109

110110
override DataFlow::Node getAResult() {
@@ -117,15 +117,25 @@ private module Postgres {
117117
PromiseFlow::loadStep(this.getALocalUse(), result, Promises::valueProp())
118118
}
119119

120-
override DataFlow::Node getAQueryArgument() { result = this.getArgument(0) }
120+
override DataFlow::Node getAQueryArgument() {
121+
result = this.getArgument(0) or result = this.getParameter(0).getMember("text").asSink()
122+
}
121123
}
122124

125+
/**
126+
* Gets the Postgres Query class.
127+
* This class can be used to create reusable query objects (see https://node-postgres.com/apis/client).
128+
*/
129+
API::Node query() { result = API::moduleImport("pg").getMember("Query") }
130+
123131
/** An expression that is passed to the `query` method and hence interpreted as SQL. */
124132
class QueryString extends SQL::SqlString {
125133
QueryString() {
126134
this = any(QueryCall qc).getAQueryArgument()
127135
or
128136
this = API::moduleImport("pg-cursor").getParameter(0).asSink()
137+
or
138+
this = query().getParameter(0).asSink()
129139
}
130140
}
131141

@@ -243,7 +253,7 @@ private module Postgres {
243253
/**
244254
* Provides classes modeling the `sqlite3` package.
245255
*/
246-
private module Sqlite {
256+
private module Sqlite3 {
247257
/** Gets an expression that constructs or returns a Sqlite database instance. */
248258
API::Node database() { result = API::Node::ofType("sqlite3", "Database") }
249259

@@ -267,6 +277,62 @@ private module Sqlite {
267277
}
268278
}
269279

280+
/**
281+
* Provides classes modeling the `sqlite` package.
282+
*/
283+
private module Sqlite {
284+
/** Gets an expression that constructs or returns a Sqlite database instance. */
285+
API::Node database() {
286+
result = API::moduleImport("sqlite").getMember("open").getReturn().getPromised()
287+
}
288+
289+
/** A call to a Sqlite query method. */
290+
private class QueryCall extends DatabaseAccess, API::CallNode {
291+
QueryCall() {
292+
this = database().getMember(["all", "each", "exec", "get", "prepare", "run"]).getACall()
293+
}
294+
295+
override DataFlow::Node getAResult() { result = this }
296+
297+
override DataFlow::Node getAQueryArgument() { result = this.getArgument(0) }
298+
}
299+
300+
/** An expression that is passed to the `query` method and hence interpreted as SQL. */
301+
class QueryString extends SQL::SqlString {
302+
QueryString() { this = any(QueryCall qc).getAQueryArgument() }
303+
}
304+
}
305+
306+
/**
307+
* Provides classes modeling the `better-sqlite3` package.
308+
*/
309+
private module BetterSqlite3 {
310+
/**
311+
* Gets a `better-sqlite3` database instance.
312+
*/
313+
API::Node database() {
314+
result =
315+
[
316+
API::moduleImport("better-sqlite3").getInstance(),
317+
API::moduleImport("better-sqlite3").getReturn()
318+
]
319+
or
320+
result = database().getMember("exec").getReturn()
321+
}
322+
323+
/** A call to a better-sqlite3 query method. */
324+
private class QueryCall extends DatabaseAccess, API::CallNode {
325+
QueryCall() { this = database().getMember(["exec", "prepare"]).getACall() }
326+
327+
override DataFlow::Node getAQueryArgument() { result = this.getArgument(0) }
328+
}
329+
330+
/** An expression that is passed to the `query` method and hence interpreted as SQL. */
331+
class QueryString extends SQL::SqlString {
332+
QueryString() { this = any(QueryCall qc).getAQueryArgument() }
333+
}
334+
}
335+
270336
/**
271337
* Provides classes modeling the `mssql` package.
272338
*/
Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
/**
2+
* Provides classes for working with SQL connectors.
3+
*/
4+
5+
import javascript
6+
7+
module ExperimentalSql {
8+
/**
9+
* Provides SQL injection Sinks for the [TypeORM](https://www.npmjs.com/package/typeorm) package
10+
*/
11+
private module TypeOrm {
12+
/**
13+
* Gets a `DataSource` instance
14+
*
15+
* `DataSource` is a pre-defined connection configuration to a specific database.
16+
*/
17+
API::Node dataSource() {
18+
result = API::moduleImport("typeorm").getMember("DataSource").getInstance()
19+
}
20+
21+
/**
22+
* Gets a `QueryRunner` instance
23+
*/
24+
API::Node queryRunner() { result = dataSource().getMember("createQueryRunner").getReturn() }
25+
26+
/**
27+
* Gets a `*QueryBuilder` instance
28+
*/
29+
API::Node queryBuilderInstance() {
30+
// a `*QueryBuilder` instance of a Data Mapper based Entity
31+
result =
32+
[
33+
// Using DataSource
34+
dataSource(),
35+
// Using repository
36+
dataSource().getMember("getRepository").getReturn(),
37+
// Using entity manager
38+
dataSource().getMember("manager"), queryRunner().getMember("manager")
39+
].getMember("createQueryBuilder").getReturn()
40+
or
41+
// A `*QueryBuilder` instance of an Active record based Entity
42+
result =
43+
API::moduleImport("typeorm")
44+
.getMember("Entity")
45+
.getReturn()
46+
.getADecoratedClass()
47+
.getMember("createQueryBuilder")
48+
.getReturn()
49+
or
50+
// A WhereExpressionBuilder can be used in complex WHERE expression
51+
result =
52+
API::moduleImport("typeorm")
53+
.getMember(["Brackets", "NotBrackets"])
54+
.getParameter(0)
55+
.getParameter(0)
56+
or
57+
// In case of custom query builders
58+
result =
59+
API::moduleImport("typeorm")
60+
.getMember([
61+
"SelectQueryBuilder", "InsertQueryBuilder", "RelationQueryBuilder",
62+
"UpdateQueryBuilder", "WhereExpressionBuilder"
63+
])
64+
.getInstance()
65+
}
66+
67+
/**
68+
* Gets function names which create any type of `QueryBuilder` like `WhereExpressionBuilder` or `InsertQueryBuilder`
69+
*/
70+
string queryBuilderMethods() {
71+
result =
72+
[
73+
"select", "addSelect", "where", "andWhere", "orWhere", "having", "orHaving", "andHaving",
74+
"orderBy", "addOrderBy", "distinctOn", "groupBy", "addCommonTableExpression",
75+
"leftJoinAndSelect", "innerJoinAndSelect", "leftJoin", "innerJoin", "leftJoinAndMapOne",
76+
"innerJoinAndMapOne", "leftJoinAndMapMany", "innerJoinAndMapMany", "orUpdate", "orIgnore",
77+
"values", "set"
78+
]
79+
}
80+
81+
/**
82+
* Gets function names that the return values of these functions can be the results of a database query run
83+
*/
84+
string queryBuilderResult() {
85+
result = ["getOne", "getOneOrFail", "getMany", "getRawOne", "getRawMany", "stream"]
86+
}
87+
88+
/**
89+
* Gets a QueryBuilder instance that has a query builder function
90+
*/
91+
API::Node getASuccessorOfBuilderInstance(string queryBuilderMethod) {
92+
result.getMember(queryBuilderMethod) = queryBuilderInstance().getASuccessor*()
93+
}
94+
95+
/**
96+
* A call to some successor functions of TypeORM `createQueryBuilder` function which are dangerous
97+
*/
98+
private class QueryBuilderCall extends DatabaseAccess, DataFlow::Node {
99+
API::Node queryBuilder;
100+
101+
QueryBuilderCall() {
102+
queryBuilder = getASuccessorOfBuilderInstance(queryBuilderMethods()) and
103+
this = queryBuilder.asSource()
104+
}
105+
106+
override DataFlow::Node getAResult() {
107+
result = queryBuilder.getMember(queryBuilderResult()).getReturn().asSource()
108+
}
109+
110+
override DataFlow::Node getAQueryArgument() {
111+
exists(string memberName | memberName = queryBuilderMethods() |
112+
memberName = ["leftJoinAndSelect", "innerJoinAndSelect", "leftJoin", "innerJoin"] and
113+
result = queryBuilder.getMember(memberName).getParameter(2).asSink()
114+
or
115+
memberName =
116+
["leftJoinAndMapOne", "innerJoinAndMapOne", "leftJoinAndMapMany", "innerJoinAndMapMany"] and
117+
result = queryBuilder.getMember(memberName).getParameter(3).asSink()
118+
or
119+
memberName =
120+
[
121+
"select", "addSelect", "where", "andWhere", "orWhere", "having", "orHaving",
122+
"andHaving", "orderBy", "addOrderBy", "distinctOn", "groupBy",
123+
"addCommonTableExpression"
124+
] and
125+
result = queryBuilder.getMember(memberName).getParameter(0).asSink()
126+
or
127+
memberName = ["orIgnore", "orUpdate"] and
128+
result = queryBuilder.getMember(memberName).getParameter([0, 1]).asSink()
129+
or
130+
// following functions if use a function as their input fields,called function parameters which are vulnerable
131+
memberName = ["values", "set"] and
132+
result =
133+
queryBuilder.getMember(memberName).getParameter(0).getAMember().getReturn().asSink()
134+
)
135+
}
136+
}
137+
138+
/**
139+
* A call to the TypeORM `query` function of a `QueryRunner`
140+
*/
141+
private class QueryRunner extends DatabaseAccess, API::CallNode {
142+
QueryRunner() { queryRunner().getMember("query").getACall() = this }
143+
144+
override DataFlow::Node getAResult() { result = this }
145+
146+
override DataFlow::Node getAQueryArgument() { result = this.getArgument(0) }
147+
}
148+
149+
/** An expression that is passed to the `query` function and hence interpreted as SQL. */
150+
class QueryString extends SQL::SqlString {
151+
QueryString() {
152+
this = any(QueryRunner qr).getAQueryArgument() or
153+
this = any(QueryBuilderCall qb).getAQueryArgument()
154+
}
155+
}
156+
}
157+
}

0 commit comments

Comments
 (0)