Skip to content

fix Prototype-polluting assignment ('Code Injection') #20895

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

odaysec
Copy link

@odaysec odaysec commented Apr 21, 2025

initializers[initializer.name] = initializer;

fix the issue validate the bucketName parameter to ensure it cannot be set to a prototype-polluting value like __proto__, constructor, or prototype. This can be achieved by adding a runtime check that explicitly rejects such values. This approach ensures that even if the type system is bypassed, the code remains secure.

The fix involves:

  1. Adding a validation step for bucketName at the beginning of the buildInitializerMethod function.
  2. Throwing an error if bucketName is set to a disallowed value.

Most JavaScript objects inherit the properties of the built-in Object.prototype object. Prototype pollution is a type of vulnerability in which an attacker is able to modify Object.prototype. Since most objects inherit from the compromised Object.prototype object, the attacker can use this to tamper with the application logic, and often escalate to remote code execution or cross-site scripting. One way to cause prototype pollution is by modifying an object obtained via a user-controlled property name. Most objects have a special __proto__ property that refers to Object.prototype. An attacker can abuse this special property to trick the application into performing unintended modifications of Object.prototype.

POC

the untrusted value req.params.id is used as the property name req.session.todos[id]. If a malicious user passes in the ID value __proto__, the variable items will then refer to Object.prototype. Finally, the modification of items then allows the attacker to inject arbitrary properties onto Object.prototype.

let express = require('express');
let app = express()

app.put('/todos/:id', (req, res) => {
    let id = req.params.id;
    let items = req.session.todos[id];
    if (!items) {
        items = req.session.todos[id] = {};
    }
    items[req.query.name] = req.query.text;
    res.end(200);
});

One way to fix this is to use Map objects to associate key/value pairs instead of regular objects, as shown below:

let express = require('express');
let app = express()

app.put('/todos/:id', (req, res) => {
    let id = req.params.id;
    let items = req.session.todos.get(id);
    if (!items) {
        items = new Map();
        req.sessions.todos.set(id, items);
    }
    items.set(req.query.name, req.query.text);
    res.end(200);
});

Another way to fix it is to prevent the __proto__ property from being used as a key, as shown below:

let express = require('express');
let app = express()

app.put('/todos/:id', (req, res) => {
    let id = req.params.id;
    if (id === '__proto__' || id === 'constructor' || id === 'prototype') {
        res.end(403);
        return;
    }
    let items = req.session.todos[id];
    if (!items) {
        items = req.session.todos[id] = {};
    }
    items[req.query.name] = req.query.text;
    res.end(200);
});

References

Object.prototype.proto
Map
CWE-78
CWE-79
CWE-94
CWE-400
CWE-471
CWE-915

Copy link
Contributor

@NullVoxPopuli NullVoxPopuli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some context about the requested changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants