CodeQL False Negative - Protototype Pollution

[DSimsek000]

Hi,
I would like to report a possible false negative for SNYK-JS-XASSIGN-1759314.

Relevant code:

const XAssign = {
   assign: function (...args) {
      /**
       * Combine object properties or concat array properties
       *
       * @param {any} acc the target or accumulator
       * @param {any} obj object to apply
       */

      function apply(acc, obj) {
         if (obj == null || typeof obj !== "object") {
            return // ignore non-object args
         }
         Object.keys(obj)
            .forEach((key) => {
               const value = obj[key]
               if (Array.isArray(value)) {
                  acc[key] =
                  acc[key] && Array.isArray(acc[key])
                     ? acc[key].concat(value)
                     : value
               } else if (typeof value === "object") {
                  acc[key] = acc[key] || {}
                  if (Array.isArray(acc[key])) {
                     acc[key] = {} // getting overridden with an Object!
                     apply(acc[key], value)
                  } else if (typeof acc[key] === "object") {
                     apply(acc[key], value)
                  } else {
                     acc[key] = value
                  }
               } else {
                  acc[key] = value
               }
            })
      }

      /**
       * Apply merge for each object argument.
       */
      const result = {}
      args.forEach((obj) => apply(result, obj))
      return result
   },
}

I ran the following query:

/**
 * @name Prototype pollution
 * @description Using externally controlled input to set properties on the prototype of an object can lead to prototype pollution.
 * @severity high
 * @kind path-problem
 * @precision high
 * @id js/prototype-pollution
 * @tags external/cwe/cwe-471 external/cwe/cwe-915
 */

import javascript
import semmle.javascript.dataflow.TaintTracking
import semmle.javascript.security.dataflow.PrototypePollutingAssignmentCustomizations::PrototypePollutingAssignment

module Config implements DataFlow::ConfigSig {
  DataFlow::FlowFeature getAFeature() { result instanceof DataFlow::FeatureHasSourceCallContext }

  predicate isSource(DataFlow::Node source) {
    exists(Function f | f.getName() = "assign" | source.asExpr() = f.getAParameter())
  }

  predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
}

module Flow = TaintTracking::Global<Config>;

import Flow::PathGraph

from Flow::PathNode source, Flow::PathNode sink
where Flow::flowPath(source, sink)
select sink.getNode(), source, sink, ""

By providing the following additional taint steps, I managed to find the vulnerability (though this may be overly broad):

predicate isAdditionalFlowStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
    exists(CallExpr ca | ca = toNode.asExpr() and ca.getAnArgument() = fromNode.asExpr())
    or
    exists(IndexExpr ie | ie = toNode.asExpr() and ie.getIndex() = fromNode.asExpr())
}

[redsun82]

Hi @DSimsek000 thanks a lot for raising this to our attention! I will forward it to our internal team working on javascript analysis.

[asgerf]

Hi 👋

The query js/prototype-pollution-utility (in this file) seems to flag this out of the box. Is there a reason you want a custom query for this?

[DSimsek000]

Hi @asgerf ,
my goal was to analyse a specific entry point and do global taint analysis. I was under the assumption that the above query would import all prototype pollution sink definitions.