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#397
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
CVE-ID: CVE-2023-32559
  • Loading branch information
tniessen authored and RafaelGSS committed Aug 9, 2023
1 parent 92300b5 commit cf348ec
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 0 deletions.
4 changes: 4 additions & 0 deletions doc/api/deprecations.md
Expand Up @@ -2212,6 +2212,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 @@ -3527,6 +3530,7 @@ Consider using alternatives such as the [`mock`][] helper function.
[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
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 cf348ec

Please sign in to comment.