github / github/codeql

False positive for Prototype-polluting function

Open
#18,327 4 comments 1 reaction 0 assignees View on GitHub
false-positive javascript
Dominant language
CodeQL
Stars
10.1k
Forks
2.1k
Avg merge
2d 15h
Merged PRs (30d)
141

Description

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

![image](https://github.com/user-attachments/assets/55d3c365-db31-4132-b528-b147cddb6043)

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

![image](https://github.com/user-attachments/assets/d1cfd2a3-fde5-439e-b3b2-cc82ae87694e)

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.

Contributor guide

Open the contributing guide

Research direction

Start with the linked CodeQL code-scanning alert and the reported JavaScript snippet; no repository file or test is named. Trace how the relevant query handles Object.entries and Set membership, then add or update regression coverage so this false positive is no longer reported.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
security
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.