Skip to content

lib: use __proto__: null when calling ObjectDefineProperty#64239

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
aduh95:proto-null-define
Jul 3, 2026
Merged

lib: use __proto__: null when calling ObjectDefineProperty#64239
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
aduh95:proto-null-define

Conversation

@aduh95

@aduh95 aduh95 commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

We have a lint rule for when the descriptor is a plain object, but it doesn't cover case where the descriptor is stored as a variable, and some instances were missing the null prototype.

As a reminder, setting __proto__: null is mandatory for property descriptor to avoid the code failing if e.g. both Object.prototype.value and Object.prototype.get are set to anything.

### Defining object own properties
When defining property descriptor (to add or update an own property to a
JavaScript object), be sure to always use a null-prototype object to avoid
prototype pollution.
```js
// User-land
Object.prototype.get = function get() {};
// Core
try {
ObjectDefineProperty({}, 'someProperty', { value: 0 });
} catch (err) {
console.log(err); // TypeError: Invalid property descriptor.
}
```
```js
// User-land
Object.prototype.get = function get() {};
// Core
ObjectDefineProperty({}, 'someProperty', { __proto__: null, value: 0 });
console.log('no errors'); // no errors.
```
Same applies when trying to modify an existing property, e.g. trying to make a
read-only property enumerable:
```js
// User-land
Object.prototype.value = 'Unrelated user-provided data';
// Core
class SomeClass {
get readOnlyProperty() { return 'genuine data'; }
}
ObjectDefineProperty(SomeClass.prototype, 'readOnlyProperty', { enumerable: true });
console.log(new SomeClass().readOnlyProperty); // Unrelated user-provided data
```
```js
// User-land
Object.prototype.value = 'Unrelated user-provided data';
// Core
const kEnumerableProperty = { __proto__: null, enumerable: true };
// In core, use const {kEnumerableProperty} = require('internal/util');
class SomeClass {
get readOnlyProperty() { return 'genuine data'; }
}
ObjectDefineProperty(SomeClass.prototype, 'readOnlyProperty', kEnumerableProperty);
console.log(new SomeClass().readOnlyProperty); // genuine data
```

Signed-off-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/http2
  • @nodejs/net
  • @nodejs/test_runner

@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Jul 1, 2026
@aduh95 aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Jul 2, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 2, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Comment on lines +352 to +359
const descriptors = ObjectGetOwnPropertyDescriptors(source);

const descValues = ObjectValues(descriptors);
for (let i = 0; i < descValues.length; ++i) {
ObjectSetPrototypeOf(descValues[i], null);
}

ObjectDefineProperties(target, descriptors);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we move this to internal/util? Looks like we're using it at least twice 🙂

@aduh95 aduh95 added the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 3, 2026
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 3, 2026
@nodejs-github-bot nodejs-github-bot merged commit dc15da3 into nodejs:main Jul 3, 2026
82 checks passed
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in dc15da3

richardlau pushed a commit that referenced this pull request Jul 3, 2026
Signed-off-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: #64239
Reviewed-By: Jordan Harband <ljharb@gmail.com>
Reviewed-By: René <contact.9a5d6388@renegade334.me.uk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants