github / github/codeql

CodeQL False Negative - Protototype Pollution

Offen
#18,665 6 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen
question
Vorherrschende Sprache
CodeQL
Sterne
10.1k
Forks
2.1k
Ø Merge
2 T. 15 Std.
Gemergte PRs (30 T.)
141

Beschreibung

Hi,
I would like to report a possible false negative for [SNYK-JS-XASSIGN-1759314](https://security.snyk.io/vuln/SNYK-JS-XASSIGN-1759314).

Relevant code:
```js
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:
```ql
/**
* @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;

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):
```ql
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())
}
```

Beitragsleitfaden

Beitragsleitfaden öffnen

Rechercherichtung

Start with the supplied Config, isSource, isSink, and isAdditionalFlowStep predicates and reproduce the flow using the JavaScript snippet and query. Compare the result with the PrototypePollutingAssignment sink; done means determining whether the missing flow is expected and capturing a regression case or a justified query change.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Bewertung

Tech-Stack
javascript
Bereich
security
Issue-Typ
Bug
Schwierigkeit
4/5
Geschätzter Aufwand
3-5 Tage
Aktivitätsstatus
Veraltet
Klarheit
Größtenteils klar
Anfängerfreundlichkeit
38/100

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.