CodeQL False Negative - Protototype Pollution
- 主要言語
- CodeQL
- スター
- 10.1k
- フォーク
- 2.1k
- 平均マージ
- 2日 15時間
- マージ済み PR(30日)
- 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())
}
```
コントリビューションガイド
調査の方向性
提供された Config、isSource、isSink、isAdditionalFlowStep predicate から始め、JavaScript スニペットと query を使ってフローを再現します。結果を PrototypePollutingAssignment sink と比較します。不足しているフローが想定されたものかどうかを判断し、回帰ケースまたは正当化された query の変更を記録できれば完了です。
索引モデルが issue の本文から書いたものです。
評価
- 技術スタック
- javascript
- 領域
- security
- issue の種類
- バグ
- 難易度
- 4/5
- 見積もり時間
- 3〜5日
- 活発さ
- 停滞
- 明瞭さ
- おおむね明確
- 初心者へのやさしさ
- 38/100