diff --git a/javascript/ql/lib/javascript.qll b/javascript/ql/lib/javascript.qll index 07fb759bd65..3b5f21d2531 100644 --- a/javascript/ql/lib/javascript.qll +++ b/javascript/ql/lib/javascript.qll @@ -126,6 +126,7 @@ import semmle.javascript.frameworks.ShellJS import semmle.javascript.frameworks.Snapdragon import semmle.javascript.frameworks.SystemCommandExecutors import semmle.javascript.frameworks.SQL +import semmle.javascript.frameworks.TypeORM import semmle.javascript.frameworks.SocketIO import semmle.javascript.frameworks.StringFormatters import semmle.javascript.frameworks.TorrentLibraries diff --git a/javascript/ql/lib/semmle/javascript/frameworks/TypeORM.qll b/javascript/ql/lib/semmle/javascript/frameworks/TypeORM.qll index 6adc5d8a081..acb39853b2c 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/TypeORM.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/TypeORM.qll @@ -1,20 +1,31 @@ import javascript +/** + * Provides SQL injection Sinks for [TypeORM](https://www.npmjs.com/package/typeorm) package + */ module TypeOrm { - // Gets an expression that constructs or returns a TypeORM database instance. + /** + * Gets an expression that constructs or returns a TypeORM database instance. + */ API::Node dataSource() { result = API::moduleImport("typeorm").getMember("DataSource").getInstance() } - // Gets an `QueryRunner` + /** + * Gets an `QueryRunner` + */ API::Node queryRunner() { result = dataSource().getMember("createQueryRunner").getReturn() } - // Gets `createQueryBuilder` return value from a Active record based Entity + /** + * Gets `createQueryBuilder` return value from a Active record based Entity + */ API::Node activeRecordQueryBuilder() { result = queryRunner().getMember("manager").getMember("createQueryBuilder").getReceiver() } - // Gets `createQueryBuilder` return value from a Data Mapper based Entity + /** + * Gets `createQueryBuilder` return value from a Data Mapper based Entity + */ API::Node dataMapperQueryBuilder() { result = [ @@ -27,13 +38,17 @@ module TypeOrm { ].getMember("createQueryBuilder").getReturn() } - // Gets return value of a `createQueryBuilder` + /** + * Gets return value of a `createQueryBuilder` + */ API::Node queryBuilderInstance() { result = dataMapperQueryBuilder() or result = activeRecordQueryBuilder() } - // Gets The Brackets that are SQL Subqueries equivalent + /** + * Gets The Brackets that are SQL Subqueries equivalent + */ API::Node brackets() { result = API::moduleImport("typeorm") @@ -42,7 +57,9 @@ module TypeOrm { .getParameter(0) } - // Gets any Successor node of Brackets, NotBrackets + /** + * Gets any Successor node of Brackets, NotBrackets + */ API::Node getASuccessorOfBrackets() { result = brackets() or result = getASuccessorOfBrackets().getAMember() or @@ -52,7 +69,9 @@ module TypeOrm { result = getASuccessorOfBrackets().getInstance() } - // Gets any Successor node of createQueryBuilder + /** + * Gets any Successor node of createQueryBuilder + */ API::Node getASuccessorOfBuilderInstance() { result = queryBuilderInstance() or result = getASuccessorOfBuilderInstance().getAMember() or @@ -80,7 +99,9 @@ module TypeOrm { ] } - // Gets functions that return results + /** + * Gets functions that return results + */ string queryBuilderResult() { result = ["getOne", "getOneOrFail", "getMany", "getRawOne", "getRawMany", "stream"] } diff --git a/javascript/ql/test/library-tests/frameworks/SQL/SqlString.expected b/javascript/ql/test/library-tests/frameworks/SQL/SqlString.expected index a7b01e6dfcf..1d76bd0691a 100644 --- a/javascript/ql/test/library-tests/frameworks/SQL/SqlString.expected +++ b/javascript/ql/test/library-tests/frameworks/SQL/SqlString.expected @@ -70,8 +70,12 @@ | spanner.js:26:12:26:38 | 'UPDATE ... = @baz' | | spanner.js:31:18:31:24 | queries | | spannerImport.js:4:8:4:17 | "SQL code" | +| sqlite3.js:7:8:7:45 | "UPDATE ... id = ?" | +| sqlite3.js:8:8:8:45 | "UPDATE ... id = ?" | | sqlite-types.ts:4:12:4:49 | "UPDATE ... id = ?" | -| sqlite.js:7:8:7:45 | "UPDATE ... id = ?" | -| sqlite.js:8:8:8:45 | "UPDATE ... id = ?" | +| sqlite.js:9:10:9:65 | 'SELECT ... id = 1" | +| sqlite.js:12:10:12:65 | 'SELECT ... id = 1" | +| sqlite.js:15:10:15:74 | 'INSERT ... ',100)' | +| sqlite.js:18:14:19:18 | 'SELECT ... id = 1" | +| sqlite.js:25:19:25:74 | 'SELECT ... id = 1" | | sqliteArray.js:6:12:6:49 | "UPDATE ... id = ?" | -| sqliteImport.js:2:8:2:44 | "UPDATE ... id = ?" | diff --git a/javascript/ql/test/library-tests/frameworks/SQL/sqlite.js b/javascript/ql/test/library-tests/frameworks/SQL/sqlite.js index da03517c839..d486545d691 100644 --- a/javascript/ql/test/library-tests/frameworks/SQL/sqlite.js +++ b/javascript/ql/test/library-tests/frameworks/SQL/sqlite.js @@ -1,10 +1,34 @@ -// Adapted from https://github.com/mapbox/node-sqlite3/wiki/API, which is -// part of the node-sqlite3 project, which is licensed under the BSD 3-Clause -// License; see file node-sqlite3-LICENSE. -var sqlite = require('sqlite3'); +import sqlite3 from 'sqlite3' +import { open } from 'sqlite' + +const unsafe = "unsafe" +open({ + filename: 'database.sqlite', + driver: sqlite3.Database +}).then(async (db) => { + db.get('SELECT name,id FROM table1 WHERE id > 5' + " OR id = 1").then(results => { + console.log(results) + }) + db.all('SELECT name,id FROM table1 WHERE id > 5' + " OR id = 1").then(results => { + console.log(results) + }) + db.run('INSERT INTO table1 (name,id) VALUES (' + `"${unsafe}"` + ',100)').then(results => { + console.log(results) + }) + db.prepare('SELECT name,id FROM table1 WHERE id > 5' + + " OR id = 1").then(results => { + results.all().then(result => { + console.log(result) + }) + }) + try { + await db.each('SELECT name,id FROM table1 WHERE id > 5' + " OR id = 1", (err, row) => { + console.log(row) + }) + + } catch (e) { + throw e + } +}) -var db = new sqlite.Database(":memory:"); -db.run("UPDATE tbl SET name = ? WHERE id = ?", "bar", 2) - .run("UPDATE tbl SET name = ? WHERE id = ?", "foo", 3); -exports.db = db; diff --git a/javascript/ql/test/library-tests/frameworks/SQL/sqlite3.js b/javascript/ql/test/library-tests/frameworks/SQL/sqlite3.js new file mode 100644 index 00000000000..da03517c839 --- /dev/null +++ b/javascript/ql/test/library-tests/frameworks/SQL/sqlite3.js @@ -0,0 +1,10 @@ +// Adapted from https://github.com/mapbox/node-sqlite3/wiki/API, which is +// part of the node-sqlite3 project, which is licensed under the BSD 3-Clause +// License; see file node-sqlite3-LICENSE. +var sqlite = require('sqlite3'); + +var db = new sqlite.Database(":memory:"); +db.run("UPDATE tbl SET name = ? WHERE id = ?", "bar", 2) + .run("UPDATE tbl SET name = ? WHERE id = ?", "foo", 3); + +exports.db = db; diff --git a/javascript/ql/test/library-tests/frameworks/TypeOrm/SqlString.expected b/javascript/ql/test/library-tests/frameworks/TypeOrm/SqlString.expected new file mode 100644 index 00000000000..3d3f5ebbce9 --- /dev/null +++ b/javascript/ql/test/library-tests/frameworks/TypeOrm/SqlString.expected @@ -0,0 +1,43 @@ +| 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" | diff --git a/javascript/ql/test/library-tests/frameworks/TypeOrm/SqlString.ql b/javascript/ql/test/library-tests/frameworks/TypeOrm/SqlString.ql new file mode 100644 index 00000000000..26ab0868c50 --- /dev/null +++ b/javascript/ql/test/library-tests/frameworks/TypeOrm/SqlString.ql @@ -0,0 +1,3 @@ +import javascript + +select any(SQL::SqlString s) diff --git a/javascript/ql/test/library-tests/frameworks/TypeOrm/test.ts b/javascript/ql/test/library-tests/frameworks/TypeOrm/test.ts new file mode 100644 index 00000000000..6c10a3663c8 --- /dev/null +++ b/javascript/ql/test/library-tests/frameworks/TypeOrm/test.ts @@ -0,0 +1,225 @@ +import { BaseEntity, Brackets, DataSource, JoinColumn, NotBrackets, OneToOne } from "typeorm"; +import { Entity, PrimaryGeneratedColumn, Column } from "typeorm" + +@Entity() +export class UserActiveRecord extends BaseEntity { + @PrimaryGeneratedColumn() + id: number + @Column() + firstName: string + @Column() + lastName: string + @Column() + age: number + + static findByName(firstName: string, lastName: string) { + return this.createQueryBuilder("user") + .where("user.firstName = :firstName", { firstName }) + .andWhere("user.lastName = :lastName", { lastName }) + .getMany() + } +} + +@Entity() +export class Profile { + @PrimaryGeneratedColumn() + id: number + @Column() + gender: string + @Column() + photo: string +} + +@Entity() +export class User { + @PrimaryGeneratedColumn() + id: number + @Column() + name: string + @OneToOne(() => Profile) + @JoinColumn() + profile: Profile +} + +@Entity() +export class User2 { + @PrimaryGeneratedColumn() + id: number + @Column() + firstName: string + @Column() + lastName: string + @Column() + age: number + +} + +export const AppDataSource = new DataSource({ + type: "sqlite", + database: "database.sqlite", + synchronize: true, + logging: false, + entities: [User, User2, Profile, UserActiveRecord], + 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 + .createQueryBuilder(User2, "User") + .insert() + .into(User2) + .values({ + firstName: "Timber", + // NOT OK + lastName: () => "Vulnerable \")--", + age: 33, + }) + .orUpdate( + // NOT OK + ["firstName inj3--\" ", "lastName inj1--\" "], + ["externalId inj2\")-- "], + ) + .getQueryAndParameters() + console.log(insertQuery) + + let updateResult = await AppDataSource.getRepository(User2).createQueryBuilder("user2") + .update(User2) + .set({ firstName: () => "Vulnerable \")--", lastName: "Saw2", age: 12 }) + .where("user2.lastName = \"Saw2\" OR 1=1",) + .execute() + console.log(updateResult) + + let deleteResult = await AppDataSource.getRepository(User2).createQueryBuilder('user2') + .delete() + .from(User2) + .where("id = 1 OR 1=1") + .execute() + console.log(deleteResult) + + + const queryRunner = AppDataSource.createQueryRunner() + let resultQueryRunner = await queryRunner.query("select * from USER2") + console.log(resultQueryRunner) + + resultQueryRunner = 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) + await AppDataSource + .createQueryBuilder(User, "User") + .innerJoin("User.profile", "profile", "User.id = :id OR 1=1", { + id: 2, + }).getMany().then(res => console.log(res)) + + await AppDataSource + .createQueryBuilder(User, "User") + .leftJoinAndMapOne("User.profile", "profile", "profile", "User.id = :id OR 1=1", { + id: 2, + }).getMany().then(res => console.log(res)) + + + await AppDataSource + .createQueryBuilder(User2, "User2") + .where((qb) => { + const subQuery = qb + .subQuery() + .select("User2.firstName") + .from(User2, "user2") + .where("user2.id = :registered") + .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) + + // Using DataSource + // NOT OK + users = await AppDataSource + .createQueryBuilder() + .select("User2") + .from(User2, "User2") + .where("User2.id = :id", { 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 + .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) + + // 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", { + 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" }); + }) + ) + .orderBy("User2.id").orWhere("id=1").getMany() + console.log("Loaded users: ", count) + + // relation + AppDataSource.createQueryBuilder().relation(User, "name") + .of(User) + .select().where("1=1").getMany().then(results => { + console.log(results) + }) + + // Brackets + let results2 = await AppDataSource.createQueryBuilder(User2, "User2") + .where("User2.id =:kind", { kind: 1 }) + .andWhere( + new Brackets((qb) => { + qb.where("User2.firstName = :firstName", { + firstName: "Timber", + }).orWhere("User2.lastName = :lastName) OR (1=1) OR (1=1", { lastName: "Saw" }); + }) + ).andWhere( + new NotBrackets((qb) => { + qb.where("User2.firstName = :firstName", { + firstName: "Timber", + }).orWhere("User2.lastName = :lastName", { lastName: "Saw" }) + }), + ).getMany() + console.log("Brackets results:", results2) +}).catch(error => console.log(error)) diff --git a/javascript/ql/test/library-tests/frameworks/TypeOrm/tsconfig.json b/javascript/ql/test/library-tests/frameworks/TypeOrm/tsconfig.json new file mode 100644 index 00000000000..dd33062c275 --- /dev/null +++ b/javascript/ql/test/library-tests/frameworks/TypeOrm/tsconfig.json @@ -0,0 +1,15 @@ +{ + "compilerOptions": { + "lib": [ + "es5", + "es6" + ], + "target": "es6", + "module": "commonjs", + "moduleResolution": "node", + "outDir": "./build", + "emitDecoratorMetadata": true, + "experimentalDecorators": true, + "sourceMap": true + } +} \ No newline at end of file