Merge pull request #13381 from erik-krogh/mongooseFindByIdAndUpdate

JS: remove the second argument of findByIdAndUpdate as a NoSQL sink

Author: erik-krogh

javascript/ql/lib/semmle/javascript/security/TaintedObject.qll

@@ -120,6 +120,22 @@ module TaintedObject {
     override predicate sanitizes(boolean outcome, Expr e) { e = x and outcome = polarity }
   }
 
+  /** A guard that checks whether an input a valid string identifier using `mongoose.Types.ObjectId.isValid` */
+  class ObjectIdGuard extends SanitizerGuard instanceof API::CallNode {
+    ObjectIdGuard() {
+      this =
+        API::moduleImport("mongoose")
+            .getMember("Types")
+            .getMember("ObjectId")
+            .getMember("isValid")
+            .getACall()
+    }
+
+    override predicate sanitizes(boolean outcome, Expr e, FlowLabel lbl) {
+      e = super.getAnArgument().asExpr() and outcome = true and lbl = label()
+    }
+  }
+
   /**
    * A sanitizer guard that validates an input against a JSON schema.
    */

javascript/ql/test/query-tests/Security/CWE-089/untyped/DatabaseAccesses.expected

@@ -27,7 +27,7 @@
 | mongoose.js:63:2:63:34 | Documen ... then(X) |
 | mongoose.js:65:2:65:51 | Documen ... on(){}) |
 | mongoose.js:67:2:68:27 | new Mon ... on(){}) |
-| mongoose.js:71:5:78:9 | Documen ... .exec() |
+| mongoose.js:71:2:78:9 | Documen ... .exec() |
 | mongoose.js:85:2:85:52 | Documen ... query)) |
 | mongoose.js:86:2:86:52 | Documen ... query)) |
 | mongoose.js:87:2:87:57 | Documen ... query)) |
@@ -42,6 +42,8 @@
 | mongoose.js:97:2:97:52 | Documen ... query)) |
 | mongoose.js:99:2:99:50 | Documen ... query)) |
 | mongoose.js:113:2:113:53 | Documen ... () { }) |
+| mongoose.js:134:3:134:52 | Documen ... on(){}) |
+| mongoose.js:136:3:136:52 | Documen ... on(){}) |
 | mysql.js:8:9:11:47 | connect ... ds) {}) |
 | mysql.js:14:9:16:47 | connect ... ds) {}) |
 | mysql.js:19:9:20:48 | connect ... ds) {}) |