From e3dbdc3887f59e401555251f6842e2ff3be0c78f Mon Sep 17 00:00:00 2001 From: amammad <77095239+amammad@users.noreply.github.com> Date: Sun, 22 Oct 2023 21:39:49 +0200 Subject: [PATCH] add custom query builder and active record querybuilder support --- .../semmle/javascript/frameworks/TypeORM.qll | 40 ++-- .../frameworks/TypeOrm/SqlString.expected | 83 ++++---- .../library-tests/frameworks/TypeOrm/test.ts | 192 +++++++++--------- 3 files changed, 161 insertions(+), 154 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/frameworks/TypeORM.qll b/javascript/ql/lib/semmle/javascript/frameworks/TypeORM.qll index acb39853b2c..71a8ef38b98 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/TypeORM.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/TypeORM.qll @@ -5,26 +5,31 @@ import javascript */ module TypeOrm { /** - * Gets an expression that constructs or returns a TypeORM database instance. + * Gets a `DataSource` instance */ API::Node dataSource() { result = API::moduleImport("typeorm").getMember("DataSource").getInstance() } /** - * Gets an `QueryRunner` + * Gets a `QueryRunner` nodes */ API::Node queryRunner() { result = dataSource().getMember("createQueryRunner").getReturn() } /** - * Gets `createQueryBuilder` return value from a Active record based Entity + * Gets a `*QueryBuilder` node of an Active record based Entity */ API::Node activeRecordQueryBuilder() { - result = queryRunner().getMember("manager").getMember("createQueryBuilder").getReceiver() + result = + API::moduleImport("typeorm") + .getMember("Entity") + .getReturn() + .getADecoratedClass() + .getMember("createQueryBuilder") } /** - * Gets `createQueryBuilder` return value from a Data Mapper based Entity + * Gets a `*QueryBuilder` node of a Data Mapper based Entity */ API::Node dataMapperQueryBuilder() { result = @@ -36,10 +41,19 @@ module TypeOrm { // Using entity manager dataSource().getMember("manager"), queryRunner().getMember("manager") ].getMember("createQueryBuilder").getReturn() + or + // in case of custom query builders + result = + API::moduleImport("typeorm") + .getMember([ + "SelectQueryBuilder", "InsertQueryBuilder", "RelationQueryBuilder", + "UpdateQueryBuilder" + ]) + .getInstance() } /** - * Gets return value of a `createQueryBuilder` + * Gets a `*QueryBuilder` node */ API::Node queryBuilderInstance() { result = dataMapperQueryBuilder() or @@ -91,8 +105,8 @@ module TypeOrm { string selectExpression() { result = [ - "select", "addSelect", "from", "where", "andWhere", "orWhere", "having", "orHaving", - "andHaving", "orderBy", "addOrderBy", "distinctOn", "groupBy", "addCommonTableExpression", + "select", "addSelect", "where", "andWhere", "orWhere", "having", "orHaving", "andHaving", + "orderBy", "addOrderBy", "distinctOn", "groupBy", "addCommonTableExpression", "leftJoinAndSelect", "innerJoinAndSelect", "leftJoin", "innerJoin", "leftJoinAndMapOne", "innerJoinAndMapOne", "leftJoinAndMapMany", "innerJoinAndMapMany", "orUpdate", "orIgnore", "values", "set" @@ -116,8 +130,8 @@ module TypeOrm { typeOrmNode = getASuccessorOfBuilderInstance() and this = typeOrmNode.asSource() or - // I'm doing following because this = TypeORMNode.asSource()s - // won't let me to get a member in getAQueryArgument + // I'm doing following because `this = TypeORMNode.asSource()` + // don't let me to get a member in getAQueryArgument typeOrmNode = getASuccessorOfBrackets() and typeOrmNode.getMember(selectExpression()).getACall() = this } @@ -137,7 +151,7 @@ module TypeOrm { or memberName = [ - "select", "addSelect", "from", "where", "andWhere", "orWhere", "having", "orHaving", + "select", "addSelect", "where", "andWhere", "orWhere", "having", "orHaving", "andHaving", "orderBy", "addOrderBy", "distinctOn", "groupBy", "addCommonTableExpression" ] and @@ -167,8 +181,8 @@ module TypeOrm { /** An expression that is passed to the `query` function and hence interpreted as SQL. */ class QueryString extends SQL::SqlString { QueryString() { - this = any(QueryRunner qc).getAQueryArgument() or - this = any(QueryBuilderCall qc).getAQueryArgument() + this = any(QueryRunner qr).getAQueryArgument() or + this = any(QueryBuilderCall qb).getAQueryArgument() } } } diff --git a/javascript/ql/test/library-tests/frameworks/TypeOrm/SqlString.expected b/javascript/ql/test/library-tests/frameworks/TypeOrm/SqlString.expected index 3d3f5ebbce9..8cb623dfaa2 100644 --- a/javascript/ql/test/library-tests/frameworks/TypeOrm/SqlString.expected +++ b/javascript/ql/test/library-tests/frameworks/TypeOrm/SqlString.expected @@ -1,43 +1,40 @@ -| test.ts:70:17:70:27 | "user.name" | -| test.ts:71:16:71:46 | "user.r ... stered" | -| test.ts:80:29:80:46 | "Vulnerable \\")--" | -| test.ts:85:13:85:57 | ["first ... --\\" "] | -| test.ts:86:13:86:37 | ["exter ... ")-- "] | -| test.ts:93:33:93:50 | "Vulnerable \\")--" | -| test.ts:94:16:94:49 | "user2. ... OR 1=1" | -| test.ts:100:15:100:19 | User2 | -| test.ts:101:16:101:30 | "id = 1 OR 1=1" | -| test.ts:107:53:107:73 | "select ... USER2" | -| test.ts:112:17:112:19 | "*" | -| test.ts:113:16:113:28 | "user.id >=3" | -| test.ts:121:47:121:68 | "User.i ... OR 1=1" | -| test.ts:127:66:127:87 | "User.i ... OR 1=1" | -| test.ts:134:16:142:9 | (qb) => ... } | -| test.ts:137:25:137:41 | "User2.firstName" | -| test.ts:138:23:138:27 | User2 | -| test.ts:139:24:139:47 | "user2. ... stered" | -| test.ts:150:93:150:109 | "User2.id =:kind" | -| test.ts:152:92:152:120 | "User2. ... OR 1=1" | -| test.ts:159:17:159:23 | "User2" | -| test.ts:160:15:160:19 | User2 | -| test.ts:161:16:161:31 | "User2.id = :id" | -| test.ts:167:51:167:79 | "User2. ... OR 1=1" | -| test.ts:171:53:171:62 | "User2.id" | -| test.ts:171:72:171:101 | "User2. ... stName" | -| test.ts:176:52:176:81 | "photo. ... emoved" | -| test.ts:182:53:182:62 | "User2.id" | -| test.ts:182:72:182:101 | "User2. ... stName" | -| test.ts:188:51:188:80 | "User2. ... stName" | -| test.ts:192:13:196:14 | new Bra ... }) | -| test.ts:193:26:193:55 | "User2. ... stName" | -| test.ts:195:28:195:73 | "User2. ... R (1=1" | -| test.ts:198:18:198:27 | "User2.id" | -| test.ts:198:38:198:43 | "id=1" | -| test.ts:204:25:204:29 | "1=1" | -| test.ts:210:16:210:32 | "User2.id =:kind" | -| test.ts:212:13:216:14 | new Bra ... }) | -| test.ts:213:26:213:55 | "User2. ... stName" | -| test.ts:215:28:215:73 | "User2. ... R (1=1" | -| test.ts:218:13:222:14 | new Not ... }) | -| test.ts:219:26:219:55 | "User2. ... stName" | -| test.ts:221:28:221:55 | "User2. ... stName" | +| test.ts:19:20:19:50 | "user.f ... rstName | +| test.ts:20:23:20:51 | "user.l ... astName | +| test.ts:83:30:83:37 | BadInput | +| test.ts:89:31:89:38 | BadInput | +| test.ts:98:29:98:36 | BadInput | +| test.ts:112:29:112:36 | BadInput | +| test.ts:116:13:116:32 | [BadInput, BadInput] | +| test.ts:117:13:117:22 | [BadInput] | +| test.ts:123:32:123:39 | BadInput | +| test.ts:124:16:124:23 | BadInput | +| test.ts:130:16:130:23 | BadInput | +| test.ts:135:29:135:36 | BadInput | +| test.ts:139:17:139:24 | BadInput | +| test.ts:140:16:140:23 | BadInput | +| test.ts:144:47:144:54 | BadInput | +| test.ts:150:66:150:73 | BadInput | +| test.ts:157:16:165:9 | (qb) => ... } | +| test.ts:160:25:160:32 | BadInput | +| test.ts:162:24:162:31 | BadInput | +| test.ts:171:92:171:119 | "User2. ... adInput | +| test.ts:176:17:176:23 | "User2" | +| test.ts:178:16:178:23 | BadInput | +| test.ts:183:51:183:78 | "User2. ... adInput | +| test.ts:186:52:186:59 | BadInput | +| test.ts:188:53:188:62 | "User2.id" | +| test.ts:188:72:188:79 | BadInput | +| test.ts:192:51:192:58 | BadInput | +| test.ts:196:13:198:14 | new Bra ... }) | +| test.ts:197:26:197:33 | BadInput | +| test.ts:197:44:197:51 | BadInput | +| test.ts:200:18:200:25 | BadInput | +| test.ts:200:36:200:43 | BadInput | +| test.ts:205:25:205:32 | BadInput | +| test.ts:211:16:211:23 | BadInput | +| test.ts:213:13:215:14 | new Bra ... }) | +| test.ts:214:26:214:33 | BadInput | +| test.ts:214:44:214:51 | BadInput | +| test.ts:217:13:219:14 | new Not ... }) | +| test.ts:218:26:218:33 | BadInput | +| test.ts:218:44:218:51 | BadInput | diff --git a/javascript/ql/test/library-tests/frameworks/TypeOrm/test.ts b/javascript/ql/test/library-tests/frameworks/TypeOrm/test.ts index 6c10a3663c8..9be880c7875 100644 --- a/javascript/ql/test/library-tests/frameworks/TypeOrm/test.ts +++ b/javascript/ql/test/library-tests/frameworks/TypeOrm/test.ts @@ -1,5 +1,7 @@ -import { BaseEntity, Brackets, DataSource, JoinColumn, NotBrackets, OneToOne } from "typeorm"; -import { Entity, PrimaryGeneratedColumn, Column } from "typeorm" +import { + BaseEntity, Brackets, DataSource, JoinColumn, NotBrackets + , OneToOne, Entity, PrimaryGeneratedColumn, Column, SelectQueryBuilder, InsertQueryBuilder +} from "typeorm"; @Entity() export class UserActiveRecord extends BaseEntity { @@ -14,8 +16,8 @@ export class UserActiveRecord extends BaseEntity { static findByName(firstName: string, lastName: string) { return this.createQueryBuilder("user") - .where("user.firstName = :firstName", { firstName }) - .andWhere("user.lastName = :lastName", { lastName }) + .where("user.firstName = " + firstName) + .andWhere("user.lastName = " + lastName) .getMany() } } @@ -63,163 +65,157 @@ export const AppDataSource = new DataSource({ migrations: [], subscribers: [], }) -AppDataSource.initialize().then(async () => { - // OK - const userQb = AppDataSource - .createQueryBuilder(User, "User") - .select("user.name") - .where("user.registered = :registered", { registered: true }).getParameters() - let insertQuery = AppDataSource +function makePaginationQuery(q: SelectQueryBuilder): SelectQueryBuilder { + return q; +} + +AppDataSource.initialize().then(async () => { + // NOT OK + const BadInput = "1=1" + + // Active record + await UserActiveRecord.findByName(BadInput, "Saw") + + // data mapper + const selectQueryBuilder = makePaginationQuery(AppDataSource + .createQueryBuilder(User, "User").select()); + selectQueryBuilder.where(BadInput).getMany().then(result => { + console.log(result) + }); + + const selectQueryBuilder2 = makePaginationQuery(AppDataSource + .createQueryBuilder(User, "User")); + selectQueryBuilder2.where(BadInput).getMany().then(result => { + console.log(result) + }); + + const insertQueryBuilder: InsertQueryBuilder = AppDataSource + .createQueryBuilder(User2, "User2").insert(); + insertQueryBuilder.into(User2) + .values({ + firstName: "Timber", + lastName: () => BadInput, + age: 33, + }).execute().then(result => { + console.log(result) + + + }) + + AppDataSource .createQueryBuilder(User2, "User") .insert() .into(User2) .values({ firstName: "Timber", - // NOT OK - lastName: () => "Vulnerable \")--", + lastName: () => BadInput, age: 33, }) .orUpdate( - // NOT OK - ["firstName inj3--\" ", "lastName inj1--\" "], - ["externalId inj2\")-- "], + [BadInput, BadInput], + [BadInput], ) .getQueryAndParameters() - console.log(insertQuery) - - let updateResult = await AppDataSource.getRepository(User2).createQueryBuilder("user2") + + await AppDataSource.getRepository(User2).createQueryBuilder("user2") .update(User2) - .set({ firstName: () => "Vulnerable \")--", lastName: "Saw2", age: 12 }) - .where("user2.lastName = \"Saw2\" OR 1=1",) + .set({firstName: () => BadInput, lastName: "Saw2", age: 12}) + .where(BadInput,) .execute() - console.log(updateResult) - - let deleteResult = await AppDataSource.getRepository(User2).createQueryBuilder('user2') + + await AppDataSource.getRepository(User2).createQueryBuilder('user2') .delete() .from(User2) - .where("id = 1 OR 1=1") + .where(BadInput) .execute() - console.log(deleteResult) - - + + const queryRunner = AppDataSource.createQueryRunner() - let resultQueryRunner = await queryRunner.query("select * from USER2") - console.log(resultQueryRunner) - - resultQueryRunner = await queryRunner.manager + await queryRunner.query(BadInput) + + await queryRunner.manager .createQueryBuilder(User2, "User") - .select("*") - .where("user.id >=3").execute() - console.log(resultQueryRunner) - - // Active record - const timber = await UserActiveRecord.findByName("Timber", "Saw") - console.log(timber) + .select(BadInput) + .where(BadInput).execute() + await AppDataSource .createQueryBuilder(User, "User") - .innerJoin("User.profile", "profile", "User.id = :id OR 1=1", { + .innerJoin("User.profile", "profile", BadInput, { id: 2, }).getMany().then(res => console.log(res)) - + await AppDataSource .createQueryBuilder(User, "User") - .leftJoinAndMapOne("User.profile", "profile", "profile", "User.id = :id OR 1=1", { + .leftJoinAndMapOne("User.profile", "profile", "profile", BadInput, { id: 2, }).getMany().then(res => console.log(res)) - - + + await AppDataSource .createQueryBuilder(User2, "User2") .where((qb) => { const subQuery = qb .subQuery() - .select("User2.firstName") + .select(BadInput) .from(User2, "user2") - .where("user2.id = :registered") + .where(BadInput) .getQuery() return "User2.id IN " + subQuery }) .setParameter("registered", true) .getMany() - - - console.log("Loading users from the database...") + + // Using repository - // OK: using parameters - let users0 = await AppDataSource.getRepository(User2).createQueryBuilder("User2").where("User2.id =:kind", { kind: '1' }).getMany() - // NOT OK - let users = await AppDataSource.getRepository(User2).createQueryBuilder("User2").where("User2.id =:kind" + " OR 1=1", { kind: 1 }).getMany() - console.log("Loaded users: ", users) - + let users = await AppDataSource.getRepository(User2).createQueryBuilder("User2").where("User2.id =:kind" + BadInput, {kind: 1}).getMany() + // Using DataSource - // NOT OK users = await AppDataSource .createQueryBuilder() .select("User2") .from(User2, "User2") - .where("User2.id = :id", { id: 1 }) + .where(BadInput, {id: 1}) .getMany() - console.log("Loaded users: ", users) - + // Using entity manager - users = await AppDataSource.manager - .createQueryBuilder(User2, "User2").where("User2.id =:kind" + " OR 1=1", { kind: '1' }).getMany() - console.log("Loaded users: ", users) - - let count = await AppDataSource - .createQueryBuilder(User2, "User2").groupBy("User2.id").having("User2.firstName = :firstName", { firstName: "Timber" }).getMany() - console.log("Loaded users: ", count) - - count = await AppDataSource + await AppDataSource.manager + .createQueryBuilder(User2, "User2").where("User2.id =:kind" + BadInput, {kind: '1'}).getMany() + await AppDataSource .createQueryBuilder(User2, "User2") - .leftJoinAndSelect("user.photos", "photo", "photo.isRemoved = :isRemoved", { - isRemoved: false, - }).getMany() - - - count = await AppDataSource - .createQueryBuilder(User2, "User2").groupBy("User2.id").having("User2.firstName = :firstName", { firstName: "Timber" }).getMany() - console.log("Loaded users: ", count) - + .leftJoinAndSelect("user.photos", "photo", BadInput).getMany() + await AppDataSource + .createQueryBuilder(User2, "User2").groupBy("User2.id").having(BadInput).getMany() // orderBy // it is a little bit restrictive, e.g. sqlite don't support it at all - count = await AppDataSource - .createQueryBuilder(User2, "User2").where("User2.firstName = :firstName", { + await AppDataSource + .createQueryBuilder(User2, "User2").where(BadInput, { firstName: "Timber", }) .where( new Brackets((qb) => { - qb.where("User2.firstName = :firstName", { - firstName: "Timber", - }).orWhere("User2.lastName = :lastName) OR (1=1) OR (1=1", { lastName: "Saw" }); + qb.where(BadInput).orWhere(BadInput); }) ) - .orderBy("User2.id").orWhere("id=1").getMany() - console.log("Loaded users: ", count) - + .orderBy(BadInput).orWhere(BadInput).getMany() + // relation AppDataSource.createQueryBuilder().relation(User, "name") .of(User) - .select().where("1=1").getMany().then(results => { - console.log(results) - }) - + .select().where(BadInput).getMany().then(results => { + console.log(results) + }) + // Brackets - let results2 = await AppDataSource.createQueryBuilder(User2, "User2") - .where("User2.id =:kind", { kind: 1 }) + await AppDataSource.createQueryBuilder(User2, "User2") + .where(BadInput) .andWhere( new Brackets((qb) => { - qb.where("User2.firstName = :firstName", { - firstName: "Timber", - }).orWhere("User2.lastName = :lastName) OR (1=1) OR (1=1", { lastName: "Saw" }); + qb.where(BadInput).orWhere(BadInput); }) ).andWhere( new NotBrackets((qb) => { - qb.where("User2.firstName = :firstName", { - firstName: "Timber", - }).orWhere("User2.lastName = :lastName", { lastName: "Saw" }) + qb.where(BadInput).orWhere(BadInput) }), ).getMany() - console.log("Brackets results:", results2) }).catch(error => console.log(error))