github / github/codeql

False positive for Prototype-polluting function

Offen
#18,327 4 Kommentare 1 Reaktion 0 zugewiesene Personen Auf GitHub ansehen
false-positive javascript
Vorherrschende Sprache
CodeQL
Sterne
10.1k
Forks
2.1k
Ø Merge
2 T. 15 Std.
Gemergte PRs (30 T.)
141

Beschreibung

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.

Beitragsleitfaden

Beitragsleitfaden öffnen

Rechercherichtung

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.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Bewertung

Tech-Stack
javascript
Bereich
security
Issue-Typ
Bug
Schwierigkeit
4/5
Geschätzter Aufwand
3-5 Tage
Aktivitätsstatus
Veraltet
Klarheit
Muss geklärt werden
Anfängerfreundlichkeit
25/100

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.