CodeQL False Negative - Protototype Pollution
- Linguagem predominante
- CodeQL
- Estrelas
- 10.1k
- Forks
- 2.1k
- Merge médio
- 2d 15h
- PRs com merge (30d)
- 141
Descrição
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())
}
```
Guia de contribuição
Direção de pesquisa
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.
Escrita pelo modelo de indexação a partir do texto da issue.
Avaliação
- Stack de tecnologia
- javascript
- Domínio
- security
- Tipo de issue
- Bug
- Dificuldade
- 4/5
- Tempo estimado
- 3-5 dias
- Status de atividade
- Estagnada
- Clareza
- Razoavelmente clara
- Facilidade para iniciantes
- 38/100