False positive for Prototype-polluting function
- 主要言語
- CodeQL
- スター
- 10.1k
- フォーク
- 2.1k
- 平均マージ
- 2日 15時間
- マージ済み PR(30日)
- 141
説明
I check for prototype polluting property keys in a set of reserved keys which include `__proto__` and `constructor`.
This shouldn't even be necessary since the key, value come from Object.entries which according to MDN will only iterate own enumerable string-keyd property. ie. never `__proto__` or `constructor`.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/entries

Still the lookup in the set does not work.
```js
// Iterate through the source own enumerable string-keyed property key-value pairs.
for (const [key, value] of Object.entries(source)) {
// This for codeql only. key, value of Object.entries should ensure that only own properties are parsed
if (!source.hasOwnProperty(key)) continue;
// The ignoreKeys contain checks against prototype pollution.
if (new Set(['__proto__', 'constructor', 'mapview']).has(key)) {
continue;
}
```
CodeQL looks at the right place but ignores the check for the set.
https://github.com/GEOLYTIX/xyz/security/code-scanning/217

The only way I can make the issue go away is by doing a === check on the string value like so.
```js
// Prevent prototype polluting assignment.
if (key === '__proto__' || key === 'constructor') return true;
```
Even though I know that this issue can not happen I need to add this extra line to make the CodeQL warning go away.
コントリビューションガイド
調査の方向性
リンクされている CodeQL コードスキャンアラートと報告された JavaScript スニペットから始めます。リポジトリのファイルやテストは指定されていません。関連するクエリが Object.entries と Set のメンバーシップをどのように処理しているかを追跡し、この誤検知が報告されなくなるようにリグレッションテストのカバレッジを追加または更新します。
索引モデルが issue の本文から書いたものです。
評価
- 技術スタック
- javascript
- 領域
- security
- issue の種類
- バグ
- 難易度
- 4/5
- 見積もり時間
- 3〜5日
- 活発さ
- 停滞
- 明瞭さ
- 説明が足りない
- 初心者へのやさしさ
- 25/100