add TypeORM to javascript.qll file

add tests
improvement on comments

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

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"]
   }

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 = ?" |

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;

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;

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" |

javascript/ql/test/library-tests/frameworks/TypeOrm/SqlString.ql

@@ -0,0 +1,3 @@
+import javascript
+
+select any(SQL::SqlString s)

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))

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
+    }
+}