Skip to content

Commit

Permalink
policy: disable process.binding() when enabled
Browse files Browse the repository at this point in the history
process.binding() can be used to trivially bypass restrictions imposed
through a policy. Since the function is deprecated already, simply
replace it with a stub when a policy is being enabled.

Fixes: https://hackerone.com/bugs?report_id=1946470
PR-URL: nodejs-private/node-private#460
CVE-ID: CVE-2023-32559
  • Loading branch information
tniessen authored and RafaelGSS committed Aug 8, 2023
1 parent fe3abdf commit d4570fa
Show file tree
Hide file tree
Showing 7 changed files with 73 additions and 0 deletions.
4 changes: 4 additions & 0 deletions doc/api/deprecations.md
Expand Up @@ -2208,6 +2208,9 @@ Type: Documentation-only (supports [`--pending-deprecation`][])

`process.binding()` is for use by Node.js internal code only.

While `process.binding()` has not reached End-of-Life status in general, it is
unavailable when [policies][] are enabled.

### DEP0112: `dgram` private APIs

<!-- YAML
Expand Down Expand Up @@ -3469,6 +3472,7 @@ In a future version of Node.js, [`message.headers`][],
[from_string_encoding]: buffer.md#static-method-bufferfromstring-encoding
[legacy URL API]: url.md#legacy-url-api
[legacy `urlObject`]: url.md#legacy-urlobject
[policies]: permissions.md#policies
[static methods of `crypto.Certificate()`]: crypto.md#class-certificate
[subpath exports]: packages.md#subpath-exports
[subpath imports]: packages.md#subpath-imports
Expand Down
8 changes: 8 additions & 0 deletions doc/api/errors.md
Expand Up @@ -692,6 +692,14 @@ APIs _not_ using `AbortSignal`s typically do not raise an error with this code.
This code does not use the regular `ERR_*` convention Node.js errors use in
order to be compatible with the web platform's `AbortError`.

<a id="ERR_ACCESS_DENIED"></a>

### `ERR_ACCESS_DENIED`

A special type of error that is triggered whenever Node.js tries to get access
to a resource restricted by the [policy][] manifest.
For example, `process.binding`.

<a id="ERR_AMBIGUOUS_ARGUMENT"></a>

### `ERR_AMBIGUOUS_ARGUMENT`
Expand Down
3 changes: 3 additions & 0 deletions lib/internal/errors.js
Expand Up @@ -957,6 +957,9 @@ module.exports = {
//
// Note: Node.js specific errors must begin with the prefix ERR_

E('ERR_ACCESS_DENIED',
'Access to this API has been restricted. Permission: %s',
Error);
E('ERR_AMBIGUOUS_ARGUMENT', 'The "%s" argument is ambiguous. %s', TypeError);
E('ERR_ARG_NOT_ITERABLE', '%s must be iterable', TypeError);
E('ERR_ASSERTION', '%s', Error);
Expand Down
10 changes: 10 additions & 0 deletions lib/internal/process/policy.js
Expand Up @@ -7,6 +7,7 @@ const {
} = primordials;

const {
ERR_ACCESS_DENIED,
ERR_MANIFEST_TDZ,
} = require('internal/errors').codes;
const { Manifest } = require('internal/policy/manifest');
Expand All @@ -32,6 +33,15 @@ module.exports = ObjectFreeze({
return o;
});
manifest = new Manifest(json, url);

// process.binding() is deprecated (DEP0111) and trivially allows bypassing
// policies, so if policies are enabled, make this API unavailable.
process.binding = function binding(_module) {
throw new ERR_ACCESS_DENIED('process.binding');
};
process._linkedBinding = function _linkedBinding(_module) {
throw new ERR_ACCESS_DENIED('process._linkedBinding');
};
},

get manifest() {
Expand Down
10 changes: 10 additions & 0 deletions test/fixtures/policy/process-binding/app.js
@@ -0,0 +1,10 @@
'use strict';

const assert = require('assert');

assert.throws(() => { process.binding(); }, {
code: 'ERR_ACCESS_DENIED'
});
assert.throws(() => { process._linkedBinding(); }, {
code: 'ERR_ACCESS_DENIED'
});
10 changes: 10 additions & 0 deletions test/fixtures/policy/process-binding/policy.json
@@ -0,0 +1,10 @@
{
"resources": {
"./app.js": {
"integrity": true,
"dependencies": {
"assert": true
}
}
}
}
28 changes: 28 additions & 0 deletions test/parallel/test-policy-process-binding.js
@@ -0,0 +1,28 @@
'use strict';

const common = require('../common');
common.requireNoPackageJSONAbove();

if (!common.hasCrypto)
common.skip('missing crypto');

const fixtures = require('../common/fixtures');

const assert = require('node:assert');
const { spawnSync } = require('node:child_process');

const dep = fixtures.path('policy', 'process-binding', 'app.js');
const depPolicy = fixtures.path(
'policy',
'process-binding',
'policy.json');
const { status } = spawnSync(
process.execPath,
[
'--experimental-policy', depPolicy, dep,
],
{
stdio: 'inherit'
},
);
assert.strictEqual(status, 0);

0 comments on commit d4570fa

Please sign in to comment.