github / github/codeql

CodeQL False Negative - Protototype Pollution

未关闭
#18,665 6 条评论 0 个 reaction 已指派 0 人 在 GitHub 查看
question
主要语言
CodeQL
星标
10.1k
派生
2.1k
平均合并
2 天 15 小时
30 天内合并 PR
141

描述

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

贡献指南

打开贡献指南

调研方向

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.

由索引模型根据 Issue 内容生成。

评估

技术栈
javascript
领域
security
Issue 类型
缺陷
难度
4/5
预计耗时
3-5 天
活跃度
停滞
描述清晰度
基本清楚
新手友好度
38/100

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。