Merge branch 'main' into extractBigReg

javascript/change-notes/2020-05-17-prototype-assignment.md

@@ -0,0 +1,3 @@
+lgtm,codescanning
+* The `js/prototype-polluting-assignment` query now flags assignments that may modify 
+  the built-in Object prototype where the property name originates from library input.

javascript/change-notes/2021-11-02-insufficient-key-size.md

@@ -0,0 +1,2 @@
+lgtm,codescanning
+* The `js/insufficient-key-size` query has been added. It highlights the creation of cryptographic keys with a short key size.

javascript/change-notes/2021-11-02-session-fixation.md

@@ -0,0 +1,2 @@
+lgtm,codescanning
+* The `js/session-fixation` query has been added. It highlights servers that reuse a session after a user has logged in.

javascript/extractor/src/com/semmle/js/extractor/FileExtractor.java

@@ -5,7 +5,7 @@ import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileReader;
 import java.io.IOException;
-import java.nio.charset.CharacterCodingException;
+import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Path;
 import java.util.LinkedHashSet;
@@ -17,7 +17,6 @@ import com.semmle.js.extractor.trapcache.CachingTrapWriter;
 import com.semmle.js.extractor.trapcache.ITrapCache;
 import com.semmle.util.data.StringUtil;
 import com.semmle.util.exception.Exceptions;
-import com.semmle.util.exception.ResourceError;
 import com.semmle.util.extraction.ExtractorOutputConfig;
 import com.semmle.util.files.FileUtil;
 import com.semmle.util.io.WholeIO;
@@ -439,16 +438,7 @@ public class FileExtractor {
     }
 
     // populate source archive
-    WholeIO wholeIO = new WholeIO(config.getDefaultEncoding(), true);
-    String source = wholeIO.read(f);
-    if (source == null) {
-      if (wholeIO.getLastException() instanceof CharacterCodingException) {
-        System.err.println("Skipped due to unsupported character encoding: " + f);
-        return 0;
-      } else {
-        throw new ResourceError("Failed to read file " + f, wholeIO.getLastException());
-      }
-    }
+    String source = new WholeIO(config.getDefaultEncoding()).strictread(f);
     outputConfig.getSourceArchive().add(f, source);
 
     // extract language-independent bits

javascript/ql/experimental/adaptivethreatmodeling/lib/experimental/adaptivethreatmodeling/EndpointFeatures.qll

@@ -56,7 +56,7 @@ private string getTokenFeature(DataFlow::Node endpoint, string featureName) {
     result =
       concat(API::Node node, string accessPath |
         node.getInducingNode().(DataFlow::CallNode).getAnArgument() = endpoint and
-        accessPath = AccessPaths::getAccessPath(node, includeStructuralInfo)
+        AccessPaths::accessPaths(node, includeStructuralInfo, accessPath, _)
       |
         accessPath, " "
       )
@@ -102,7 +102,9 @@ private string getACallBasedTokenFeatureComponent(
     //
     // would have a callee API name of `mongoose`.
     featureName = "calleeApiName" and
-    result = getAnApiName(call)
+    exists(API::Node apiNode |
+      AccessPaths::accessPaths(apiNode, false, _, result) and call = apiNode.getInducingNode()
+    )
   )
 }
 
@@ -145,16 +147,6 @@ module FunctionBodies {
   }
 }
 
-/**
- * Returns a name of the API that a node originates from, if the node originates from an API.
- *
- * This predicate may have multiple results if the node corresponds to multiple nodes in the API graph forest.
- */
-pragma[inline]
-private string getAnApiName(DataFlow::Node node) {
-  API::moduleImport(result).getASuccessor*().getInducingNode() = node
-}
-
 /**
  * This module provides functionality for getting a representation of the access path of nodes
  * within the program.
@@ -200,65 +192,72 @@ private module AccessPaths {
   }
 
   /** Get the access path for the node. This includes structural information like `member`, `param`, and `functionalarg` if `includeStructuralInfo` is true. */
-  string getAccessPath(API::Node node, Boolean includeStructuralInfo) {
-    node = API::moduleImport(result)
+  predicate accessPaths(
+    API::Node node, Boolean includeStructuralInfo, string accessPath, string apiName
+  ) {
+    //node = API::moduleImport(result)
+    node = API::moduleImport(apiName) and accessPath = apiName
     or
-    exists(API::Node base, string baseName |
-      base.getDepth() < node.getDepth() and baseName = getAccessPath(base, includeStructuralInfo)
+    exists(API::Node previousNode, string previousAccessPath |
+      previousNode.getDepth() < node.getDepth() and
+      accessPaths(previousNode, includeStructuralInfo, previousAccessPath, apiName)
     |
       // e.g. `new X`, `X()`
-      node = [base.getInstance(), base.getReturn()] and
+      node = [previousNode.getInstance(), previousNode.getReturn()] and
       if includeStructuralInfo = true
-      then result = baseName + " instanceorreturn"
-      else result = baseName
+      then accessPath = previousAccessPath + " instanceorreturn"
+      else accessPath = previousAccessPath
       or
       // e.g. `x.y`, `x[y]`, `const { y } = x`, where `y` is non-numeric and is known at analysis
       // time.
       exists(string member |
-        node = base.getMember(member) and
-        not node = base.getUnknownMember() and
+        node = previousNode.getMember(member) and
+        not node = previousNode.getUnknownMember() and
         not isNumericString(member) and
-        not (member = "default" and base = API::moduleImport(_)) and
+        not (member = "default" and previousNode = API::moduleImport(_)) and
         not member = "then" // use the 'promised' edges for .then callbacks
       |
         if includeStructuralInfo = true
-        then result = baseName + " member " + member
-        else result = baseName + " " + member
+        then accessPath = previousAccessPath + " member " + member
+        else accessPath = previousAccessPath + " " + member
       )
       or
       // e.g. `x.y`, `x[y]`, `const { y } = x`, where `y` is numeric or not known at analysis time.
       (
-        node = base.getUnknownMember() or
-        node = base.getMember(any(string s | isNumericString(s)))
+        node = previousNode.getUnknownMember() or
+        node = previousNode.getMember(any(string s | isNumericString(s)))
       ) and
-      if includeStructuralInfo = true then result = baseName + " member" else result = baseName
+      if includeStructuralInfo = true
+      then accessPath = previousAccessPath + " member"
+      else accessPath = previousAccessPath
       or
       // e.g. `x.then(y => ...)`
-      node = base.getPromised() and
-      result = baseName
+      node = previousNode.getPromised() and
+      accessPath = previousAccessPath
       or
       // e.g. `x.y((a, b) => ...)`
       // Name callback parameters after their name in the source code.
       // For example, the `res` parameter in `express.get('/foo', (req, res) => {...})` will be
       // named `express member get functionalarg param res`.
       exists(string paramName |
-        node = getNamedParameter(base.getAParameter(), paramName) and
+        node = getNamedParameter(previousNode.getAParameter(), paramName) and
         (
           if includeStructuralInfo = true
-          then result = baseName + " functionalarg param " + paramName
-          else result = baseName + " " + paramName
+          then accessPath = previousAccessPath + " functionalarg param " + paramName
+          else accessPath = previousAccessPath + " " + paramName
         )
         or
         exists(string callbackName, string index |
           node =
-            getNamedParameter(base.getASuccessor("param " + index).getMember(callbackName),
+            getNamedParameter(previousNode.getASuccessor("param " + index).getMember(callbackName),
               paramName) and
           index != "-1" and // ignore receiver
           if includeStructuralInfo = true
           then
-            result =
-              baseName + " functionalarg " + index + " " + callbackName + " param " + paramName
-          else result = baseName + " " + index + " " + callbackName + " " + paramName
+            accessPath =
+              previousAccessPath + " functionalarg " + index + " " + callbackName + " param " +
+                paramName
+          else accessPath = previousAccessPath + " " + index + " " + callbackName + " " + paramName
         )
       )
     )

javascript/ql/lib/semmle/javascript/GlobalAccessPaths.qll

@@ -426,6 +426,17 @@ module AccessPath {
       result = AccessPath::getAReferenceTo(root, accessPath)
     )
     or
+    // step over extend calls. Handle aliasing both ways through the extend call.
+    exists(
+      DataFlow::SourceNode rootOne, DataFlow::SourceNode rootTwo, string accessPath,
+      ExtendCall extendCall
+    |
+      rootOne = [extendCall, extendCall.getAnOperand().getALocalSource()] and
+      rootTwo = [extendCall, extendCall.getAnOperand().getALocalSource()] and
+      node = pragma[only_bind_into](AccessPath::getAReferenceTo(rootOne, accessPath)) and
+      result = AccessPath::getAReferenceTo(rootTwo, accessPath)
+    )
+    or
     result = node.getALocalSource()
   }
 

javascript/ql/lib/semmle/javascript/PackageExports.qll

@@ -11,11 +11,36 @@ private import semmle.javascript.internal.CachedStages
  * Gets a parameter that is a library input to a top-level package.
  */
 cached
-DataFlow::ParameterNode getALibraryInputParameter() {
+DataFlow::SourceNode getALibraryInputParameter() {
   Stages::Taint::ref() and
   exists(int bound, DataFlow::FunctionNode func |
-    func = getAValueExportedByPackage().getABoundFunctionValue(bound) and
+    func = getAValueExportedByPackage().getABoundFunctionValue(bound)
+  |
     result = func.getParameter(any(int arg | arg >= bound))
+    or
+    result = getAnArgumentsRead(func.getFunction())
+  )
+}
+
+private DataFlow::SourceNode getAnArgumentsRead(Function func) {
+  exists(DataFlow::PropRead read |
+    not read.getPropertyName() = "length" and
+    result = read
+  |
+    read.getBase() = func.getArgumentsVariable().getAnAccess().flow()
+    or
+    exists(DataFlow::MethodCallNode call |
+      call =
+        DataFlow::globalVarRef("Array")
+            .getAPropertyRead("prototype")
+            .getAPropertyRead("slice")
+            .getAMethodCall("call")
+      or
+      call = DataFlow::globalVarRef("Array").getAMethodCall("from")
+    |
+      call.getArgument(0) = func.getArgumentsVariable().getAnAccess().flow() and
+      call.flowsTo(read.getBase())
+    )
   )
 }
 

javascript/ql/lib/semmle/javascript/dataflow/AbstractProperties.qll

@@ -86,6 +86,7 @@ class AbstractProtoProperty extends AbstractProperty {
  * has to be toplevel predicate to avoid a spurious type join with `AbstractProperty`,
  * which in turn introduces a materialization.
  */
+pragma[noinline]
 private AbstractValue getAnAssignedValue(AbstractValue b, string p) {
   exists(AnalyzedPropertyWrite apw | apw.writesValue(b, p, result))
 }

javascript/ql/lib/semmle/javascript/dataflow/Configuration.qll

@@ -493,6 +493,7 @@ private predicate barrierGuardBlocksEdge(
  *
  * This predicate exists to get a better join-order for the `barrierGuardBlocksEdge` predicate above.
  */
+pragma[noinline]
 private BasicBlock getADominatedBasicBlock(BarrierGuardNode guard, ConditionGuardNode cond) {
   barrierGuardIsRelevant(guard) and
   guard.getEnclosingExpr() = cond.getTest() and
@@ -996,6 +997,7 @@ private predicate exploratoryLoadStep(
  *
  * This private predicate is only used in `exploratoryLoadStep`, and exists as a separate predicate to give the compiler a hint about join-ordering.
  */
+pragma[noinline]
 private string getAForwardRelevantLoadProperty(DataFlow::Configuration cfg) {
   exists(DataFlow::Node previous | isRelevantForward(previous, cfg) |
     basicStoreStep(previous, _, result) or
@@ -1055,6 +1057,7 @@ private predicate exploratoryBackwardStoreStep(
  *
  * This private predicate is only used in `exploratoryBackwardStoreStep`, and exists as a separate predicate to give the compiler a hint about join-ordering.
  */
+pragma[noinline]
 private string getABackwardsRelevantStoreProperty(DataFlow::Configuration cfg) {
   exists(DataFlow::Node mid | isRelevant(mid, cfg) |
     basicLoadStep(mid, _, result) or
@@ -1142,7 +1145,8 @@ private predicate reachableFromInput(
   DataFlow::Configuration cfg, PathSummary summary
 ) {
   callInputStep(f, invk, input, nd, cfg) and
-  summary = PathSummary::level()
+  summary = PathSummary::level() and
+  not cfg.isLabeledBarrier(nd, summary.getEndLabel())
   or
   exists(DataFlow::Node mid, PathSummary oldSummary |
     reachableFromInput(f, invk, input, mid, cfg, oldSummary) and
@@ -1672,6 +1676,7 @@ private predicate onPath(DataFlow::Node nd, DataFlow::Configuration cfg, PathSum
  *
  * This predicate has been outlined from `onPath` to give the optimizer a hint about join-ordering.
  */
+pragma[noinline]
 private predicate onPathStep(
   DataFlow::Node nd, DataFlow::Configuration cfg, PathSummary summary, PathSummary stepSummary,
   DataFlow::Node mid

javascript/ql/lib/semmle/javascript/dataflow/internal/CallGraphs.qll

@@ -79,6 +79,12 @@ module CallGraph {
       cls.getAClassReference(t.continue()) = result
     )
     or
+    exists(DataFlow::ObjectLiteralNode object, string prop |
+      function = object.getAPropertySource(prop) and
+      result = getAnObjectLiteralRef(object).getAPropertyRead(prop) and
+      t.start()
+    )
+    or
     exists(DataFlow::FunctionNode outer |
       result = getAFunctionReference(outer, 0, t.continue()).getAnInvocation() and
       locallyReturnedFunction(outer, function)
@@ -197,11 +203,39 @@ module CallGraph {
     )
     or
     exists(DataFlow::ObjectLiteralNode object, string name |
-      ref = object.getAPropertyRead(name) and
+      ref = getAnObjectLiteralRef(object).getAPropertyRead(name) and
       result = object.getPropertyGetter(name)
       or
-      ref = object.getAPropertyWrite(name) and
+      ref = getAnObjectLiteralRef(object).getAPropertyWrite(name) and
       result = object.getPropertySetter(name)
     )
   }
+
+  private predicate shouldTrackObjectLiteral(DataFlow::ObjectLiteralNode node) {
+    (
+      node.getAPropertySource() instanceof DataFlow::FunctionNode
+      or
+      exists(node.getPropertyGetter(_))
+      or
+      exists(node.getPropertySetter(_))
+    ) and
+    not node.getTopLevel().isExterns()
+  }
+
+  /**
+   * Gets a step summary for tracking object literals.
+   *
+   * To avoid false flow from callbacks passed in via "named parameters", we only track object
+   * literals out of returns, not into calls.
+   */
+  private StepSummary objectLiteralStep() { result = LevelStep() or result = ReturnStep() }
+
+  /** Gets a node that refers to the given object literal, via a limited form of type tracking. */
+  cached
+  DataFlow::SourceNode getAnObjectLiteralRef(DataFlow::ObjectLiteralNode node) {
+    shouldTrackObjectLiteral(node) and
+    result = node
+    or
+    StepSummary::step(getAnObjectLiteralRef(node), result, objectLiteralStep())
+  }
 }

javascript/ql/lib/semmle/javascript/filters/ClassifyFiles.qll

@@ -45,6 +45,49 @@ private predicate looksLikeExterns(TopLevel tl) {
   )
 }
 
+/**
+ * Holds if `f` contains generated or minified code.
+ */
+predicate isGeneratedCodeFile(File f) { isGenerated(f.getATopLevel()) }
+
+/**
+ * Holds if `f` contains test code.
+ */
+predicate isTestFile(File f) {
+  exists(Test t | t.getFile() = f)
+  or
+  exists(string stemExt | stemExt = "test" or stemExt = "spec" |
+    f = getTestFile(any(File orig), stemExt)
+  )
+  or
+  f.getAbsolutePath().regexpMatch(".*/__(mocks|tests)__/.*")
+}
+
+/**
+ * Holds if `f` contains externs declarations.
+ */
+predicate isExternsFile(File f) {
+  (f.getATopLevel().isExterns() or looksLikeExterns(f.getATopLevel()))
+}
+
+/**
+ * Holds if `f` contains library code.
+ */
+predicate isLibaryFile(File f) { f.getATopLevel() instanceof FrameworkLibraryInstance }
+
+/**
+ * Holds if `f` contains template code.
+ */
+predicate isTemplateFile(File f) {
+  exists(JSParseError err | maybeCausedByTemplate(err) | f = err.getFile())
+  or
+  // Polymer templates
+  exists(HTML::Element elt | elt.getName() = "template" |
+    f = elt.getFile() and
+    not f.getExtension() = "vue"
+  )
+}
+
 /**
  * Holds if `f` is classified as belonging to `category`.
  *
@@ -55,33 +98,15 @@ private predicate looksLikeExterns(TopLevel tl) {
  *   - `"library"`: `f` contains library code;
  *   - `"template"`: `f` contains template code.
  */
+pragma[inline]
 predicate classify(File f, string category) {
-  isGenerated(f.getATopLevel()) and category = "generated"
+  isGeneratedCodeFile(f) and category = "generated"
   or
-  (
-    exists(Test t | t.getFile() = f)
-    or
-    exists(string stemExt | stemExt = "test" or stemExt = "spec" |
-      f = getTestFile(any(File orig), stemExt)
-    )
-    or
-    f.getAbsolutePath().regexpMatch(".*/__(mocks|tests)__/.*")
-  ) and
-  category = "test"
+  isTestFile(f) and category = "test"
   or
-  (f.getATopLevel().isExterns() or looksLikeExterns(f.getATopLevel())) and
-  category = "externs"
+  isExternsFile(f) and category = "externs"
   or
-  f.getATopLevel() instanceof FrameworkLibraryInstance and category = "library"
+  isLibaryFile(f) and category = "library"
   or
-  exists(JSParseError err | maybeCausedByTemplate(err) |
-    f = err.getFile() and category = "template"
-  )
-  or
-  // Polymer templates
-  exists(HTML::Element elt | elt.getName() = "template" |
-    f = elt.getFile() and
-    category = "template" and
-    not f.getExtension() = "vue"
-  )
+  isTemplateFile(f) and category = "template"
 }

javascript/ql/lib/semmle/javascript/frameworks/CryptoLibraries.qll

@@ -25,6 +25,26 @@ abstract class CryptographicOperation extends Expr {
  */
 abstract class CryptographicKey extends DataFlow::ValueNode { }
 
+/**
+ * The creation of a cryptographic key.
+ */
+abstract class CryptographicKeyCreation extends DataFlow::Node {
+  /**
+   * Gets the algorithm used to create the key.
+   */
+  abstract CryptographicAlgorithm getAlgorithm();
+
+  /**
+   * Gets the size of the key.
+   */
+  abstract int getSize();
+
+  /**
+   * Gets whether the key is symmetric.
+   */
+  abstract predicate isSymmetricKey();
+}
+
 /**
  * A key used in a cryptographic algorithm, viewed as a `CredentialsExpr`.
  */
@@ -141,14 +161,9 @@ private module NodeJSCrypto {
        *       Also matches `createHash`, `createHmac`, `createSign` instead of `createCipher`.
        */
 
-      exists(DataFlow::SourceNode mod, string createSuffix |
-        createSuffix = "Hash" or
-        createSuffix = "Hmac" or
-        createSuffix = "Sign" or
-        createSuffix = "Cipher"
-      |
+      exists(DataFlow::SourceNode mod |
         mod = DataFlow::moduleImport("crypto") and
-        this = mod.getAMemberCall("create" + createSuffix) and
+        this = mod.getAMemberCall("create" + ["Hash", "Hmac", "Sign", "Cipher"]) and
         algorithm.matchesName(getArgument(0).getStringValue())
       )
     }
@@ -156,6 +171,52 @@ private module NodeJSCrypto {
     CryptographicAlgorithm getAlgorithm() { result = algorithm }
   }
 
+  private class CreateKey extends CryptographicKeyCreation, DataFlow::CallNode {
+    boolean symmetric;
+
+    CreateKey() {
+      // crypto.generateKey(type, options, callback)
+      // crypto.generateKeyPair(type, options, callback)
+      // crypto.generateKeyPairSync(type, options)
+      // crypto.generateKeySync(type, options)
+      exists(DataFlow::SourceNode mod, string keyType |
+        keyType = "Key" and symmetric = true
+        or
+        keyType = "KeyPair" and symmetric = false
+      |
+        mod = DataFlow::moduleImport("crypto") and
+        this = mod.getAMemberCall("generate" + keyType + ["", "Sync"])
+      )
+    }
+
+    override CryptographicAlgorithm getAlgorithm() {
+      result.matchesName(getArgument(0).getStringValue())
+    }
+
+    override int getSize() {
+      symmetric = true and
+      result = getOptionArgument(1, "length").getIntValue()
+      or
+      symmetric = false and
+      result = getOptionArgument(1, "modulusLength").getIntValue()
+    }
+
+    override predicate isSymmetricKey() { symmetric = true }
+  }
+
+  private class CreateDiffieHellmanKey extends CryptographicKeyCreation, DataFlow::CallNode {
+    // require("crypto").createDiffieHellman(prime_length);
+    CreateDiffieHellmanKey() {
+      this = DataFlow::moduleMember("crypto", "createDiffieHellman").getACall()
+    }
+
+    override CryptographicAlgorithm getAlgorithm() { none() }
+
+    override int getSize() { result = getArgument(0).getIntValue() }
+
+    override predicate isSymmetricKey() { none() }
+  }
+
   private class Apply extends CryptographicOperation, MethodCallExpr {
     InstantiatedAlgorithm instantiation;
 
@@ -282,6 +343,35 @@ private module CryptoJS {
       )
     }
   }
+
+  private class CreateKey extends CryptographicKeyCreation, DataFlow::CallNode {
+    string algorithm;
+    int optionArg;
+
+    CreateKey() {
+      // var key = CryptoJS.PBKDF2(password, salt, { keySize: 8 });
+      this =
+        getAlgorithmExpr(any(CryptographicAlgorithm algo | algo.getName() = algorithm)).getACall() and
+      optionArg = 2
+      or
+      // var key = CryptoJS.algo.PBKDF2.create({ keySize: 8 });
+      this =
+        DataFlow::moduleMember("crypto-js", "algo")
+            .getAPropertyRead(algorithm)
+            .getAMethodCall("create") and
+      optionArg = 0
+    }
+
+    override CryptographicAlgorithm getAlgorithm() { result.matchesName(algorithm) }
+
+    override int getSize() {
+      result = getOptionArgument(optionArg, "keySize").getIntValue() * 32 // size is in words
+      or
+      result = getArgument(optionArg).getIntValue() * 32 // size is in words
+    }
+
+    override predicate isSymmetricKey() { any() }
+  }
 }
 
 /**
@@ -467,6 +557,39 @@ private module Forge {
   private class Key extends CryptographicKey {
     Key() { this = any(KeyCipher cipher).getKey() }
   }
+
+  private class CreateKey extends CryptographicKeyCreation, DataFlow::CallNode {
+    CryptographicAlgorithm algorithm;
+
+    CreateKey() {
+      // var cipher = forge.rc2.createEncryptionCipher(key, 128);
+      this =
+        getAnImportNode()
+            .getAPropertyRead(any(string s | algorithm.matchesName(s)))
+            .getAMemberCall("createEncryptionCipher")
+      or
+      // var key = forge.random.getBytesSync(16);
+      // var cipher = forge.cipher.createCipher('AES-CBC', key);
+      this =
+        getAnImportNode()
+            .getAPropertyRead("cipher")
+            .getAMemberCall(["createCipher", "createDecipher"]) and
+      algorithm.matchesName(this.getArgument(0).getStringValue())
+    }
+
+    override CryptographicAlgorithm getAlgorithm() { result = algorithm }
+
+    override int getSize() {
+      result = this.getArgument(1).getIntValue()
+      or
+      exists(DataFlow::CallNode call | call.getCalleeName() = ["getBytes", "getBytesSync"] |
+        getArgument(1).getALocalSource() = call and
+        result = call.getArgument(0).getIntValue() * 8 // bytes to bits
+      )
+    }
+
+    override predicate isSymmetricKey() { any() }
+  }
 }
 
 /**
@@ -556,13 +679,38 @@ private module Hasha {
 
     override CryptographicAlgorithm getAlgorithm() { result = algorithm }
   }
+}
 
-  /**
-   * Provides classes for working with the `express-jwt` package (https://github.com/auth0/express-jwt);
-   */
-  module ExpressJwt {
-    private class Key extends CryptographicKey {
-      Key() { this = DataFlow::moduleMember("express-jwt", "sign").getACall().getArgument(1) }
-    }
+/**
+ * Provides classes for working with the `express-jwt` package (https://github.com/auth0/express-jwt);
+ */
+private module ExpressJwt {
+  private class Key extends CryptographicKey {
+    Key() { this = DataFlow::moduleMember("express-jwt", "sign").getACall().getArgument(1) }
+  }
+}
+
+/**
+ * Provides classes for working with the `node-rsa` package (https://www.npmjs.com/package/node-rsa)
+ */
+private module NodeRsa {
+  private class CreateKey extends CryptographicKeyCreation, API::InvokeNode {
+    CryptographicAlgorithm algorithm;
+
+    CreateKey() {
+      this = API::moduleImport("node-rsa").getAnInstantiation()
+      or
+      this = API::moduleImport("node-rsa").getInstance().getMember("generateKeyPair").getACall()
+    }
+
+    override CryptographicAlgorithm getAlgorithm() { result.matchesName("rsa") }
+
+    override int getSize() {
+      result = this.getArgument(0).getIntValue()
+      or
+      result = this.getOptionArgument(0, "b").getIntValue()
+    }
+
+    override predicate isSymmetricKey() { none() }
   }
 }

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

@@ -46,7 +46,7 @@ private module AlgorithmNames {
     name = ["ARGON2", "PBKDF2", "BCRYPT", "SCRYPT"]
   }
 
-  predicate isWeakPasswordHashingAlgorithm(string name) { none() }
+  predicate isWeakPasswordHashingAlgorithm(string name) { name = "EVPKDF" }
 }
 
 private import AlgorithmNames
@@ -85,11 +85,13 @@ abstract class CryptographicAlgorithm extends TCryptographicAlgorithm {
 
   /**
    * Holds if the name of this algorithm matches `name` modulo case,
-   * white space, dashes, and underscores.
+   * white space, dashes, underscores, and anything after a dash in the name
+   * (to ignore modes of operation, such as CBC or ECB).
    */
   bindingset[name]
   predicate matchesName(string name) {
-    name.toUpperCase().regexpReplaceAll("[-_ ]", "") = getName()
+    [name.toUpperCase(), name.toUpperCase().regexpCapture("^(\\w+)(?:-.*)?$", 1)]
+        .regexpReplaceAll("[-_ ]", "") = getName()
   }
 
   /**

javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutingAssignmentCustomizations.qll

@@ -13,7 +13,12 @@ module PrototypePollutingAssignment {
   /**
    * A data flow source for untrusted data from which the special `__proto__` property name may be arise.
    */
-  abstract class Source extends DataFlow::Node { }
+  abstract class Source extends DataFlow::Node {
+    /**
+     * Gets a string that describes the type of source.
+     */
+    abstract string describe();
+  }
 
   /**
    * A data flow sink for prototype-polluting assignments or untrusted property names.
@@ -44,6 +49,8 @@ module PrototypePollutingAssignment {
       this = any(DataFlow::PropWrite write).getBase()
       or
       this = any(ExtendCall c).getDestinationOperand()
+      or
+      this = any(DeleteExpr del).getOperand().flow().(DataFlow::PropRef).getBase()
     }
 
     override DataFlow::FlowLabel getAFlowLabel() { result instanceof ObjectPrototype }
@@ -52,5 +59,18 @@ module PrototypePollutingAssignment {
   /** A remote flow source or location.{hash,search} as a taint source. */
   private class DefaultSource extends Source {
     DefaultSource() { this instanceof RemoteFlowSource }
+
+    override string describe() { result = "user controlled input" }
+  }
+
+  import semmle.javascript.PackageExports as Exports
+
+  /**
+   * A parameter of an exported function, seen as a source prototype-polluting assignment.
+   */
+  class ExternalInputSource extends Source, DataFlow::SourceNode {
+    ExternalInputSource() { this = Exports::getALibraryInputParameter() }
+
+    override string describe() { result = "library input" }
   }
 }

javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutingAssignmentQuery.qll

@@ -10,7 +10,8 @@
 private import javascript
 private import semmle.javascript.DynamicPropertyAccess
 private import semmle.javascript.dataflow.InferredTypes
-private import PrototypePollutingAssignmentCustomizations::PrototypePollutingAssignment
+import PrototypePollutingAssignmentCustomizations::PrototypePollutingAssignment
+private import semmle.javascript.filters.ClassifyFiles as ClassifyFiles
 
 // Materialize flow labels
 private class ConcreteObjectPrototype extends ObjectPrototype {
@@ -31,7 +32,27 @@ class Configuration extends TaintTracking::Configuration {
     node instanceof Sanitizer
     or
     // Concatenating with a string will in practice prevent the string `__proto__` from arising.
-    node instanceof StringOps::ConcatenationRoot
+    exists(StringOps::ConcatenationRoot root | node = root |
+      // Exclude the string coercion `"" + node` from this filter.
+      not node.(StringOps::ConcatenationNode).isCoercion()
+    )
+    or
+    node instanceof DataFlow::ThisNode
+    or
+    // Stop at .replace() calls that likely prevent __proto__ from arising
+    exists(StringReplaceCall replace |
+      node = replace and
+      replace.getAReplacedString() = ["_", "p", "r", "o", "t"] and
+      // Replacing with "_" is likely to be exploitable
+      not replace.getRawReplacement().getStringValue() = "_" and
+      (
+        replace.isGlobal()
+        or
+        // Non-global replace with a non-empty string can also prevent __proto__ by
+        // inserting a chunk of text that doesn't fit anywhere in __proto__
+        not replace.getRawReplacement().getStringValue() = ""
+      )
+    )
   }
 
   override predicate isAdditionalFlowStep(
@@ -62,6 +83,29 @@ class Configuration extends TaintTracking::Configuration {
       inlbl.isTaint() and
       outlbl instanceof ObjectPrototype
     )
+    or
+    DataFlow::localFieldStep(pred, succ) and inlbl = outlbl
+  }
+
+  override predicate hasFlowPath(DataFlow::SourcePathNode source, DataFlow::SinkPathNode sink) {
+    super.hasFlowPath(source, sink) and
+    // require that there is a path without unmatched return steps
+    DataFlow::hasPathWithoutUnmatchedReturn(source, sink) and
+    // filter away paths that start with library inputs and end with a write to a fixed property.
+    not exists(ExternalInputSource src, Sink snk, DataFlow::PropWrite write |
+      source.getNode() = src and sink.getNode() = snk
+    |
+      snk = write.getBase() and
+      (
+        // fixed property name
+        exists(write.getPropertyName())
+        or
+        // non-string property name (likely number)
+        exists(Expr prop | prop = write.getPropertyNameExpr() |
+          not prop.analyze().getAType() = TTString()
+        )
+      )
+    )
   }
 
   override predicate isLabeledBarrier(DataFlow::Node node, DataFlow::FlowLabel lbl) {
@@ -78,7 +122,8 @@ class Configuration extends TaintTracking::Configuration {
     guard instanceof InstanceofCheck or
     guard instanceof IsArrayCheck or
     guard instanceof TypeofCheck or
-    guard instanceof EqualityCheck
+    guard instanceof EqualityCheck or
+    guard instanceof IncludesCheck
   }
 }
 
@@ -91,7 +136,8 @@ private DataFlow::SourceNode prototypeLessObject(DataFlow::TypeTracker t) {
   t.start() and
   // We assume the argument to Object.create is not Object.prototype, since most
   // users wouldn't bother to call Object.create in that case.
-  result = DataFlow::globalVarRef("Object").getAMemberCall("create")
+  result = DataFlow::globalVarRef("Object").getAMemberCall("create") and
+  not result.getFile() instanceof TestFile
   or
   // Allow use of SharedFlowSteps to track a bit further
   exists(DataFlow::Node mid |
@@ -102,6 +148,14 @@ private DataFlow::SourceNode prototypeLessObject(DataFlow::TypeTracker t) {
   exists(DataFlow::TypeTracker t2 | result = prototypeLessObject(t2).track(t2, t))
 }
 
+/**
+ * A test file.
+ * Objects created in such files are ignored in the `prototypeLessObject` predicate.
+ */
+private class TestFile extends File {
+  TestFile() { ClassifyFiles::isTestFile(this) }
+}
+
 /** Holds if `Object.prototype` has a member named `prop`. */
 private predicate isPropertyPresentOnObjectPrototype(string prop) {
   exists(ExternalInstanceMemberDecl decl |
@@ -198,3 +252,15 @@ private class EqualityCheck extends TaintTracking::SanitizerGuardNode, DataFlow:
     outcome = astNode.getPolarity().booleanNot()
   }
 }
+
+/**
+ * Sanitizer guard of the form `x.includes("__proto__")`.
+ */
+private class IncludesCheck extends TaintTracking::LabeledSanitizerGuardNode, InclusionTest {
+  IncludesCheck() { this.getContainedNode().mayHaveStringValue("__proto__") }
+
+  override predicate sanitizes(boolean outcome, Expr e) {
+    e = getContainerNode().asExpr() and
+    outcome = getPolarity().booleanNot()
+  }
+}

javascript/ql/lib/semmle/javascript/security/dataflow/TaintedPathCustomizations.qll

@@ -445,17 +445,25 @@ module TaintedPath {
   /**
    * An expression of form `x.includes("..")` or similar.
    */
-  class ContainsDotDotSanitizer extends BarrierGuardNode {
-    StringOps::Includes contains;
-
-    ContainsDotDotSanitizer() {
-      this = contains and
-      isDotDotSlashPrefix(contains.getSubstring())
-    }
+  class ContainsDotDotSanitizer extends BarrierGuardNode instanceof StringOps::Includes {
+    ContainsDotDotSanitizer() { isDotDotSlashPrefix(super.getSubstring()) }
 
     override predicate blocks(boolean outcome, Expr e, DataFlow::FlowLabel label) {
-      e = contains.getBaseString().asExpr() and
-      outcome = contains.getPolarity().booleanNot() and
+      e = super.getBaseString().asExpr() and
+      outcome = super.getPolarity().booleanNot() and
+      label.(Label::PosixPath).canContainDotDotSlash() // can still be bypassed by normalized absolute path
+    }
+  }
+
+  /**
+   * An expression of form `x.matches(/\.\./)` or similar.
+   */
+  class ContainsDotDotRegExpSanitizer extends BarrierGuardNode instanceof StringOps::RegExpTest {
+    ContainsDotDotRegExpSanitizer() { super.getRegExp().getAMatchedString() = [".", "..", "../"] }
+
+    override predicate blocks(boolean outcome, Expr e, DataFlow::FlowLabel label) {
+      e = super.getStringOperand().asExpr() and
+      outcome = super.getPolarity().booleanNot() and
       label.(Label::PosixPath).canContainDotDotSlash() // can still be bypassed by normalized absolute path
     }
   }

javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeShellCommandConstructionCustomizations.qll

@@ -50,14 +50,14 @@ module UnsafeShellCommandConstruction {
   /**
    * A parameter of an exported function, seen as a source for shell command constructed from library input.
    */
-  class ExternalInputSource extends Source, DataFlow::ParameterNode {
+  class ExternalInputSource extends Source, DataFlow::SourceNode {
     ExternalInputSource() {
       this = Exports::getALibraryInputParameter() and
       not (
         // looks to be on purpose.
-        this.getName() = ["cmd", "command"]
+        this.(DataFlow::ParameterNode).getName() = ["cmd", "command"]
         or
-        this.getName().regexpMatch(".*(Cmd|Command)$") // ends with "Cmd" or "Command"
+        this.(DataFlow::ParameterNode).getName().regexpMatch(".*(Cmd|Command)$") // ends with "Cmd" or "Command"
       )
     }
   }

javascript/ql/lib/semmle/javascript/security/performance/PolynomialReDoSCustomizations.qll

@@ -124,7 +124,7 @@ module PolynomialReDoS {
   /**
    * A parameter of an exported function, seen as a source for polynomial-redos.
    */
-  class ExternalInputSource extends Source, DataFlow::ParameterNode {
+  class ExternalInputSource extends Source, DataFlow::SourceNode {
     ExternalInputSource() { this = Exports::getALibraryInputParameter() }
 
     override string getKind() { result = "library" }

javascript/ql/src/Security/CWE-295/DisablingCertificateValidation.ql

@@ -8,6 +8,7 @@
  * @id js/disabling-certificate-validation
  * @tags security
  *       external/cwe/cwe-295
+ *       external/cwe/cwe-297
  */
 
 import javascript

javascript/ql/src/Security/CWE-312/CleartextLogging.ql

@@ -11,6 +11,7 @@
  *       external/cwe/cwe-312
  *       external/cwe/cwe-315
  *       external/cwe/cwe-359
+ *       external/cwe/cwe-532
  */
 
 import javascript

javascript/ql/src/Security/CWE-326/InsufficientKeySize.qhelp

@@ -0,0 +1,43 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>
+Modern encryption relies on it being computationally infeasible to break the cipher and decode a message without the key.
+As computational power increases, the ability to break ciphers grows and keys need to become larger.
+</p>
+</overview>
+
+<recommendation>
+<p>
+An encryption key should be at least 2048-bit long when using RSA encryption, and 128-bit long when using 
+symmetric encryption.
+</p>
+</recommendation>
+
+<references>
+<li>
+Wikipedia:
+<a href="https://en.wikipedia.org/wiki/RSA_(cryptosystem)">RSA</a>.
+</li>
+<li>
+Wikipedia:
+<a href="https://en.wikipedia.org/wiki/Advanced_Encryption_Standard">AES</a>.
+</li>
+<li>
+NodeJS: 
+<a href="https://nodejs.org/api/crypto.html">Crypto</a>.
+</li>
+<li>
+NIST:
+<a href="https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-131Ar1.pdf">
+Recommendation for Transitioning the Use of Cryptographic Algorithms and Key Lengths</a>.
+</li>
+<li>
+Wikipedia:
+<a href="https://en.wikipedia.org/wiki/Key_size">Key size</a>
+</li>
+</references>
+</qhelp>

javascript/ql/src/Security/CWE-326/InsufficientKeySize.ql

@@ -0,0 +1,36 @@
+/**
+ * @name Use of a weak cryptographic key
+ * @description Using a weak cryptographic key can allow an attacker to compromise security.
+ * @kind problem
+ * @problem.severity warning
+ * @security-severity 7.5
+ * @precision high
+ * @id js/insufficient-key-size
+ * @tags security
+ *       external/cwe/cwe-326
+ */
+
+import javascript
+
+from CryptographicKeyCreation key, int size, string msg, string algo
+where
+  size = key.getSize() and
+  (
+    algo = key.getAlgorithm() + " "
+    or
+    not exists(key.getAlgorithm()) and algo = ""
+  ) and
+  (
+    size < 128 and
+    key.isSymmetricKey() and
+    msg =
+      "Creation of an symmetric " + algo + "key uses " + size +
+        " bits, which is below 128 and considered breakable."
+    or
+    size < 2048 and
+    not key.isSymmetricKey() and
+    msg =
+      "Creation of an asymmetric " + algo + "key uses " + size +
+        " bits, which is below 2048 and considered breakable."
+  )
+select key, msg

javascript/ql/src/Security/CWE-346/CorsMisconfigurationForCredentials.ql

@@ -9,6 +9,7 @@
  * @tags security
  *       external/cwe/cwe-346
  *       external/cwe/cwe-639
+ *       external/cwe/cwe-942
  */
 
 import javascript

javascript/ql/src/Security/CWE-384/SessionFixation.qhelp

@@ -0,0 +1,48 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>
+Reusing a session could allow an attacker to gain unauthorized access to another account. Always
+ensure that, when a user logs in or out, the current session is abandoned so that a new
+session may be started.
+</p>
+</overview>
+
+<recommendation>
+
+<p>
+Always use <code>req.session.regenerate(...);</code> to start a new session when
+a user logs in or out.
+</p>
+
+</recommendation>
+<example>
+
+<p>
+The following example shows the previous session being used after authentication.
+This would allow a previous user to use the new user's account.
+</p>
+
+<sample src="examples/SessionFixation.js" />
+
+<p>
+This code example solves the problem by not reusing the session, and instead calling <code>req.session.regenerate()</code>
+to ensure that the session is not reused.
+</p>
+<sample src="examples/SessionFixationFixed.js" />
+
+</example>
+<references>
+<li>
+  OWASP: <a href="https://www.owasp.org/index.php/Session_fixation">Session fixation</a>
+</li>
+<li>
+  Stack Overflow: <a href="https://stackoverflow.com/questions/22209354/creating-a-new-session-after-authentication-with-passport/30468384#30468384">Creating a new session after authentication with Passport</a>
+</li>
+<li>
+  jscrambler.com: <a href="https://blog.jscrambler.com/best-practices-for-secure-session-management-in-node">Best practices for secure session management in Node</a>
+</li>
+</references>
+</qhelp>

javascript/ql/src/Security/CWE-384/SessionFixation.ql

@@ -0,0 +1,58 @@
+/**
+ * @name Failure to abandon session
+ * @description Reusing an existing session as a different user could allow
+ *              an attacker to access someone else's account by using
+ *              their session.
+ * @kind problem
+ * @problem.severity warning
+ * @security-severity 5
+ * @precision medium
+ * @id js/session-fixation
+ * @tags security
+ *       external/cwe/cwe-384
+ */
+
+import javascript
+
+/**
+ * Holds if `setup` uses express-session (or similar) to log in a user.
+ */
+pragma[inline]
+predicate isLoginSetup(Express::RouteSetup setup) {
+  // either some path that contains "login" with a write to `req.session`
+  setup.getPath().matches("%login%") and
+  exists(
+    setup
+        .getARouteHandler()
+        .(Express::RouteHandler)
+        .getARequestSource()
+        .ref()
+        .getAPropertyRead("session")
+        .getAPropertyWrite()
+  )
+  or
+  // or an authentication method is used (e.g. `passport.authenticate`)
+  setup.getARouteHandler().(DataFlow::CallNode).getCalleeName() = "authenticate"
+}
+
+/**
+ * Holds if `handler` regenerates its session using `req.session.regenerate`.
+ */
+pragma[inline]
+predicate regeneratesSession(Express::RouteSetup setup) {
+  exists(
+    setup
+        .getARouteHandler()
+        .(Express::RouteHandler)
+        .getARequestSource()
+        .ref()
+        .getAPropertyRead("session")
+        .getAPropertyRead("regenerate")
+  )
+}
+
+from Express::RouteSetup setup
+where
+  isLoginSetup(setup) and
+  not regeneratesSession(setup)
+select setup, "Route handler does not invalidate session following login"

javascript/ql/src/Security/CWE-384/examples/SessionFixation.js

@@ -0,0 +1,18 @@
+const express = require('express');
+const session = require('express-session');
+var bodyParser = require('body-parser')
+const app = express();
+app.use(bodyParser.urlencoded({ extended: false }))
+app.use(session({
+    secret: 'keyboard cat'
+}));
+
+app.post('/login', function (req, res) {
+    // Check that username password matches
+    if (req.body.username === 'admin' && req.body.password === 'admin') {
+        req.session.authenticated = true;
+        res.redirect('/');
+    } else {
+        res.redirect('/login');
+    }
+});

javascript/ql/src/Security/CWE-384/examples/SessionFixationFixed.js

@@ -0,0 +1,24 @@
+const express = require('express');
+const session = require('express-session');
+var bodyParser = require('body-parser')
+const app = express();
+app.use(bodyParser.urlencoded({ extended: false }))
+app.use(session({
+    secret: 'keyboard cat'
+}));
+
+app.post('/login', function (req, res) {
+    // Check that username password matches
+    if (req.body.username === 'admin' && req.body.password === 'admin') {
+        req.session.regenerate(function (err) {
+            if (err) {
+                res.send('Error');
+            } else {
+                req.session.authenticated = true;
+                res.redirect('/');
+            }
+        });
+    } else {
+        res.redirect('/login');
+    }
+});

javascript/ql/src/Security/CWE-915/PrototypePollutingAssignment.ql

@@ -24,4 +24,4 @@ from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
 where cfg.hasFlowPath(source, sink)
 select sink, source, sink,
   "This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@.",
-  source.getNode(), "here"
+  source.getNode(), source.getNode().(Source).describe()

javascript/ql/src/experimental/Security/CWE-918/SSRF.js

@@ -0,0 +1,15 @@
+const axios = require('axios');
+
+export const handler = async (req, res, next) => {
+  const { target } = req.body;
+
+  try {
+    // BAD: `target` is controlled by the attacker
+    const response = await axios.get('https://example.com/current_api/' + target);
+
+    // process request response
+    use(response);
+  } catch (err) {
+    // process error
+  }
+};

javascript/ql/src/experimental/Security/CWE-918/SSRF.qhelp

@@ -0,0 +1,49 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<overview>
+    <p>
+        Directly incorporating user input into an HTTP request without validating the input can facilitate
+        server side request forgery attacks, where the attacker essentially controls the request.
+    </p>
+</overview>
+
+<recommendation>
+    <p>
+        To guard against server side request forgery, it is advisable to avoid putting user input directly into a
+        network request. If using user input is necessary, then is mandatory to validate them. Only allow numeric and alphanumeric values.
+        URL encoding is not a solution in certain scenarios, such as, an architecture build over NGINX proxies.
+    </p>
+</recommendation>
+
+<example>
+    <p>
+        The following example shows an HTTP request parameter being used directly in a URL request without
+        validating the input, which facilitates an SSRF attack. The request <code>axios.get("https://example.com/current_api/"+target)</code> is
+        vulnerable since attackers can choose the value of <code>target</code> to be anything they want. For
+        instance, the attacker can choose <code>"../super_secret_api"</code> as the target, causing the
+        URL to become <code>"https://example.com/super_secret_api"</code>.
+    </p>
+
+    <p>
+        A request to <code>https://example.com/super_secret_api</code> may be problematic if that api is not
+        meant to be directly accessible from the attacker's machine.
+    </p>
+
+    <sample src="SSRF.js"/>
+
+    <p>
+        One way to remedy the problem is to validate the user input to only allow alphanumeric values:
+    </p>
+
+    <sample src="SSRFGood.js"/>
+</example>
+
+<references>
+
+<li>OWASP: <a href="https://www.owasp.org/www-community/attacks/Server_Side_Request_Forgery">SSRF</a></li>
+
+</references>
+</qhelp>

javascript/ql/src/experimental/Security/CWE-918/SSRF.ql

@@ -0,0 +1,19 @@
+/**
+ * @id javascript/ssrf
+ * @kind path-problem
+ * @name Uncontrolled data used in network request
+ * @description Sending network requests with user-controlled data as part of the URL allows for request forgery attacks.
+ * @problem.severity error
+ * @precision medium
+ * @tags security
+ *       external/cwe/cwe-918
+ */
+
+import javascript
+import SSRF
+import DataFlow::PathGraph
+
+from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink, DataFlow::Node request
+where
+  cfg.hasFlowPath(source, sink) and request = sink.getNode().(RequestForgery::Sink).getARequest()
+select sink, source, sink, "The URL of this request depends on a user-provided value"

javascript/ql/src/experimental/Security/CWE-918/SSRF.qll

@@ -0,0 +1,154 @@
+import javascript
+import semmle.javascript.security.dataflow.RequestForgeryCustomizations
+import semmle.javascript.security.dataflow.UrlConcatenation
+
+class Configuration extends TaintTracking::Configuration {
+  Configuration() { this = "SSRF" }
+
+  override predicate isSource(DataFlow::Node source) { source instanceof RequestForgery::Source }
+
+  override predicate isSink(DataFlow::Node sink) { sink instanceof RequestForgery::Sink }
+
+  override predicate isSanitizer(DataFlow::Node node) {
+    super.isSanitizer(node) or
+    node instanceof RequestForgery::Sanitizer
+  }
+
+  private predicate hasSanitizingSubstring(DataFlow::Node nd) {
+    nd.getStringValue().regexpMatch(".*[?#].*")
+    or
+    hasSanitizingSubstring(StringConcatenation::getAnOperand(nd))
+    or
+    hasSanitizingSubstring(nd.getAPredecessor())
+  }
+
+  private predicate strictSanitizingPrefixEdge(DataFlow::Node source, DataFlow::Node sink) {
+    exists(DataFlow::Node operator, int n |
+      StringConcatenation::taintStep(source, sink, operator, n) and
+      hasSanitizingSubstring(StringConcatenation::getOperand(operator, [0 .. n - 1]))
+    )
+  }
+
+  override predicate isSanitizerEdge(DataFlow::Node source, DataFlow::Node sink) {
+    strictSanitizingPrefixEdge(source, sink)
+  }
+
+  override predicate isSanitizerGuard(TaintTracking::SanitizerGuardNode nd) {
+    nd instanceof IntegerCheck or
+    nd instanceof ValidatorCheck or
+    nd instanceof TernaryOperatorSanitizerGuard
+  }
+}
+
+/**
+ * This sanitizers models the next example:
+ * let valid = req.params.id ? Number.isInteger(req.params.id) : false
+ * if (valid) { sink(req.params.id) }
+ *
+ * This sanitizer models this way of using ternary operators,
+ * when the sanitizer guard is used as any of the branches
+ * instead of being used as the condition.
+ *
+ * This sanitizer sanitize the corresponding if statement branch.
+ */
+class TernaryOperatorSanitizer extends RequestForgery::Sanitizer {
+  TernaryOperatorSanitizer() {
+    exists(
+      TaintTracking::SanitizerGuardNode guard, IfStmt ifStmt, DataFlow::Node taintedInput,
+      boolean outcome, Stmt r, DataFlow::Node falseNode
+    |
+      ifStmt.getCondition().flow().getAPredecessor+() = guard and
+      ifStmt.getCondition().flow().getAPredecessor+() = falseNode and
+      falseNode.asExpr().(BooleanLiteral).mayHaveBooleanValue(false) and
+      not ifStmt.getCondition() instanceof LogicalBinaryExpr and
+      guard.sanitizes(outcome, taintedInput.asExpr()) and
+      (
+        outcome = true and r = ifStmt.getThen() and not ifStmt.getCondition() instanceof LogNotExpr
+        or
+        outcome = false and r = ifStmt.getElse() and not ifStmt.getCondition() instanceof LogNotExpr
+        or
+        outcome = false and r = ifStmt.getThen() and ifStmt.getCondition() instanceof LogNotExpr
+        or
+        outcome = true and r = ifStmt.getElse() and ifStmt.getCondition() instanceof LogNotExpr
+      ) and
+      r.getFirstControlFlowNode()
+          .getBasicBlock()
+          .(ReachableBasicBlock)
+          .dominates(this.getBasicBlock())
+    )
+  }
+}
+
+/**
+ * This sanitizer guard is another way of modeling the example from above
+ * In this case:
+ * let valid = req.params.id ? Number.isInteger(req.params.id) : false
+ * if (!valid) { return }
+ * sink(req.params.id)
+ *
+ * The previous sanitizer is not enough,
+ * because we are sanitizing the entire if statement branch
+ * but we need to sanitize the use of this variable from now on.
+ *
+ * Thats why we model this sanitizer guard which says that
+ * the result of the ternary operator execution is a sanitizer guard.
+ */
+class TernaryOperatorSanitizerGuard extends TaintTracking::SanitizerGuardNode {
+  TaintTracking::SanitizerGuardNode originalGuard;
+
+  TernaryOperatorSanitizerGuard() {
+    this.getAPredecessor+().asExpr().(BooleanLiteral).mayHaveBooleanValue(false) and
+    this.getAPredecessor+() = originalGuard and
+    not this.asExpr() instanceof LogicalBinaryExpr
+  }
+
+  override predicate sanitizes(boolean outcome, Expr e) {
+    not this.asExpr() instanceof LogNotExpr and
+    originalGuard.sanitizes(outcome, e)
+    or
+    exists(boolean originalOutcome |
+      this.asExpr() instanceof LogNotExpr and
+      originalGuard.sanitizes(originalOutcome, e) and
+      (
+        originalOutcome = true and outcome = false
+        or
+        originalOutcome = false and outcome = true
+      )
+    )
+  }
+}
+
+/**
+ * Number.isInteger is a sanitizer guard because a number can't be used to exploit a SSRF.
+ */
+class IntegerCheck extends TaintTracking::SanitizerGuardNode, DataFlow::CallNode {
+  IntegerCheck() { this = DataFlow::globalVarRef("Number").getAMemberCall("isInteger") }
+
+  override predicate sanitizes(boolean outcome, Expr e) {
+    outcome = true and
+    e = getArgument(0).asExpr()
+  }
+}
+
+/**
+ * ValidatorCheck identifies if exists a call to validator's library methods.
+ * validator is a library which has a variety of input-validation functions. We are interesed in
+ * checking that source is a number (any type of number) or an alphanumeric value.
+ */
+class ValidatorCheck extends TaintTracking::SanitizerGuardNode, DataFlow::CallNode {
+  ValidatorCheck() {
+    exists(DataFlow::SourceNode mod, string method |
+      mod = DataFlow::moduleImport("validator") and
+      this = mod.getAChainedMethodCall(method) and
+      method in [
+          "isAlphanumeric", "isAlpha", "isDecimal", "isFloat", "isHexadecimal", "isHexColor",
+          "isInt", "isNumeric", "isOctal", "isUUID"
+        ]
+    )
+  }
+
+  override predicate sanitizes(boolean outcome, Expr e) {
+    outcome = true and
+    e = getArgument(0).asExpr()
+  }
+}

javascript/ql/src/experimental/Security/CWE-918/SSRFGood.js

@@ -0,0 +1,20 @@
+const axios = require('axios');
+const validator = require('validator');
+
+export const handler = async (req, res, next) => {
+  const { target } = req.body;
+
+  if (!validator.isAlphanumeric(target)) {
+    return next(new Error('Bad request'));
+  }
+
+  try {
+    // `target` is validated
+    const response = await axios.get('https://example.com/current_api/' + target);
+
+    // process request response
+    use(response);
+  } catch (err) {
+    // process error
+  }
+};

javascript/ql/test/experimental/Security/CWE-918/SSRF.expected

@@ -0,0 +1,185 @@
+nodes
+| check-domain.js:16:9:16:27 | url |
+| check-domain.js:16:15:16:27 | req.query.url |
+| check-domain.js:16:15:16:27 | req.query.url |
+| check-domain.js:17:13:17:15 | url |
+| check-domain.js:17:13:17:15 | url |
+| check-domain.js:26:15:26:27 | req.query.url |
+| check-domain.js:26:15:26:27 | req.query.url |
+| check-domain.js:26:15:26:27 | req.query.url |
+| check-middleware.js:9:13:9:43 | "test.c ... tainted |
+| check-middleware.js:9:13:9:43 | "test.c ... tainted |
+| check-middleware.js:9:27:9:43 | req.query.tainted |
+| check-middleware.js:9:27:9:43 | req.query.tainted |
+| check-path.js:19:13:19:43 | 'test.c ... tainted |
+| check-path.js:19:13:19:43 | 'test.c ... tainted |
+| check-path.js:19:27:19:43 | req.query.tainted |
+| check-path.js:19:27:19:43 | req.query.tainted |
+| check-path.js:22:13:22:63 | 'test.c ... ainted) |
+| check-path.js:22:13:22:63 | 'test.c ... ainted) |
+| check-path.js:22:27:22:63 | encodeU ... ainted) |
+| check-path.js:22:46:22:62 | req.query.tainted |
+| check-path.js:22:46:22:62 | req.query.tainted |
+| check-path.js:23:13:23:45 | `/addre ... inted}` |
+| check-path.js:23:13:23:45 | `/addre ... inted}` |
+| check-path.js:23:27:23:43 | req.query.tainted |
+| check-path.js:23:27:23:43 | req.query.tainted |
+| check-path.js:24:13:24:65 | `/addre ... nted)}` |
+| check-path.js:24:13:24:65 | `/addre ... nted)}` |
+| check-path.js:24:27:24:63 | encodeU ... ainted) |
+| check-path.js:24:46:24:62 | req.query.tainted |
+| check-path.js:24:46:24:62 | req.query.tainted |
+| check-path.js:33:15:33:45 | 'test.c ... tainted |
+| check-path.js:33:15:33:45 | 'test.c ... tainted |
+| check-path.js:33:29:33:45 | req.query.tainted |
+| check-path.js:33:29:33:45 | req.query.tainted |
+| check-path.js:37:15:37:45 | 'test.c ... tainted |
+| check-path.js:37:15:37:45 | 'test.c ... tainted |
+| check-path.js:37:29:37:45 | req.query.tainted |
+| check-path.js:37:29:37:45 | req.query.tainted |
+| check-path.js:45:13:45:44 | `${base ... inted}` |
+| check-path.js:45:13:45:44 | `${base ... inted}` |
+| check-path.js:45:26:45:42 | req.query.tainted |
+| check-path.js:45:26:45:42 | req.query.tainted |
+| check-regex.js:24:15:24:42 | baseURL ... tainted |
+| check-regex.js:24:15:24:42 | baseURL ... tainted |
+| check-regex.js:24:25:24:42 | req.params.tainted |
+| check-regex.js:24:25:24:42 | req.params.tainted |
+| check-regex.js:31:15:31:45 | "test.c ... tainted |
+| check-regex.js:31:15:31:45 | "test.c ... tainted |
+| check-regex.js:31:29:31:45 | req.query.tainted |
+| check-regex.js:31:29:31:45 | req.query.tainted |
+| check-regex.js:34:15:34:42 | baseURL ... tainted |
+| check-regex.js:34:15:34:42 | baseURL ... tainted |
+| check-regex.js:34:25:34:42 | req.params.tainted |
+| check-regex.js:34:25:34:42 | req.params.tainted |
+| check-regex.js:41:13:41:43 | "test.c ... tainted |
+| check-regex.js:41:13:41:43 | "test.c ... tainted |
+| check-regex.js:41:27:41:43 | req.query.tainted |
+| check-regex.js:41:27:41:43 | req.query.tainted |
+| check-validator.js:15:15:15:45 | "test.c ... tainted |
+| check-validator.js:15:15:15:45 | "test.c ... tainted |
+| check-validator.js:15:29:15:45 | req.query.tainted |
+| check-validator.js:15:29:15:45 | req.query.tainted |
+| check-validator.js:27:15:27:45 | "test.c ... tainted |
+| check-validator.js:27:15:27:45 | "test.c ... tainted |
+| check-validator.js:27:29:27:45 | req.query.tainted |
+| check-validator.js:27:29:27:45 | req.query.tainted |
+| check-validator.js:50:15:50:45 | "test.c ... tainted |
+| check-validator.js:50:15:50:45 | "test.c ... tainted |
+| check-validator.js:50:29:50:45 | req.query.tainted |
+| check-validator.js:50:29:50:45 | req.query.tainted |
+| check-validator.js:54:9:54:37 | numberURL |
+| check-validator.js:54:21:54:37 | req.query.tainted |
+| check-validator.js:54:21:54:37 | req.query.tainted |
+| check-validator.js:59:15:59:45 | "test.c ... tainted |
+| check-validator.js:59:15:59:45 | "test.c ... tainted |
+| check-validator.js:59:29:59:45 | req.query.tainted |
+| check-validator.js:59:29:59:45 | req.query.tainted |
+| check-validator.js:62:15:62:37 | "test.c ... mberURL |
+| check-validator.js:62:15:62:37 | "test.c ... mberURL |
+| check-validator.js:62:29:62:37 | numberURL |
+| check-validator.js:68:15:68:45 | "test.c ... tainted |
+| check-validator.js:68:15:68:45 | "test.c ... tainted |
+| check-validator.js:68:29:68:45 | req.query.tainted |
+| check-validator.js:68:29:68:45 | req.query.tainted |
+edges
+| check-domain.js:16:9:16:27 | url | check-domain.js:17:13:17:15 | url |
+| check-domain.js:16:9:16:27 | url | check-domain.js:17:13:17:15 | url |
+| check-domain.js:16:15:16:27 | req.query.url | check-domain.js:16:9:16:27 | url |
+| check-domain.js:16:15:16:27 | req.query.url | check-domain.js:16:9:16:27 | url |
+| check-domain.js:26:15:26:27 | req.query.url | check-domain.js:26:15:26:27 | req.query.url |
+| check-middleware.js:9:27:9:43 | req.query.tainted | check-middleware.js:9:13:9:43 | "test.c ... tainted |
+| check-middleware.js:9:27:9:43 | req.query.tainted | check-middleware.js:9:13:9:43 | "test.c ... tainted |
+| check-middleware.js:9:27:9:43 | req.query.tainted | check-middleware.js:9:13:9:43 | "test.c ... tainted |
+| check-middleware.js:9:27:9:43 | req.query.tainted | check-middleware.js:9:13:9:43 | "test.c ... tainted |
+| check-path.js:19:27:19:43 | req.query.tainted | check-path.js:19:13:19:43 | 'test.c ... tainted |
+| check-path.js:19:27:19:43 | req.query.tainted | check-path.js:19:13:19:43 | 'test.c ... tainted |
+| check-path.js:19:27:19:43 | req.query.tainted | check-path.js:19:13:19:43 | 'test.c ... tainted |
+| check-path.js:19:27:19:43 | req.query.tainted | check-path.js:19:13:19:43 | 'test.c ... tainted |
+| check-path.js:22:27:22:63 | encodeU ... ainted) | check-path.js:22:13:22:63 | 'test.c ... ainted) |
+| check-path.js:22:27:22:63 | encodeU ... ainted) | check-path.js:22:13:22:63 | 'test.c ... ainted) |
+| check-path.js:22:46:22:62 | req.query.tainted | check-path.js:22:27:22:63 | encodeU ... ainted) |
+| check-path.js:22:46:22:62 | req.query.tainted | check-path.js:22:27:22:63 | encodeU ... ainted) |
+| check-path.js:23:27:23:43 | req.query.tainted | check-path.js:23:13:23:45 | `/addre ... inted}` |
+| check-path.js:23:27:23:43 | req.query.tainted | check-path.js:23:13:23:45 | `/addre ... inted}` |
+| check-path.js:23:27:23:43 | req.query.tainted | check-path.js:23:13:23:45 | `/addre ... inted}` |
+| check-path.js:23:27:23:43 | req.query.tainted | check-path.js:23:13:23:45 | `/addre ... inted}` |
+| check-path.js:24:27:24:63 | encodeU ... ainted) | check-path.js:24:13:24:65 | `/addre ... nted)}` |
+| check-path.js:24:27:24:63 | encodeU ... ainted) | check-path.js:24:13:24:65 | `/addre ... nted)}` |
+| check-path.js:24:46:24:62 | req.query.tainted | check-path.js:24:27:24:63 | encodeU ... ainted) |
+| check-path.js:24:46:24:62 | req.query.tainted | check-path.js:24:27:24:63 | encodeU ... ainted) |
+| check-path.js:33:29:33:45 | req.query.tainted | check-path.js:33:15:33:45 | 'test.c ... tainted |
+| check-path.js:33:29:33:45 | req.query.tainted | check-path.js:33:15:33:45 | 'test.c ... tainted |
+| check-path.js:33:29:33:45 | req.query.tainted | check-path.js:33:15:33:45 | 'test.c ... tainted |
+| check-path.js:33:29:33:45 | req.query.tainted | check-path.js:33:15:33:45 | 'test.c ... tainted |
+| check-path.js:37:29:37:45 | req.query.tainted | check-path.js:37:15:37:45 | 'test.c ... tainted |
+| check-path.js:37:29:37:45 | req.query.tainted | check-path.js:37:15:37:45 | 'test.c ... tainted |
+| check-path.js:37:29:37:45 | req.query.tainted | check-path.js:37:15:37:45 | 'test.c ... tainted |
+| check-path.js:37:29:37:45 | req.query.tainted | check-path.js:37:15:37:45 | 'test.c ... tainted |
+| check-path.js:45:26:45:42 | req.query.tainted | check-path.js:45:13:45:44 | `${base ... inted}` |
+| check-path.js:45:26:45:42 | req.query.tainted | check-path.js:45:13:45:44 | `${base ... inted}` |
+| check-path.js:45:26:45:42 | req.query.tainted | check-path.js:45:13:45:44 | `${base ... inted}` |
+| check-path.js:45:26:45:42 | req.query.tainted | check-path.js:45:13:45:44 | `${base ... inted}` |
+| check-regex.js:24:25:24:42 | req.params.tainted | check-regex.js:24:15:24:42 | baseURL ... tainted |
+| check-regex.js:24:25:24:42 | req.params.tainted | check-regex.js:24:15:24:42 | baseURL ... tainted |
+| check-regex.js:24:25:24:42 | req.params.tainted | check-regex.js:24:15:24:42 | baseURL ... tainted |
+| check-regex.js:24:25:24:42 | req.params.tainted | check-regex.js:24:15:24:42 | baseURL ... tainted |
+| check-regex.js:31:29:31:45 | req.query.tainted | check-regex.js:31:15:31:45 | "test.c ... tainted |
+| check-regex.js:31:29:31:45 | req.query.tainted | check-regex.js:31:15:31:45 | "test.c ... tainted |
+| check-regex.js:31:29:31:45 | req.query.tainted | check-regex.js:31:15:31:45 | "test.c ... tainted |
+| check-regex.js:31:29:31:45 | req.query.tainted | check-regex.js:31:15:31:45 | "test.c ... tainted |
+| check-regex.js:34:25:34:42 | req.params.tainted | check-regex.js:34:15:34:42 | baseURL ... tainted |
+| check-regex.js:34:25:34:42 | req.params.tainted | check-regex.js:34:15:34:42 | baseURL ... tainted |
+| check-regex.js:34:25:34:42 | req.params.tainted | check-regex.js:34:15:34:42 | baseURL ... tainted |
+| check-regex.js:34:25:34:42 | req.params.tainted | check-regex.js:34:15:34:42 | baseURL ... tainted |
+| check-regex.js:41:27:41:43 | req.query.tainted | check-regex.js:41:13:41:43 | "test.c ... tainted |
+| check-regex.js:41:27:41:43 | req.query.tainted | check-regex.js:41:13:41:43 | "test.c ... tainted |
+| check-regex.js:41:27:41:43 | req.query.tainted | check-regex.js:41:13:41:43 | "test.c ... tainted |
+| check-regex.js:41:27:41:43 | req.query.tainted | check-regex.js:41:13:41:43 | "test.c ... tainted |
+| check-validator.js:15:29:15:45 | req.query.tainted | check-validator.js:15:15:15:45 | "test.c ... tainted |
+| check-validator.js:15:29:15:45 | req.query.tainted | check-validator.js:15:15:15:45 | "test.c ... tainted |
+| check-validator.js:15:29:15:45 | req.query.tainted | check-validator.js:15:15:15:45 | "test.c ... tainted |
+| check-validator.js:15:29:15:45 | req.query.tainted | check-validator.js:15:15:15:45 | "test.c ... tainted |
+| check-validator.js:27:29:27:45 | req.query.tainted | check-validator.js:27:15:27:45 | "test.c ... tainted |
+| check-validator.js:27:29:27:45 | req.query.tainted | check-validator.js:27:15:27:45 | "test.c ... tainted |
+| check-validator.js:27:29:27:45 | req.query.tainted | check-validator.js:27:15:27:45 | "test.c ... tainted |
+| check-validator.js:27:29:27:45 | req.query.tainted | check-validator.js:27:15:27:45 | "test.c ... tainted |
+| check-validator.js:50:29:50:45 | req.query.tainted | check-validator.js:50:15:50:45 | "test.c ... tainted |
+| check-validator.js:50:29:50:45 | req.query.tainted | check-validator.js:50:15:50:45 | "test.c ... tainted |
+| check-validator.js:50:29:50:45 | req.query.tainted | check-validator.js:50:15:50:45 | "test.c ... tainted |
+| check-validator.js:50:29:50:45 | req.query.tainted | check-validator.js:50:15:50:45 | "test.c ... tainted |
+| check-validator.js:54:9:54:37 | numberURL | check-validator.js:62:29:62:37 | numberURL |
+| check-validator.js:54:21:54:37 | req.query.tainted | check-validator.js:54:9:54:37 | numberURL |
+| check-validator.js:54:21:54:37 | req.query.tainted | check-validator.js:54:9:54:37 | numberURL |
+| check-validator.js:59:29:59:45 | req.query.tainted | check-validator.js:59:15:59:45 | "test.c ... tainted |
+| check-validator.js:59:29:59:45 | req.query.tainted | check-validator.js:59:15:59:45 | "test.c ... tainted |
+| check-validator.js:59:29:59:45 | req.query.tainted | check-validator.js:59:15:59:45 | "test.c ... tainted |
+| check-validator.js:59:29:59:45 | req.query.tainted | check-validator.js:59:15:59:45 | "test.c ... tainted |
+| check-validator.js:62:29:62:37 | numberURL | check-validator.js:62:15:62:37 | "test.c ... mberURL |
+| check-validator.js:62:29:62:37 | numberURL | check-validator.js:62:15:62:37 | "test.c ... mberURL |
+| check-validator.js:68:29:68:45 | req.query.tainted | check-validator.js:68:15:68:45 | "test.c ... tainted |
+| check-validator.js:68:29:68:45 | req.query.tainted | check-validator.js:68:15:68:45 | "test.c ... tainted |
+| check-validator.js:68:29:68:45 | req.query.tainted | check-validator.js:68:15:68:45 | "test.c ... tainted |
+| check-validator.js:68:29:68:45 | req.query.tainted | check-validator.js:68:15:68:45 | "test.c ... tainted |
+#select
+| check-domain.js:17:13:17:15 | url | check-domain.js:16:15:16:27 | req.query.url | check-domain.js:17:13:17:15 | url | The URL of this request depends on a user-provided value |
+| check-domain.js:26:15:26:27 | req.query.url | check-domain.js:26:15:26:27 | req.query.url | check-domain.js:26:15:26:27 | req.query.url | The URL of this request depends on a user-provided value |
+| check-middleware.js:9:13:9:43 | "test.c ... tainted | check-middleware.js:9:27:9:43 | req.query.tainted | check-middleware.js:9:13:9:43 | "test.c ... tainted | The URL of this request depends on a user-provided value |
+| check-path.js:19:13:19:43 | 'test.c ... tainted | check-path.js:19:27:19:43 | req.query.tainted | check-path.js:19:13:19:43 | 'test.c ... tainted | The URL of this request depends on a user-provided value |
+| check-path.js:22:13:22:63 | 'test.c ... ainted) | check-path.js:22:46:22:62 | req.query.tainted | check-path.js:22:13:22:63 | 'test.c ... ainted) | The URL of this request depends on a user-provided value |
+| check-path.js:23:13:23:45 | `/addre ... inted}` | check-path.js:23:27:23:43 | req.query.tainted | check-path.js:23:13:23:45 | `/addre ... inted}` | The URL of this request depends on a user-provided value |
+| check-path.js:24:13:24:65 | `/addre ... nted)}` | check-path.js:24:46:24:62 | req.query.tainted | check-path.js:24:13:24:65 | `/addre ... nted)}` | The URL of this request depends on a user-provided value |
+| check-path.js:33:15:33:45 | 'test.c ... tainted | check-path.js:33:29:33:45 | req.query.tainted | check-path.js:33:15:33:45 | 'test.c ... tainted | The URL of this request depends on a user-provided value |
+| check-path.js:37:15:37:45 | 'test.c ... tainted | check-path.js:37:29:37:45 | req.query.tainted | check-path.js:37:15:37:45 | 'test.c ... tainted | The URL of this request depends on a user-provided value |
+| check-path.js:45:13:45:44 | `${base ... inted}` | check-path.js:45:26:45:42 | req.query.tainted | check-path.js:45:13:45:44 | `${base ... inted}` | The URL of this request depends on a user-provided value |
+| check-regex.js:24:15:24:42 | baseURL ... tainted | check-regex.js:24:25:24:42 | req.params.tainted | check-regex.js:24:15:24:42 | baseURL ... tainted | The URL of this request depends on a user-provided value |
+| check-regex.js:31:15:31:45 | "test.c ... tainted | check-regex.js:31:29:31:45 | req.query.tainted | check-regex.js:31:15:31:45 | "test.c ... tainted | The URL of this request depends on a user-provided value |
+| check-regex.js:34:15:34:42 | baseURL ... tainted | check-regex.js:34:25:34:42 | req.params.tainted | check-regex.js:34:15:34:42 | baseURL ... tainted | The URL of this request depends on a user-provided value |
+| check-regex.js:41:13:41:43 | "test.c ... tainted | check-regex.js:41:27:41:43 | req.query.tainted | check-regex.js:41:13:41:43 | "test.c ... tainted | The URL of this request depends on a user-provided value |
+| check-validator.js:15:15:15:45 | "test.c ... tainted | check-validator.js:15:29:15:45 | req.query.tainted | check-validator.js:15:15:15:45 | "test.c ... tainted | The URL of this request depends on a user-provided value |
+| check-validator.js:27:15:27:45 | "test.c ... tainted | check-validator.js:27:29:27:45 | req.query.tainted | check-validator.js:27:15:27:45 | "test.c ... tainted | The URL of this request depends on a user-provided value |
+| check-validator.js:50:15:50:45 | "test.c ... tainted | check-validator.js:50:29:50:45 | req.query.tainted | check-validator.js:50:15:50:45 | "test.c ... tainted | The URL of this request depends on a user-provided value |
+| check-validator.js:59:15:59:45 | "test.c ... tainted | check-validator.js:59:29:59:45 | req.query.tainted | check-validator.js:59:15:59:45 | "test.c ... tainted | The URL of this request depends on a user-provided value |
+| check-validator.js:62:15:62:37 | "test.c ... mberURL | check-validator.js:54:21:54:37 | req.query.tainted | check-validator.js:62:15:62:37 | "test.c ... mberURL | The URL of this request depends on a user-provided value |
+| check-validator.js:68:15:68:45 | "test.c ... tainted | check-validator.js:68:29:68:45 | req.query.tainted | check-validator.js:68:15:68:45 | "test.c ... tainted | The URL of this request depends on a user-provided value |

javascript/ql/test/experimental/Security/CWE-918/SSRF.qlref

@@ -0,0 +1 @@
+./experimental/Security/CWE-918/SSRF.ql

javascript/ql/test/experimental/Security/CWE-918/check-domain.js

@@ -0,0 +1,34 @@
+// native modules
+const url = require('url');
+
+// dependencies
+const axios = require('axios');
+const express = require('express');
+
+// constants
+const VALID_DOMAINS = ['example.com', 'example-2.com'];
+
+// start
+const app = express();
+
+app.get('/check-with-axios', req => {
+  // without validation
+  const url = req.query.url;
+  axios.get(url); //SSRF
+
+  // validating domain only
+  const decodedURI = decodeURIComponent(req.query.url);
+  const { hostname } = url.parse(decodedURI);
+
+  const { hostname } = url.parse(decodedURI);
+
+  if (isValidDomain(hostname, validDomains)) {
+    axios.get(req.query.url); //SSRF
+  }
+});
+
+const isValidDomain = (hostname, validDomains) => (
+  validDomains.some(domain => (
+    hostname === domain || hostname.endsWith(`.${domain}`))
+  )
+);

javascript/ql/test/experimental/Security/CWE-918/check-middleware.js

@@ -0,0 +1,19 @@
+// dependencies
+const axios = require('axios');
+const express = require('express');
+
+// start
+const app = express();
+
+app.get('/check-with-axios', validationMiddleware, req => {
+  axios.get("test.com/" + req.query.tainted); // OK is sanitized by the middleware - False Positive
+});
+
+
+const validationMiddleware = (req, res, next) => {
+  if (!Number.isInteger(req.query.tainted)) {
+    return res.sendStatus(400);
+  }
+
+  next();
+}

javascript/ql/test/experimental/Security/CWE-918/check-path.js

@@ -0,0 +1,57 @@
+// native modules
+const path = require('path');
+const url = require('url');
+
+// dependencies
+const axios = require('axios');
+const express = require('express');
+
+// constants
+const VALID_PATHS = ['/api/users/me', '/help', '/system/health'];
+
+// start
+const app = express();
+
+app.get('/check-with-axios', req => {
+  const hardcoded = 'hardcodeado';
+
+  axios.get('test.com/' + hardcoded); // OK
+  axios.get('test.com/' + req.query.tainted); // SSRF
+  axios.get('test.com/' + Number(req.query.tainted)); // OK
+  axios.get('test.com/' + req.user.id); // OK
+  axios.get('test.com/' + encodeURIComponent(req.query.tainted)); // SSRF
+  axios.get(`/addresses/${req.query.tainted}`); // SSRF
+  axios.get(`/addresses/${encodeURIComponent(req.query.tainted)}`); // SSRF
+  
+  if (Number.isInteger(req.query.tainted)) {
+    axios.get('test.com/' + req.query.tainted); // OK
+  }
+
+  if (isValidInput(req.query.tainted)){
+    axios.get('test.com/' + req.query.tainted); // OK
+  } else {
+    axios.get('test.com/' + req.query.tainted); // SSRF
+  }
+
+  if (doesntCheckAnything(req.query.tainted)) {
+    axios.get('test.com/' + req.query.tainted); // SSRF
+  }
+
+  if (isValidPath(req.query.tainted, VALID_PATHS)) {
+    axios.get('test.com/' + req.query.tainted) // OK
+  }
+
+  let baseURL = require('config').base
+  axios.get(`${baseURL}${req.query.tainted}`); // SSRF
+
+  if(!isValidInput(req.query.tainted)) {
+    return;
+  }
+  axios.get("test.com/" + req.query.tainted); // OK
+});
+
+const isValidPath = (path, validPaths) => validPaths.includes(path);
+
+const isValidInput = (path) => Number.isInteger(path);
+
+const doesntCheckAnything = (path) => true;

javascript/ql/test/experimental/Security/CWE-918/check-regex.js

@@ -0,0 +1,46 @@
+// dependencies
+const axios = require('axios');
+const express = require('express');
+
+// start
+const app = express();
+
+app.get('/check-with-axios', req => {
+  if (req.query.tainted.match(/^[0-9a-z]+$/)) { // letters and numbers
+    axios.get("test.com/" + req.query.tainted); // OK
+  }
+  if (req.query.tainted.match(/^[0-9a-z\-_]+$/)) { // letters, numbers, - and _
+    axios.get("test.com/" + req.query.tainted); // OK
+  }
+  if (req.query.tainted.match(/^.*$/)) { // anything
+    axios.get("test.com/" + req.query.tainted); // SSRF - False Negative
+  }
+
+  const baseURL = "test.com/"
+  if (isValidPath(req.params.tainted) ) {
+    axios.get(baseURL + req.params.tainted); // OK
+  }
+  if (!isValidPath(req.params.tainted) ) {
+    axios.get(baseURL + req.params.tainted); // SSRF
+  } else {
+    axios.get(baseURL + req.params.tainted); // OK
+  }
+  
+  // Blacklists are not safe
+  if (!req.query.tainted.match(/^[/\.%]+$/)) {
+    axios.get("test.com/" + req.query.tainted); // SSRF
+  }
+  if (!isInBlacklist(req.params.tainted) ) {
+    axios.get(baseURL + req.params.tainted); // SSRF
+  }
+
+  if (!isValidPath(req.params.tainted)) {
+    return;
+  }
+
+  axios.get("test.com/" + req.query.tainted); // OK - False Positive
+});
+
+const isValidPath = path =>  path.match(/^[0-9a-z]+$/);
+
+const isInBlackList = path =>  path.match(/^[/\.%]+$/);

javascript/ql/test/experimental/Security/CWE-918/check-validator.js

@@ -0,0 +1,98 @@
+// dependencies
+const axios = require('axios');
+const express = require('express');
+const validator = require('validator');
+
+// start
+const app = express();
+
+app.get("/check-with-axios", req => {
+  // alphanumeric
+  if (validator.isAlphanumeric(req.query.tainted)) {
+    axios.get("test.com/" + req.query.tainted); // OK
+  }
+  if (isAlphanumeric(req.query.tainted)) {
+    axios.get("test.com/" + req.query.tainted); // SSRF
+  }
+  if (validAlphanumeric(req.query.tainted)) {
+    axios.get("test.com/" + req.query.tainted); // OK
+  }
+  if (validAlpha(req.query.tainted)) {
+    axios.get("test.com/" + req.query.tainted); // OK
+  }
+  if (validNumber(req.query.tainted)) {
+    axios.get("test.com/" + req.query.tainted); // OK
+  }
+  if (wrongValidation(req.query.tainted)) {
+    axios.get("test.com/" + req.query.tainted); // SSRF
+  }
+
+  // numbers
+  if (validHexadecimal(req.query.tainted)) {
+    axios.get("test.com/" + req.query.tainted); // OK
+  }
+  if (validHexaColor(req.query.tainted)) {
+    axios.get("test.com/" + req.query.tainted); // OK
+  }
+  if (validDecimal(req.query.tainted)) {
+    axios.get("test.com/" + req.query.tainted); // OK
+  }
+  if (validFloat(req.query.tainted)) {
+    axios.get("test.com/" + req.query.tainted); // OK
+  }
+  if (validInt(req.query.tainted)) {
+    axios.get("test.com/" + req.query.tainted); // OK
+  }
+  if (validOctal(req.query.tainted)) {
+    axios.get("test.com/" + req.query.tainted); // OK
+  }
+  if (validHexa(req.query.tainted)) {
+    axios.get("test.com/" + req.query.tainted); // OK. False Positive
+  }
+
+  // with simple assignation
+  const numberURL = req.query.tainted;
+  if (validNumber(numberURL)) {
+    axios.get("test.com/" + numberURL); // OK
+  }
+  if (validNumber(numberURL)) {
+    axios.get("test.com/" + req.query.tainted); // OK. False Positive
+  }
+  if (validNumber(req.query.tainted)) {
+    axios.get("test.com/" + numberURL); // OK. False Positive
+  }
+
+  if (validHexadecimal(req.query.tainted) || validHexaColor(req.query.tainted) || 
+      validDecimal(req.query.tainted) || validFloat(req.query.tainted) || validInt(req.query.tainted) || 
+      validNumber(req.query.tainted) || validOctal(req.query.tainted)) {
+    axios.get("test.com/" + req.query.tainted); // OK. False Positive
+  }
+});
+
+// safe validators
+const validAlphanumeric = url => validator.isAlphanumeric(url);
+
+const validAlpha = url => validator.isAlpha(url);
+
+const validDecimal = url => validator.isDecimal(url);
+
+const validFloat = url => validator.isFloat(url);
+
+const validInt = url => validator.isInt(url);
+
+const validNumber = url => validator.isNumeric(url);
+
+const validOctal = url => validator.isOctal(url);
+
+const validHexa = url => validator.isHexadecimal(url) || validator.isHexColor(url);
+
+const validHexadecimal = url => validator.isHexadecimal(url);
+
+const validHexaColor = url => validator.isHexColor(url);
+
+const validUUID = url => validator.isUUID(url);
+
+// unsafe validators
+const wrongValidation = url => validator.isByteLength(url, {min:4,max:8});
+
+const isAlphanumeric = url => true;

javascript/ql/test/experimental/Security/CWE-918/ternary-operator.js

@@ -0,0 +1,182 @@
+const express = require('express');
+const app = express();
+
+app.use(express.json());
+
+app.get('/direct-ternary-operator', function (req, res) {
+    let taintedURL = req.params.url
+    
+    let v = req.params.url ? req.params.url == "someURL" : false
+    if (v) {
+        req_frontend_restclient.get(req.params.url) // OK
+    }
+
+    let v1 = taintedURL ? taintedURL == "someURL" : false
+    if (v1) {
+        req_frontend_restclient.get(taintedURL) // OK
+    }
+
+    let v2 = taintedURL ? valid(taintedURL) : false
+    if (v2) {
+        req_frontend_restclient.get(taintedURL) // OK
+    }
+
+    let v3 = req.params.url ? valid(req.params.url) : false
+    if (v3) {
+        req_frontend_restclient.get(req.params.url) // OK
+    }
+
+    let v4 = req.params.url == undefined ? false : valid(req.params.url)
+    if (v4) {
+        req_frontend_restclient.get(req.params.url) // OK
+    }
+
+    let v5 = req.params.url == undefined ? true : valid(req.params.url)
+    if (v5) {
+        req_frontend_restclient.get(req.params.url) // SSRF
+    }
+
+    let v6 = req.params.url ? valid(req.params.url) : true
+    if (v6) {
+        req_frontend_restclient.get(req.params.url) // SSRF
+    }
+
+    let f = false
+    let v7 = req.params.url ? valid(req.params.url) : true
+    if (v7) {
+        req_frontend_restclient.get(req.params.url) // SSRF
+    }
+
+    let v8 = req.params.url == undefined ? false : valid(req.params.url)
+    if (!v8) {
+        return
+    }
+    req_frontend_restclient.get(req.params.url) // OK
+})
+
+app.get('/functions', function (req, res) {
+    let taintedURL = req.params.url
+
+    if (valid2(taintedURL)) {
+        req_frontend_restclient.get(taintedURL) // OK
+    }
+
+    if (!invalid(taintedURL)) {
+        req_frontend_restclient.get(taintedURL) // False positive
+    }
+
+    if (valid2(req.params.url)){
+        req_frontend_restclient.get(req.params.url) // OK
+    }
+
+    if (!assertAlphanumeric(req.params.url)) {
+        return
+    }
+    req_frontend_restclient.get(req.params.url); // OK
+})
+
+app.get('/normal-use-of-ternary-operator', function (req, res) {
+    let taintedURL = req.params.url
+    
+    let url =  valid(req.params.url) ? req.params.url : undefined
+    req_frontend_restclient.get(url) // OK
+
+    let url =  valid(taintedURL) ? taintedURL : undefined
+    req_frontend_restclient.get(url) // OK
+
+    let url4 = req.params.url.match(/^[\w.-]+$/) ? req.params.url : undefined
+    req_frontend_restclient.get(url4) // OK
+})
+
+app.get('/throw-errors', function (req, res) {
+    req_frontend_restclient.get(valid3(req.params.url)) // False positive
+
+    req_frontend_restclient.get(assertOther(req.params.url)); // False positive
+
+    req_frontend_restclient.get(assertOther2(req.params.url)); // False positive
+});
+
+app.get('/bad-endpoint', function (req, res) {
+    req_frontend_restclient.get(req.params.url); // SSRF
+
+    const valid = req.params.url ? req.params.url == "someURL" : false
+    if (!valid) {
+        throw new Error(`Invalid parameter: "${req.params.url}", must be alphanumeric`);
+    }
+    req_frontend_restclient.get(req.params.url); // OK
+})
+
+
+app.get('/bad-endpoint-variable', function (req, res) {
+    let taintedURL = req.params.url
+    req_frontend_restclient.get(taintedURL); // SSRF
+
+    const valid = taintedURL ? taintedURL == "someURL" : false
+    if (!valid) {
+        return
+    }
+    req_frontend_restclient.get(taintedURL); // False positive
+})
+
+app.get('/not-invalid', function (req, res) {
+    const invalidParam = req.params.url ? !Number.isInteger(req.params.url) : false
+    if (invalidParam) {
+        return
+    }
+    req_frontend_restclient.get(req.params.url); // False positive
+})
+
+
+app.get('/bad-endpoint-2', function (req, res) {
+    other(req.params.url)
+})
+
+function other(taintedURL) {
+    req_frontend_restclient.get(taintedURL); // SSRF
+
+    const valid = taintedURL ? taintedURL == "someURL" : false
+    if (!valid) {
+        return
+    }
+    req_frontend_restclient.get(taintedURL); // False positive
+}
+
+function assertAlphanumeric(value) {
+    return value ? value.match(/^[\w.-]+$/) : false;
+}
+
+function assertOther(value) {
+    const valid = value ? !!value.match(/^[\w.-]+$/) : false;
+    if (!valid) {
+      throw new Error(`Invalid parameter: "${value}", must be alphanumeric`);
+    }
+    return value;
+}
+
+function assertOther2(value) {
+    const valid = value ? value.match(/^[\w.-]+$/) : false;
+    if (!valid) {
+      throw new Error(`Invalid parameter: "${value}", must be alphanumeric`);
+    }
+    return value;
+}
+
+function invalid(value) {
+    return value ? !Number.isInteger(value) : true 
+}
+
+function valid(value) {
+    return value.match(/^[\w.-]+$/)
+}
+
+function valid2(value) {
+    return value ? value == "someURL" : false
+}
+
+function valid3(value) {
+    const valid = value ? value == "someURL" : false
+    if (!valid) {
+        throw new Error(`Invalid parameter: "${value}", must be alphanumeric`);
+    }
+    return value;
+}

javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.expected

@@ -2116,6 +2116,19 @@ nodes
 | normalizedPaths.js:381:25:381:28 | path |
 | normalizedPaths.js:381:25:381:28 | path |
 | normalizedPaths.js:381:25:381:28 | path |
+| normalizedPaths.js:385:7:385:46 | path |
+| normalizedPaths.js:385:7:385:46 | path |
+| normalizedPaths.js:385:14:385:46 | pathMod ... uery.x) |
+| normalizedPaths.js:385:14:385:46 | pathMod ... uery.x) |
+| normalizedPaths.js:385:35:385:45 | req.query.x |
+| normalizedPaths.js:385:35:385:45 | req.query.x |
+| normalizedPaths.js:385:35:385:45 | req.query.x |
+| normalizedPaths.js:388:19:388:22 | path |
+| normalizedPaths.js:388:19:388:22 | path |
+| normalizedPaths.js:388:19:388:22 | path |
+| normalizedPaths.js:399:21:399:24 | path |
+| normalizedPaths.js:399:21:399:24 | path |
+| normalizedPaths.js:399:21:399:24 | path |
 | other-fs-libraries.js:9:7:9:48 | path |
 | other-fs-libraries.js:9:7:9:48 | path |
 | other-fs-libraries.js:9:7:9:48 | path |
@@ -6998,6 +7011,20 @@ edges
 | normalizedPaths.js:381:25:381:28 | path | normalizedPaths.js:381:19:381:29 | slash(path) |
 | normalizedPaths.js:381:25:381:28 | path | normalizedPaths.js:381:19:381:29 | slash(path) |
 | normalizedPaths.js:381:25:381:28 | path | normalizedPaths.js:381:19:381:29 | slash(path) |
+| normalizedPaths.js:385:7:385:46 | path | normalizedPaths.js:388:19:388:22 | path |
+| normalizedPaths.js:385:7:385:46 | path | normalizedPaths.js:388:19:388:22 | path |
+| normalizedPaths.js:385:7:385:46 | path | normalizedPaths.js:388:19:388:22 | path |
+| normalizedPaths.js:385:7:385:46 | path | normalizedPaths.js:388:19:388:22 | path |
+| normalizedPaths.js:385:7:385:46 | path | normalizedPaths.js:399:21:399:24 | path |
+| normalizedPaths.js:385:7:385:46 | path | normalizedPaths.js:399:21:399:24 | path |
+| normalizedPaths.js:385:7:385:46 | path | normalizedPaths.js:399:21:399:24 | path |
+| normalizedPaths.js:385:7:385:46 | path | normalizedPaths.js:399:21:399:24 | path |
+| normalizedPaths.js:385:14:385:46 | pathMod ... uery.x) | normalizedPaths.js:385:7:385:46 | path |
+| normalizedPaths.js:385:14:385:46 | pathMod ... uery.x) | normalizedPaths.js:385:7:385:46 | path |
+| normalizedPaths.js:385:35:385:45 | req.query.x | normalizedPaths.js:385:14:385:46 | pathMod ... uery.x) |
+| normalizedPaths.js:385:35:385:45 | req.query.x | normalizedPaths.js:385:14:385:46 | pathMod ... uery.x) |
+| normalizedPaths.js:385:35:385:45 | req.query.x | normalizedPaths.js:385:14:385:46 | pathMod ... uery.x) |
+| normalizedPaths.js:385:35:385:45 | req.query.x | normalizedPaths.js:385:14:385:46 | pathMod ... uery.x) |
 | other-fs-libraries.js:9:7:9:48 | path | other-fs-libraries.js:11:19:11:22 | path |
 | other-fs-libraries.js:9:7:9:48 | path | other-fs-libraries.js:11:19:11:22 | path |
 | other-fs-libraries.js:9:7:9:48 | path | other-fs-libraries.js:11:19:11:22 | path |
@@ -9670,6 +9697,8 @@ edges
 | normalizedPaths.js:363:21:363:31 | requestPath | normalizedPaths.js:354:14:354:27 | req.query.path | normalizedPaths.js:363:21:363:31 | requestPath | This path depends on $@. | normalizedPaths.js:354:14:354:27 | req.query.path | a user-provided value |
 | normalizedPaths.js:379:19:379:22 | path | normalizedPaths.js:377:14:377:27 | req.query.path | normalizedPaths.js:379:19:379:22 | path | This path depends on $@. | normalizedPaths.js:377:14:377:27 | req.query.path | a user-provided value |
 | normalizedPaths.js:381:19:381:29 | slash(path) | normalizedPaths.js:377:14:377:27 | req.query.path | normalizedPaths.js:381:19:381:29 | slash(path) | This path depends on $@. | normalizedPaths.js:377:14:377:27 | req.query.path | a user-provided value |
+| normalizedPaths.js:388:19:388:22 | path | normalizedPaths.js:385:35:385:45 | req.query.x | normalizedPaths.js:388:19:388:22 | path | This path depends on $@. | normalizedPaths.js:385:35:385:45 | req.query.x | a user-provided value |
+| normalizedPaths.js:399:21:399:24 | path | normalizedPaths.js:385:35:385:45 | req.query.x | normalizedPaths.js:399:21:399:24 | path | This path depends on $@. | normalizedPaths.js:385:35:385:45 | req.query.x | a user-provided value |
 | other-fs-libraries.js:11:19:11:22 | path | other-fs-libraries.js:9:24:9:30 | req.url | other-fs-libraries.js:11:19:11:22 | path | This path depends on $@. | other-fs-libraries.js:9:24:9:30 | req.url | a user-provided value |
 | other-fs-libraries.js:12:27:12:30 | path | other-fs-libraries.js:9:24:9:30 | req.url | other-fs-libraries.js:12:27:12:30 | path | This path depends on $@. | other-fs-libraries.js:9:24:9:30 | req.url | a user-provided value |
 | other-fs-libraries.js:13:24:13:27 | path | other-fs-libraries.js:9:24:9:30 | req.url | other-fs-libraries.js:13:24:13:27 | path | This path depends on $@. | other-fs-libraries.js:9:24:9:30 | req.url | a user-provided value |

javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/normalizedPaths.js

@@ -379,4 +379,26 @@ app.get('/slash-stuff', (req, res) => {
   fs.readFileSync(path); // NOT OK
 
   fs.readFileSync(slash(path)); // NOT OK
-});
+});
+
+app.get('/dotdot-regexp', (req, res) => {
+  let path = pathModule.normalize(req.query.x);
+  if (pathModule.isAbsolute(path))
+    return;
+  fs.readFileSync(path); // NOT OK
+  if (!path.match(/\./)) {
+    fs.readFileSync(path); // OK
+  }
+  if (!path.match(/\.\./)) {
+    fs.readFileSync(path); // OK
+  }
+  if (!path.match(/\.\.\//)) {
+    fs.readFileSync(path); // OK
+  }
+  if (!path.match(/\.\.\/foo/)) {
+    fs.readFileSync(path); // NOT OK
+  }
+  if (!path.match(/(\.\.\/|\.\.\\)/)) {
+    fs.readFileSync(path); // OK
+  }
+});

javascript/ql/test/query-tests/Security/CWE-079/UnsafeJQueryPlugin/UnsafeJQueryPlugin.expected

@@ -121,6 +121,12 @@ nodes
 | unsafe-jquery-plugin.js:179:5:179:11 | options |
 | unsafe-jquery-plugin.js:179:5:179:18 | options.target |
 | unsafe-jquery-plugin.js:179:5:179:18 | options.target |
+| unsafe-jquery-plugin.js:185:28:185:34 | options |
+| unsafe-jquery-plugin.js:185:28:185:34 | options |
+| unsafe-jquery-plugin.js:186:21:186:27 | options |
+| unsafe-jquery-plugin.js:186:21:186:30 | options.of |
+| unsafe-jquery-plugin.js:192:19:192:28 | options.of |
+| unsafe-jquery-plugin.js:192:19:192:28 | options.of |
 edges
 | unsafe-jquery-plugin.js:2:38:2:44 | options | unsafe-jquery-plugin.js:3:5:3:11 | options |
 | unsafe-jquery-plugin.js:2:38:2:44 | options | unsafe-jquery-plugin.js:3:5:3:11 | options |
@@ -245,6 +251,11 @@ edges
 | unsafe-jquery-plugin.js:178:27:178:33 | options | unsafe-jquery-plugin.js:179:5:179:11 | options |
 | unsafe-jquery-plugin.js:179:5:179:11 | options | unsafe-jquery-plugin.js:179:5:179:18 | options.target |
 | unsafe-jquery-plugin.js:179:5:179:11 | options | unsafe-jquery-plugin.js:179:5:179:18 | options.target |
+| unsafe-jquery-plugin.js:185:28:185:34 | options | unsafe-jquery-plugin.js:186:21:186:27 | options |
+| unsafe-jquery-plugin.js:185:28:185:34 | options | unsafe-jquery-plugin.js:186:21:186:27 | options |
+| unsafe-jquery-plugin.js:186:21:186:27 | options | unsafe-jquery-plugin.js:186:21:186:30 | options.of |
+| unsafe-jquery-plugin.js:186:21:186:30 | options.of | unsafe-jquery-plugin.js:192:19:192:28 | options.of |
+| unsafe-jquery-plugin.js:186:21:186:30 | options.of | unsafe-jquery-plugin.js:192:19:192:28 | options.of |
 #select
 | unsafe-jquery-plugin.js:3:5:3:11 | options | unsafe-jquery-plugin.js:2:38:2:44 | options | unsafe-jquery-plugin.js:3:5:3:11 | options | Potential XSS vulnerability in the $@. | unsafe-jquery-plugin.js:2:19:63:2 | functio ... \\t\\t}\\n\\n\\t} | '$.fn.my_plugin' plugin |
 | unsafe-jquery-plugin.js:5:5:5:18 | options.target | unsafe-jquery-plugin.js:2:38:2:44 | options | unsafe-jquery-plugin.js:5:5:5:18 | options.target | Potential XSS vulnerability in the $@. | unsafe-jquery-plugin.js:2:19:63:2 | functio ... \\t\\t}\\n\\n\\t} | '$.fn.my_plugin' plugin |
@@ -268,3 +279,4 @@ edges
 | unsafe-jquery-plugin.js:157:44:157:59 | options.target.a | unsafe-jquery-plugin.js:153:38:153:44 | options | unsafe-jquery-plugin.js:157:44:157:59 | options.target.a | Potential XSS vulnerability in the $@. | unsafe-jquery-plugin.js:153:19:158:2 | functio ... NCY]\\n\\t} | '$.fn.my_plugin' plugin |
 | unsafe-jquery-plugin.js:170:6:170:11 | target | unsafe-jquery-plugin.js:160:38:160:44 | options | unsafe-jquery-plugin.js:170:6:170:11 | target | Potential XSS vulnerability in the $@. | unsafe-jquery-plugin.js:160:19:173:2 | functio ... \\t\\t}\\n\\n\\t} | '$.fn.my_plugin' plugin |
 | unsafe-jquery-plugin.js:179:5:179:18 | options.target | unsafe-jquery-plugin.js:178:27:178:33 | options | unsafe-jquery-plugin.js:179:5:179:18 | options.target | Potential XSS vulnerability in the $@. | unsafe-jquery-plugin.js:178:18:180:2 | functio ... T OK\\n\\t} | '$.fn.my_plugin' plugin |
+| unsafe-jquery-plugin.js:192:19:192:28 | options.of | unsafe-jquery-plugin.js:185:28:185:34 | options | unsafe-jquery-plugin.js:192:19:192:28 | options.of | Potential XSS vulnerability in the $@. | unsafe-jquery-plugin.js:185:18:194:2 | functio ... et);\\n\\t} | '$.fn.position' plugin |

javascript/ql/test/query-tests/Security/CWE-079/UnsafeJQueryPlugin/unsafe-jquery-plugin.js

@@ -182,4 +182,14 @@
 		$(document).find(options.target); // OK
 	}});
 
+	$.fn.position = function( options ) {
+		if ( !options || !options.of ) {
+			return doSomethingElse( this, arguments );
+		}
+		// extending options
+		options = $.extend( {}, options );
+	
+		var target = $( options.of ); // NOT OK
+		console.log(target);
+	};
 });

javascript/ql/test/query-tests/Security/CWE-326/InsufficientKeySize.expected

@@ -0,0 +1,11 @@
+| tst.js:3:14:3:71 | crypto. ... 1024 }) | Creation of an asymmetric RSA key uses 1024 bits, which is below 2048 and considered breakable. |
+| tst.js:7:14:7:59 | crypto. ... : 64 }) | Creation of an symmetric key uses 64 bits, which is below 128 and considered breakable. |
+| tst.js:13:14:13:56 | CryptoJ ... e: 2 }) | Creation of an symmetric PBKDF2 key uses 64 bits, which is below 128 and considered breakable. |
+| tst.js:14:14:14:60 | CryptoJ ... e: 2 }) | Creation of an symmetric PBKDF2 key uses 64 bits, which is below 128 and considered breakable. |
+| tst.js:15:14:15:60 | CryptoJ ... e: 2 }) | Creation of an symmetric EVPKDF key uses 64 bits, which is below 128 and considered breakable. |
+| tst.js:19:12:19:57 | forge.r ... rd, 64) | Creation of an symmetric RC2 key uses 64 bits, which is below 128 and considered breakable. |
+| tst.js:26:12:26:53 | forge.c ... , key2) | Creation of an symmetric AES key uses 64 bits, which is below 128 and considered breakable. |
+| tst.js:30:12:30:56 | forge.c ... , key3) | Creation of an symmetric 3DES key uses 64 bits, which is below 128 and considered breakable. |
+| tst.js:35:13:35:43 | crypto. ... an(512) | Creation of an asymmetric key uses 512 bits, which is below 2048 and considered breakable. |
+| tst.js:39:13:39:33 | new Nod ... : 512}) | Creation of an asymmetric RSA key uses 512 bits, which is below 2048 and considered breakable. |
+| tst.js:43:1:43:31 | key.gen ...  65537) | Creation of an asymmetric RSA key uses 512 bits, which is below 2048 and considered breakable. |

javascript/ql/test/query-tests/Security/CWE-326/InsufficientKeySize.qlref

@@ -0,0 +1 @@
+Security/CWE-326/InsufficientKeySize.ql

javascript/ql/test/query-tests/Security/CWE-326/tst.js

@@ -0,0 +1,44 @@
+const crypto = require("crypto");
+
+const bad1 = crypto.generateKeyPairSync("rsa", { modulusLength: 1024 }); // NOT OK
+
+const good1 = crypto.generateKeyPairSync("rsa", { modulusLength: 4096 }); // OK
+
+const bad2 = crypto.generateKeySync("hmac", { length: 64 }); // NOT OK
+
+const good2 = crypto.generateKeySync("aes", { length: 256 }); // OK
+
+var CryptoJS = require("crypto-js");
+
+const bad3 = CryptoJS.algo.PBKDF2.create({ keySize: 2 }); // NOT OK
+const bad4 = CryptoJS.PBKDF2(password, salt, { keySize: 2 }); // NOT OK
+const bad5 = CryptoJS.EvpKDF(password, salt, { keySize: 2 }); // NOT OK
+const bad6 = CryptoJS.PBKDF2(password, salt, { keySize: 8 }); // OK
+
+const forge = require("node-forge");
+var bad7 = forge.rc2.createEncryptionCipher(password, 64); // NOT OK
+var good3 = forge.rc2.createEncryptionCipher(password, 128); // OK
+
+var key1 = forge.random.getBytesSync(16);
+var good4 = forge.cipher.createCipher('AES-CBC', key1); // OK
+
+var key2 = forge.random.getBytesSync(8);
+var bad8 = forge.cipher.createCipher('AES-CBC', key2); // NOT OK
+
+var myBuffer = forge.util.createBuffer(manyBytes);
+var key3 = myBuffer.getBytes(8);
+var bad9 = forge.cipher.createDecipher('3DES-CBC', key3); // NOT OK
+
+var key4 = myBuffer.getBytes(16);
+var good5 = forge.cipher.createDecipher('AES-CBC', key4); // OK
+
+var bad10 = crypto.createDiffieHellman(512);
+var good6 = crypto.createDiffieHellman(2048);
+
+const NodeRSA = require('node-rsa');
+var bad11 = new NodeRSA({b: 512}); // NOT OK
+var good7 = new NodeRSA({b: 4096}); // OK
+
+var key = new NodeRSA(); // OK
+key.generateKeyPair(512, 65537); // NOT OK
+key.generateKeyPair(4096, 65537); // OK

javascript/ql/test/query-tests/Security/CWE-384/SessionFixation.expected

@@ -0,0 +1,2 @@
+| tst.js:9:1:14:2 | app.get ... n');\\n}) | Route handler does not invalidate session following login |
+| tst.js:27:1:29:2 | app.get ... n');\\n}) | Route handler does not invalidate session following login |

javascript/ql/test/query-tests/Security/CWE-384/SessionFixation.qlref

@@ -0,0 +1 @@
+Security/CWE-384/SessionFixation.ql

javascript/ql/test/query-tests/Security/CWE-384/tst.js

@@ -0,0 +1,40 @@
+const express = require('express');
+const session = require('express-session');
+const passport = require('passport');
+const app = express();
+app.use(session({
+    secret: 'keyboard cat'
+}));
+// handle login
+app.get('/login', function (req, res) { // NOT OK - no regenerate
+    req.session.user = {
+        userId: something
+    };
+    res.send('logged in');
+});
+
+// with regenerate
+app.get('/login2', function (req, res) { // OK
+    req.session.regenerate(function (err) {
+        req.session.user = {
+            userId: something
+        };
+        res.send('logged in');
+    });
+});
+
+// using passport
+app.get('/passport', passport.authenticate('local'), function (req, res) { // NOT OK - no regenerate
+    res.send('logged in');
+});
+
+// with regenerate, still using passport
+app.get('/passport2', passport.authenticate('local'), function (req, res) { // OK
+    var passport = req._passport.instance;
+    req.session.regenerate(function(err, done, user) {
+        req.session[passport._key] = {};
+        req._passport.instance = passport;
+        req._passport.session = req.session[passport._key];
+        res.send('logged in');
+    });
+});

javascript/ql/test/query-tests/Security/CWE-915/PrototypePollutingAssignment/PrototypePollutingAssignment.expected

@@ -1,4 +1,93 @@
 nodes
+| lib.js:1:38:1:40 | obj |
+| lib.js:1:43:1:46 | path |
+| lib.js:1:43:1:46 | path |
+| lib.js:1:43:1:46 | path |
+| lib.js:2:7:2:27 | currentPath |
+| lib.js:2:7:2:27 | currentPath |
+| lib.js:2:21:2:24 | path |
+| lib.js:2:21:2:24 | path |
+| lib.js:2:21:2:27 | path[0] |
+| lib.js:2:21:2:27 | path[0] |
+| lib.js:6:7:6:9 | obj |
+| lib.js:6:7:6:9 | obj |
+| lib.js:11:17:11:32 | obj[currentPath] |
+| lib.js:11:17:11:32 | obj[currentPath] |
+| lib.js:11:21:11:31 | currentPath |
+| lib.js:11:21:11:31 | currentPath |
+| lib.js:11:35:11:38 | path |
+| lib.js:11:35:11:38 | path |
+| lib.js:11:35:11:47 | path.slice(1) |
+| lib.js:11:35:11:47 | path.slice(1) |
+| lib.js:14:38:14:41 | path |
+| lib.js:14:38:14:41 | path |
+| lib.js:15:3:15:14 | obj[path[0]] |
+| lib.js:15:3:15:14 | obj[path[0]] |
+| lib.js:15:7:15:10 | path |
+| lib.js:15:7:15:13 | path[0] |
+| lib.js:20:7:20:25 | path |
+| lib.js:20:14:20:25 | arguments[1] |
+| lib.js:20:14:20:25 | arguments[1] |
+| lib.js:22:3:22:14 | obj[path[0]] |
+| lib.js:22:3:22:14 | obj[path[0]] |
+| lib.js:22:7:22:10 | path |
+| lib.js:22:7:22:13 | path[0] |
+| lib.js:25:44:25:47 | path |
+| lib.js:25:44:25:47 | path |
+| lib.js:26:10:26:21 | obj[path[0]] |
+| lib.js:26:10:26:21 | obj[path[0]] |
+| lib.js:26:14:26:17 | path |
+| lib.js:26:14:26:20 | path[0] |
+| lib.js:32:7:32:20 | path |
+| lib.js:32:14:32:20 | args[1] |
+| lib.js:32:14:32:20 | args[1] |
+| lib.js:34:3:34:14 | obj[path[0]] |
+| lib.js:34:3:34:14 | obj[path[0]] |
+| lib.js:34:7:34:10 | path |
+| lib.js:34:7:34:13 | path[0] |
+| lib.js:40:7:40:20 | path |
+| lib.js:40:14:40:20 | args[1] |
+| lib.js:40:14:40:20 | args[1] |
+| lib.js:42:3:42:14 | obj[path[0]] |
+| lib.js:42:3:42:14 | obj[path[0]] |
+| lib.js:42:7:42:10 | path |
+| lib.js:42:7:42:13 | path[0] |
+| lib.js:45:13:45:13 | s |
+| lib.js:45:13:45:13 | s |
+| lib.js:46:10:46:10 | s |
+| lib.js:52:9:52:22 | path |
+| lib.js:52:16:52:22 | id("x") |
+| lib.js:55:11:55:22 | obj[path[0]] |
+| lib.js:55:11:55:22 | obj[path[0]] |
+| lib.js:55:15:55:18 | path |
+| lib.js:55:15:55:21 | path[0] |
+| lib.js:59:18:59:18 | s |
+| lib.js:59:18:59:18 | s |
+| lib.js:61:17:61:17 | s |
+| lib.js:68:11:68:26 | path |
+| lib.js:68:18:68:26 | this.path |
+| lib.js:70:13:70:24 | obj[path[0]] |
+| lib.js:70:13:70:24 | obj[path[0]] |
+| lib.js:70:17:70:20 | path |
+| lib.js:70:17:70:23 | path[0] |
+| lib.js:83:7:83:25 | path |
+| lib.js:83:14:83:25 | arguments[1] |
+| lib.js:83:14:83:25 | arguments[1] |
+| lib.js:86:7:86:26 | proto |
+| lib.js:86:15:86:26 | obj[path[0]] |
+| lib.js:86:19:86:22 | path |
+| lib.js:86:19:86:25 | path[0] |
+| lib.js:87:10:87:14 | proto |
+| lib.js:87:10:87:14 | proto |
+| lib.js:90:43:90:46 | path |
+| lib.js:90:43:90:46 | path |
+| lib.js:91:7:91:28 | maybeProto |
+| lib.js:91:20:91:28 | obj[path] |
+| lib.js:91:24:91:27 | path |
+| lib.js:92:3:92:12 | maybeProto |
+| lib.js:92:3:92:12 | maybeProto |
+| lib.js:95:3:95:12 | maybeProto |
+| lib.js:95:3:95:12 | maybeProto |
 | tst.js:5:9:5:38 | taint |
 | tst.js:5:17:5:38 | String( ... y.data) |
 | tst.js:5:24:5:37 | req.query.data |
@@ -23,7 +112,113 @@ nodes
 | tst.js:45:9:45:11 | obj |
 | tst.js:48:9:48:11 | obj |
 | tst.js:48:9:48:11 | obj |
+| tst.js:77:9:77:38 | taint |
+| tst.js:77:17:77:38 | String( ... y.data) |
+| tst.js:77:24:77:37 | req.query.data |
+| tst.js:77:24:77:37 | req.query.data |
+| tst.js:80:5:80:17 | object[taint] |
+| tst.js:80:5:80:17 | object[taint] |
+| tst.js:80:12:80:16 | taint |
+| tst.js:82:5:82:22 | object["" + taint] |
+| tst.js:82:5:82:22 | object["" + taint] |
+| tst.js:82:12:82:21 | "" + taint |
+| tst.js:82:17:82:21 | taint |
+| tst.js:87:9:87:21 | object[taint] |
+| tst.js:87:9:87:21 | object[taint] |
+| tst.js:87:16:87:20 | taint |
+| tst.js:94:5:94:37 | obj[req ... ', '')] |
+| tst.js:94:5:94:37 | obj[req ... ', '')] |
+| tst.js:94:9:94:19 | req.query.x |
+| tst.js:94:9:94:19 | req.query.x |
+| tst.js:94:9:94:36 | req.que ... _', '') |
+| tst.js:97:5:97:46 | obj[req ... g, '')] |
+| tst.js:97:5:97:46 | obj[req ... g, '')] |
+| tst.js:97:9:97:19 | req.query.x |
+| tst.js:97:9:97:19 | req.query.x |
+| tst.js:97:9:97:45 | req.que ... /g, '') |
 edges
+| lib.js:1:38:1:40 | obj | lib.js:6:7:6:9 | obj |
+| lib.js:1:38:1:40 | obj | lib.js:6:7:6:9 | obj |
+| lib.js:1:43:1:46 | path | lib.js:2:21:2:24 | path |
+| lib.js:1:43:1:46 | path | lib.js:2:21:2:24 | path |
+| lib.js:1:43:1:46 | path | lib.js:2:21:2:24 | path |
+| lib.js:1:43:1:46 | path | lib.js:11:35:11:38 | path |
+| lib.js:1:43:1:46 | path | lib.js:11:35:11:38 | path |
+| lib.js:1:43:1:46 | path | lib.js:11:35:11:38 | path |
+| lib.js:2:7:2:27 | currentPath | lib.js:11:21:11:31 | currentPath |
+| lib.js:2:7:2:27 | currentPath | lib.js:11:21:11:31 | currentPath |
+| lib.js:2:21:2:24 | path | lib.js:2:21:2:27 | path[0] |
+| lib.js:2:21:2:24 | path | lib.js:2:21:2:27 | path[0] |
+| lib.js:2:21:2:27 | path[0] | lib.js:2:7:2:27 | currentPath |
+| lib.js:2:21:2:27 | path[0] | lib.js:2:7:2:27 | currentPath |
+| lib.js:11:17:11:32 | obj[currentPath] | lib.js:1:38:1:40 | obj |
+| lib.js:11:17:11:32 | obj[currentPath] | lib.js:1:38:1:40 | obj |
+| lib.js:11:21:11:31 | currentPath | lib.js:11:17:11:32 | obj[currentPath] |
+| lib.js:11:21:11:31 | currentPath | lib.js:11:17:11:32 | obj[currentPath] |
+| lib.js:11:35:11:38 | path | lib.js:11:35:11:47 | path.slice(1) |
+| lib.js:11:35:11:38 | path | lib.js:11:35:11:47 | path.slice(1) |
+| lib.js:11:35:11:47 | path.slice(1) | lib.js:1:43:1:46 | path |
+| lib.js:11:35:11:47 | path.slice(1) | lib.js:1:43:1:46 | path |
+| lib.js:14:38:14:41 | path | lib.js:15:7:15:10 | path |
+| lib.js:14:38:14:41 | path | lib.js:15:7:15:10 | path |
+| lib.js:15:7:15:10 | path | lib.js:15:7:15:13 | path[0] |
+| lib.js:15:7:15:13 | path[0] | lib.js:15:3:15:14 | obj[path[0]] |
+| lib.js:15:7:15:13 | path[0] | lib.js:15:3:15:14 | obj[path[0]] |
+| lib.js:20:7:20:25 | path | lib.js:22:7:22:10 | path |
+| lib.js:20:14:20:25 | arguments[1] | lib.js:20:7:20:25 | path |
+| lib.js:20:14:20:25 | arguments[1] | lib.js:20:7:20:25 | path |
+| lib.js:22:7:22:10 | path | lib.js:22:7:22:13 | path[0] |
+| lib.js:22:7:22:13 | path[0] | lib.js:22:3:22:14 | obj[path[0]] |
+| lib.js:22:7:22:13 | path[0] | lib.js:22:3:22:14 | obj[path[0]] |
+| lib.js:25:44:25:47 | path | lib.js:26:14:26:17 | path |
+| lib.js:25:44:25:47 | path | lib.js:26:14:26:17 | path |
+| lib.js:26:14:26:17 | path | lib.js:26:14:26:20 | path[0] |
+| lib.js:26:14:26:20 | path[0] | lib.js:26:10:26:21 | obj[path[0]] |
+| lib.js:26:14:26:20 | path[0] | lib.js:26:10:26:21 | obj[path[0]] |
+| lib.js:32:7:32:20 | path | lib.js:34:7:34:10 | path |
+| lib.js:32:14:32:20 | args[1] | lib.js:32:7:32:20 | path |
+| lib.js:32:14:32:20 | args[1] | lib.js:32:7:32:20 | path |
+| lib.js:34:7:34:10 | path | lib.js:34:7:34:13 | path[0] |
+| lib.js:34:7:34:13 | path[0] | lib.js:34:3:34:14 | obj[path[0]] |
+| lib.js:34:7:34:13 | path[0] | lib.js:34:3:34:14 | obj[path[0]] |
+| lib.js:40:7:40:20 | path | lib.js:42:7:42:10 | path |
+| lib.js:40:14:40:20 | args[1] | lib.js:40:7:40:20 | path |
+| lib.js:40:14:40:20 | args[1] | lib.js:40:7:40:20 | path |
+| lib.js:42:7:42:10 | path | lib.js:42:7:42:13 | path[0] |
+| lib.js:42:7:42:13 | path[0] | lib.js:42:3:42:14 | obj[path[0]] |
+| lib.js:42:7:42:13 | path[0] | lib.js:42:3:42:14 | obj[path[0]] |
+| lib.js:45:13:45:13 | s | lib.js:46:10:46:10 | s |
+| lib.js:45:13:45:13 | s | lib.js:46:10:46:10 | s |
+| lib.js:46:10:46:10 | s | lib.js:52:16:52:22 | id("x") |
+| lib.js:52:9:52:22 | path | lib.js:55:15:55:18 | path |
+| lib.js:52:16:52:22 | id("x") | lib.js:52:9:52:22 | path |
+| lib.js:55:15:55:18 | path | lib.js:55:15:55:21 | path[0] |
+| lib.js:55:15:55:21 | path[0] | lib.js:55:11:55:22 | obj[path[0]] |
+| lib.js:55:15:55:21 | path[0] | lib.js:55:11:55:22 | obj[path[0]] |
+| lib.js:59:18:59:18 | s | lib.js:61:17:61:17 | s |
+| lib.js:59:18:59:18 | s | lib.js:61:17:61:17 | s |
+| lib.js:61:17:61:17 | s | lib.js:68:18:68:26 | this.path |
+| lib.js:68:11:68:26 | path | lib.js:70:17:70:20 | path |
+| lib.js:68:18:68:26 | this.path | lib.js:68:11:68:26 | path |
+| lib.js:70:17:70:20 | path | lib.js:70:17:70:23 | path[0] |
+| lib.js:70:17:70:23 | path[0] | lib.js:70:13:70:24 | obj[path[0]] |
+| lib.js:70:17:70:23 | path[0] | lib.js:70:13:70:24 | obj[path[0]] |
+| lib.js:83:7:83:25 | path | lib.js:86:19:86:22 | path |
+| lib.js:83:14:83:25 | arguments[1] | lib.js:83:7:83:25 | path |
+| lib.js:83:14:83:25 | arguments[1] | lib.js:83:7:83:25 | path |
+| lib.js:86:7:86:26 | proto | lib.js:87:10:87:14 | proto |
+| lib.js:86:7:86:26 | proto | lib.js:87:10:87:14 | proto |
+| lib.js:86:15:86:26 | obj[path[0]] | lib.js:86:7:86:26 | proto |
+| lib.js:86:19:86:22 | path | lib.js:86:19:86:25 | path[0] |
+| lib.js:86:19:86:25 | path[0] | lib.js:86:15:86:26 | obj[path[0]] |
+| lib.js:90:43:90:46 | path | lib.js:91:24:91:27 | path |
+| lib.js:90:43:90:46 | path | lib.js:91:24:91:27 | path |
+| lib.js:91:7:91:28 | maybeProto | lib.js:92:3:92:12 | maybeProto |
+| lib.js:91:7:91:28 | maybeProto | lib.js:92:3:92:12 | maybeProto |
+| lib.js:91:7:91:28 | maybeProto | lib.js:95:3:95:12 | maybeProto |
+| lib.js:91:7:91:28 | maybeProto | lib.js:95:3:95:12 | maybeProto |
+| lib.js:91:20:91:28 | obj[path] | lib.js:91:7:91:28 | maybeProto |
+| lib.js:91:24:91:27 | path | lib.js:91:20:91:28 | obj[path] |
 | tst.js:5:9:5:38 | taint | tst.js:8:12:8:16 | taint |
 | tst.js:5:9:5:38 | taint | tst.js:9:12:9:16 | taint |
 | tst.js:5:9:5:38 | taint | tst.js:12:25:12:29 | taint |
@@ -47,11 +242,45 @@ edges
 | tst.js:33:23:33:25 | obj | tst.js:45:9:45:11 | obj |
 | tst.js:33:23:33:25 | obj | tst.js:48:9:48:11 | obj |
 | tst.js:33:23:33:25 | obj | tst.js:48:9:48:11 | obj |
+| tst.js:77:9:77:38 | taint | tst.js:80:12:80:16 | taint |
+| tst.js:77:9:77:38 | taint | tst.js:82:17:82:21 | taint |
+| tst.js:77:9:77:38 | taint | tst.js:87:16:87:20 | taint |
+| tst.js:77:17:77:38 | String( ... y.data) | tst.js:77:9:77:38 | taint |
+| tst.js:77:24:77:37 | req.query.data | tst.js:77:17:77:38 | String( ... y.data) |
+| tst.js:77:24:77:37 | req.query.data | tst.js:77:17:77:38 | String( ... y.data) |
+| tst.js:80:12:80:16 | taint | tst.js:80:5:80:17 | object[taint] |
+| tst.js:80:12:80:16 | taint | tst.js:80:5:80:17 | object[taint] |
+| tst.js:82:12:82:21 | "" + taint | tst.js:82:5:82:22 | object["" + taint] |
+| tst.js:82:12:82:21 | "" + taint | tst.js:82:5:82:22 | object["" + taint] |
+| tst.js:82:17:82:21 | taint | tst.js:82:12:82:21 | "" + taint |
+| tst.js:87:16:87:20 | taint | tst.js:87:9:87:21 | object[taint] |
+| tst.js:87:16:87:20 | taint | tst.js:87:9:87:21 | object[taint] |
+| tst.js:94:9:94:19 | req.query.x | tst.js:94:9:94:36 | req.que ... _', '') |
+| tst.js:94:9:94:19 | req.query.x | tst.js:94:9:94:36 | req.que ... _', '') |
+| tst.js:94:9:94:36 | req.que ... _', '') | tst.js:94:5:94:37 | obj[req ... ', '')] |
+| tst.js:94:9:94:36 | req.que ... _', '') | tst.js:94:5:94:37 | obj[req ... ', '')] |
+| tst.js:97:9:97:19 | req.query.x | tst.js:97:9:97:45 | req.que ... /g, '') |
+| tst.js:97:9:97:19 | req.query.x | tst.js:97:9:97:45 | req.que ... /g, '') |
+| tst.js:97:9:97:45 | req.que ... /g, '') | tst.js:97:5:97:46 | obj[req ... g, '')] |
+| tst.js:97:9:97:45 | req.que ... /g, '') | tst.js:97:5:97:46 | obj[req ... g, '')] |
 #select
-| tst.js:8:5:8:17 | object[taint] | tst.js:5:24:5:37 | req.query.data | tst.js:8:5:8:17 | object[taint] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | tst.js:5:24:5:37 | req.query.data | here |
-| tst.js:9:5:9:17 | object[taint] | tst.js:5:24:5:37 | req.query.data | tst.js:9:5:9:17 | object[taint] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | tst.js:5:24:5:37 | req.query.data | here |
-| tst.js:14:5:14:32 | unsafeG ...  taint) | tst.js:5:24:5:37 | req.query.data | tst.js:14:5:14:32 | unsafeG ...  taint) | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | tst.js:5:24:5:37 | req.query.data | here |
-| tst.js:34:5:34:7 | obj | tst.js:5:24:5:37 | req.query.data | tst.js:34:5:34:7 | obj | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | tst.js:5:24:5:37 | req.query.data | here |
-| tst.js:39:9:39:11 | obj | tst.js:5:24:5:37 | req.query.data | tst.js:39:9:39:11 | obj | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | tst.js:5:24:5:37 | req.query.data | here |
-| tst.js:45:9:45:11 | obj | tst.js:5:24:5:37 | req.query.data | tst.js:45:9:45:11 | obj | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | tst.js:5:24:5:37 | req.query.data | here |
-| tst.js:48:9:48:11 | obj | tst.js:5:24:5:37 | req.query.data | tst.js:48:9:48:11 | obj | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | tst.js:5:24:5:37 | req.query.data | here |
+| lib.js:6:7:6:9 | obj | lib.js:1:43:1:46 | path | lib.js:6:7:6:9 | obj | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | lib.js:1:43:1:46 | path | library input |
+| lib.js:15:3:15:14 | obj[path[0]] | lib.js:14:38:14:41 | path | lib.js:15:3:15:14 | obj[path[0]] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | lib.js:14:38:14:41 | path | library input |
+| lib.js:22:3:22:14 | obj[path[0]] | lib.js:20:14:20:25 | arguments[1] | lib.js:22:3:22:14 | obj[path[0]] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | lib.js:20:14:20:25 | arguments[1] | library input |
+| lib.js:26:10:26:21 | obj[path[0]] | lib.js:25:44:25:47 | path | lib.js:26:10:26:21 | obj[path[0]] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | lib.js:25:44:25:47 | path | library input |
+| lib.js:34:3:34:14 | obj[path[0]] | lib.js:32:14:32:20 | args[1] | lib.js:34:3:34:14 | obj[path[0]] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | lib.js:32:14:32:20 | args[1] | library input |
+| lib.js:42:3:42:14 | obj[path[0]] | lib.js:40:14:40:20 | args[1] | lib.js:42:3:42:14 | obj[path[0]] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | lib.js:40:14:40:20 | args[1] | library input |
+| lib.js:70:13:70:24 | obj[path[0]] | lib.js:59:18:59:18 | s | lib.js:70:13:70:24 | obj[path[0]] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | lib.js:59:18:59:18 | s | library input |
+| lib.js:87:10:87:14 | proto | lib.js:83:14:83:25 | arguments[1] | lib.js:87:10:87:14 | proto | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | lib.js:83:14:83:25 | arguments[1] | library input |
+| tst.js:8:5:8:17 | object[taint] | tst.js:5:24:5:37 | req.query.data | tst.js:8:5:8:17 | object[taint] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | tst.js:5:24:5:37 | req.query.data | user controlled input |
+| tst.js:9:5:9:17 | object[taint] | tst.js:5:24:5:37 | req.query.data | tst.js:9:5:9:17 | object[taint] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | tst.js:5:24:5:37 | req.query.data | user controlled input |
+| tst.js:14:5:14:32 | unsafeG ...  taint) | tst.js:5:24:5:37 | req.query.data | tst.js:14:5:14:32 | unsafeG ...  taint) | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | tst.js:5:24:5:37 | req.query.data | user controlled input |
+| tst.js:34:5:34:7 | obj | tst.js:5:24:5:37 | req.query.data | tst.js:34:5:34:7 | obj | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | tst.js:5:24:5:37 | req.query.data | user controlled input |
+| tst.js:39:9:39:11 | obj | tst.js:5:24:5:37 | req.query.data | tst.js:39:9:39:11 | obj | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | tst.js:5:24:5:37 | req.query.data | user controlled input |
+| tst.js:45:9:45:11 | obj | tst.js:5:24:5:37 | req.query.data | tst.js:45:9:45:11 | obj | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | tst.js:5:24:5:37 | req.query.data | user controlled input |
+| tst.js:48:9:48:11 | obj | tst.js:5:24:5:37 | req.query.data | tst.js:48:9:48:11 | obj | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | tst.js:5:24:5:37 | req.query.data | user controlled input |
+| tst.js:80:5:80:17 | object[taint] | tst.js:77:24:77:37 | req.query.data | tst.js:80:5:80:17 | object[taint] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | tst.js:77:24:77:37 | req.query.data | user controlled input |
+| tst.js:82:5:82:22 | object["" + taint] | tst.js:77:24:77:37 | req.query.data | tst.js:82:5:82:22 | object["" + taint] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | tst.js:77:24:77:37 | req.query.data | user controlled input |
+| tst.js:87:9:87:21 | object[taint] | tst.js:77:24:77:37 | req.query.data | tst.js:87:9:87:21 | object[taint] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | tst.js:77:24:77:37 | req.query.data | user controlled input |
+| tst.js:94:5:94:37 | obj[req ... ', '')] | tst.js:94:9:94:19 | req.query.x | tst.js:94:5:94:37 | obj[req ... ', '')] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | tst.js:94:9:94:19 | req.query.x | user controlled input |
+| tst.js:97:5:97:46 | obj[req ... g, '')] | tst.js:97:9:97:19 | req.query.x | tst.js:97:5:97:46 | obj[req ... g, '')] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | tst.js:97:9:97:19 | req.query.x | user controlled input |

javascript/ql/test/query-tests/Security/CWE-915/PrototypePollutingAssignment/lib.js

@@ -0,0 +1,96 @@
+module.exports.set = function recSet(obj, path, value) {
+  var currentPath = path[0];
+  var currentValue = obj[currentPath];
+  if (path.length === 1) {
+    if (currentValue === void 0) {
+      obj[currentPath] = value; // NOT OK
+    }
+    return currentValue;
+  }
+
+  return recSet(obj[currentPath], path.slice(1), value);
+}
+
+module.exports.set2 = function (obj, path, value) {
+  obj[path[0]][path[1]] = value; // NOT OK
+}
+
+module.exports.setWithArgs = function() {
+  var obj = arguments[0];
+  var path = arguments[1];
+  var value = arguments[2];
+  obj[path[0]][path[1]] = value; // NOT OK
+}
+
+module.exports.usedInTest = function (obj, path, value) {
+  return obj[path[0]][path[1]] = value; // NOT OK
+}
+
+module.exports.setWithArgs2 = function() {
+  const args = Array.prototype.slice.call(arguments);
+  var obj = args[0];
+  var path = args[1];
+  var value = args[2];
+  obj[path[0]][path[1]] = value; // NOT OK
+}
+
+module.exports.setWithArgs3 = function() {
+  const args = Array.from(arguments);
+  var obj = args[0];
+  var path = args[1];
+  var value = args[2];
+  obj[path[0]][path[1]] = value; // NOT OK
+}
+
+function id(s) {
+  return s;
+}
+
+module.exports.id = id;
+
+module.exports.notVulnerable = function () {
+  const path = id("x");
+  const value = id("y");
+  const obj = id("z");
+  return (obj[path[0]][path[1]] = value); // OK
+}
+
+class Foo {
+  constructor(o, s, v) {
+    this.obj = o;
+    this.path = s;
+    this.value = v;
+  }
+
+  doXss() {
+    // not called here, but still bad.
+    const obj = this.obj;
+    const path = this.path;
+    const value = this.value;
+    return (obj[path[0]][path[1]] = value); // NOT OK
+  }
+
+  safe() {
+    const obj = this.obj;
+    obj[path[0]] = this.value; // OK
+  }
+}
+
+module.exports.Foo = Foo;
+
+module.exports.delete = function() {
+  var obj = arguments[0];
+  var path = arguments[1];
+  delete obj[path[0]]; // OK
+  var prop = arguments[2];
+  var proto = obj[path[0]];
+  delete proto[prop]; // NOT OK
+}
+
+module.exports.fixedProp = function (obj, path, value) {
+  var maybeProto = obj[path];
+  maybeProto.foo = value; // OK - fixed properties from library inputs are OK.
+
+  var i = 0;
+  maybeProto[i + 2] = value; // OK - number properties are OK.
+}

javascript/ql/test/query-tests/Security/CWE-915/PrototypePollutingAssignment/lib.spec.js

@@ -0,0 +1,9 @@
+const lib = require("./lib");
+
+describe("lib", () => {
+    it("should work", () => {
+        const obj = Object.create(null);
+
+        lib.usedInTest(obj, "foo", "my-value");
+    });
+});

javascript/ql/test/query-tests/Security/CWE-915/PrototypePollutingAssignment/package.json

@@ -0,0 +1,5 @@
+{
+    "name": "my-lib",
+    "version": "0.0.7",
+    "main": "./lib.js"
+}

javascript/ql/test/query-tests/Security/CWE-915/PrototypePollutingAssignment/tst.js

@@ -71,3 +71,29 @@ class Box {
         this.foo = 'bar'; // OK - 'this' won't refer to Object.prototype
     }
 }
+
+
+app.get('/', (req, res) => {
+    let taint = String(req.query.data);
+
+    let object = {};
+    object[taint][taint] = taint; // NOT OK
+
+    object["" + taint]["" + taint] = taint; // NOT OK
+
+    if (!taint.includes("__proto__")) {
+        object[taint][taint] = taint; // OK
+    } else {
+        object[taint][taint] = taint; // NOT OK
+    }
+});
+
+app.get('/foo', (req, res) => {
+    let obj = {};
+    obj[req.query.x.replace('_', '-')].x = 'foo'; // OK
+    obj[req.query.x.replace('_', '')].x = 'foo'; // NOT OK
+    obj[req.query.x.replace(/_/g, '')].x = 'foo'; // OK
+    obj[req.query.x.replace(/_/g, '-')].x = 'foo'; // OK
+    obj[req.query.x.replace(/__proto__/g, '')].x = 'foo'; // NOT OK - "__pr__proto__oto__"
+    obj[req.query.x.replace('o', '0')].x = 'foo'; // OK
+});