Skip to content

Commit 3247bab

Browse files
authored
Merge pull request #19762 from trailofbits/VF/type-orm-model-improvements
Improve TypeORM model
2 parents 6ae1656 + 8a75165 commit 3247bab

File tree

3 files changed

+38
-1
lines changed

3 files changed

+38
-1
lines changed

javascript/ql/src/experimental/semmle/javascript/SQL.qll

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,11 +146,42 @@ module ExperimentalSql {
146146
override DataFlow::Node getAQueryArgument() { result = this.getArgument(0) }
147147
}
148148

149+
/**
150+
* A call to a TypeORM `Repository` (https://orkhan.gitbook.io/typeorm/docs/repository-api)
151+
*/
152+
private class RepositoryCall extends DatabaseAccess {
153+
API::Node repository;
154+
155+
RepositoryCall() {
156+
(
157+
repository = API::moduleImport("typeorm").getMember("Repository").getInstance() or
158+
repository = dataSource().getMember("getRepository").getReturn()
159+
) and
160+
this = repository.getMember(_).asSource()
161+
}
162+
163+
override DataFlow::Node getAResult() {
164+
result =
165+
repository
166+
.getMember([
167+
"find", "findBy", "findOne", "findOneBy", "findOneOrFail", "findOneByOrFail",
168+
"findAndCount", "findAndCountBy"
169+
])
170+
.getReturn()
171+
.asSource()
172+
}
173+
174+
override DataFlow::Node getAQueryArgument() {
175+
result = repository.getMember("query").getParameter(0).asSink()
176+
}
177+
}
178+
149179
/** An expression that is passed to the `query` function and hence interpreted as SQL. */
150180
class QueryString extends SQL::SqlString {
151181
QueryString() {
152182
this = any(QueryRunner qr).getAQueryArgument() or
153-
this = any(QueryBuilderCall qb).getAQueryArgument()
183+
this = any(QueryBuilderCall qb).getAQueryArgument() or
184+
this = any(RepositoryCall rc).getAQueryArgument()
154185
}
155186
}
156187
}

javascript/ql/test/experimental/TypeOrm/test.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,4 +217,9 @@ AppDataSource.initialize().then(async () => {
217217
qb.where(BadInput).orWhere(BadInput) // test: SQLInjectionPoint
218218
}),
219219
).getMany()
220+
221+
// Repository.query sink
222+
await AppDataSource.getRepository(User2)
223+
.query(BadInput) // test: SQLInjectionPoint
224+
220225
}).catch(error => console.log(error))

javascript/ql/test/experimental/TypeOrm/tests.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,4 +29,5 @@ passingPositiveTests
2929
| PASSED | SQLInjectionPoint | test.ts:210:28:210:53 | // test ... onPoint |
3030
| PASSED | SQLInjectionPoint | test.ts:213:56:213:81 | // test ... onPoint |
3131
| PASSED | SQLInjectionPoint | test.ts:217:56:217:81 | // test ... onPoint |
32+
| PASSED | SQLInjectionPoint | test.ts:223:29:223:54 | // test ... onPoint |
3233
failingPositiveTests

0 commit comments

Comments
 (0)