From efbcec09ef2d3dc14bf6367821300b03fd8cee44 Mon Sep 17 00:00:00 2001
From: Max Schaefer <max@semmle.com>
Date: Mon, 23 Mar 2020 17:10:26 +0000
Subject: [PATCH] JavaScript: Add type tracking to Postgres model.

---
 change-notes/1.24/analysis-javascript.md        |  1 +
 .../ql/src/semmle/javascript/frameworks/SQL.qll | 17 ++++++++++++-----
 .../frameworks/SQL/SqlString.expected           |  1 +
 .../library-tests/frameworks/SQL/postgres5.js   |  9 +++++++++
 4 files changed, 23 insertions(+), 5 deletions(-)
 create mode 100644 javascript/ql/test/library-tests/frameworks/SQL/postgres5.js

diff --git a/change-notes/1.24/analysis-javascript.md b/change-notes/1.24/analysis-javascript.md
index 100263eca361..754288fe7cda 100644
--- a/change-notes/1.24/analysis-javascript.md
+++ b/change-notes/1.24/analysis-javascript.md
@@ -42,6 +42,7 @@
   - [ncp](https://www.npmjs.com/package/ncp)
   - [node-dir](https://www.npmjs.com/package/node-dir)
   - [path-exists](https://www.npmjs.com/package/path-exists)
+  - [pg](https://www.npmjs.com/package/pg)
   - [react](https://www.npmjs.com/package/react)
   - [recursive-readdir](https://www.npmjs.com/package/recursive-readdir)
   - [request](https://www.npmjs.com/package/request)
diff --git a/javascript/ql/src/semmle/javascript/frameworks/SQL.qll b/javascript/ql/src/semmle/javascript/frameworks/SQL.qll
index e598352cacd9..1844e5658c87 100644
--- a/javascript/ql/src/semmle/javascript/frameworks/SQL.qll
+++ b/javascript/ql/src/semmle/javascript/frameworks/SQL.qll
@@ -132,15 +132,22 @@ private module Postgres {
     result = DataFlow::moduleImport("pg-pool").getAnInstantiation()
   }
 
+  private DataFlow::SourceNode clientOrPool(DataFlow::TypeTracker t) {
+    t.start() and
+    (result = client() or result = newPool())
+    or
+    exists(DataFlow::TypeTracker t2 | result = clientOrPool(t2).track(t2, t))
+  }
+
+  private DataFlow::SourceNode clientOrPool() {
+    result = clientOrPool(DataFlow::TypeTracker::end())
+  }
+
   /** A call to the Postgres `query` method. */
   private class QueryCall extends DatabaseAccess, DataFlow::ValueNode {
     override MethodCallExpr astNode;
 
-    QueryCall() {
-      exists(DataFlow::SourceNode recv | recv = client() or recv = newPool() |
-        this = recv.getAMethodCall("query")
-      )
-    }
+    QueryCall() { this = clientOrPool().getAMethodCall("query") }
 
     override DataFlow::Node getAQueryArgument() {
       result = DataFlow::valueNode(astNode.getArgument(0))
diff --git a/javascript/ql/test/library-tests/frameworks/SQL/SqlString.expected b/javascript/ql/test/library-tests/frameworks/SQL/SqlString.expected
index ace4b4fa5488..c9f43c1d4873 100644
--- a/javascript/ql/test/library-tests/frameworks/SQL/SqlString.expected
+++ b/javascript/ql/test/library-tests/frameworks/SQL/SqlString.expected
@@ -12,6 +12,7 @@
 | postgres1.js:37:21:37:24 | text |
 | postgres2.js:30:16:30:41 | 'SELECT ... number' |
 | postgres3.js:15:16:15:40 | 'SELECT ... s name' |
+| postgres5.js:8:21:8:25 | query |
 | sequelize2.js:10:17:10:118 | 'SELECT ... Y name' |
 | sequelize.js:8:17:8:118 | 'SELECT ... Y name' |
 | spanner2.js:5:26:5:35 | "SQL code" |
diff --git a/javascript/ql/test/library-tests/frameworks/SQL/postgres5.js b/javascript/ql/test/library-tests/frameworks/SQL/postgres5.js
new file mode 100644
index 000000000000..fb660fd870dd
--- /dev/null
+++ b/javascript/ql/test/library-tests/frameworks/SQL/postgres5.js
@@ -0,0 +1,9 @@
+const pg = require('pg');
+
+function PgWrapper() {
+    this.pool = new pg.Pool({});
+}
+
+PgWrapper.prototype.query = function (query, params, cb) {
+    this.pool.query(query, params || [], cb);
+};