Skip to content

Commit 4aa0eff

Browse files
tniessenRafaelGSS
authored andcommitted
policy: disable process.binding() when enabled
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 <[email protected]> CVE-ID: CVE-2023-32559
1 parent 4fdf70f commit 4aa0eff

File tree

6 files changed

+65
-0
lines changed

6 files changed

+65
-0
lines changed

doc/api/deprecations.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2215,6 +2215,9 @@ Type: Documentation-only (supports [`--pending-deprecation`][])
22152215

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

2218+
While `process.binding()` has not reached End-of-Life status in general, it is
2219+
unavailable when [policies][] are enabled.
2220+
22182221
### DEP0112: `dgram` private APIs
22192222

22202223
<!-- YAML
@@ -3532,6 +3535,7 @@ Consider using alternatives such as the [`mock`][] helper function.
35323535
[from_string_encoding]: buffer.md#static-method-bufferfromstring-encoding
35333536
[legacy URL API]: url.md#legacy-url-api
35343537
[legacy `urlObject`]: url.md#legacy-urlobject
3538+
[policies]: permissions.md#policies
35353539
[static methods of `crypto.Certificate()`]: crypto.md#class-certificate
35363540
[subpath exports]: packages.md#subpath-exports
35373541
[subpath imports]: packages.md#subpath-imports

lib/internal/errors.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -957,6 +957,9 @@ module.exports = {
957957
//
958958
// Note: Node.js specific errors must begin with the prefix ERR_
959959

960+
E('ERR_ACCESS_DENIED',
961+
'Access to this API has been restricted. Permission: %s',
962+
Error);
960963
E('ERR_AMBIGUOUS_ARGUMENT', 'The "%s" argument is ambiguous. %s', TypeError);
961964
E('ERR_ARG_NOT_ITERABLE', '%s must be iterable', TypeError);
962965
E('ERR_ASSERTION', '%s', Error);

lib/internal/process/policy.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ const {
77
} = primordials;
88

99
const {
10+
ERR_ACCESS_DENIED,
1011
ERR_MANIFEST_TDZ,
1112
} = require('internal/errors').codes;
1213
const { Manifest } = require('internal/policy/manifest');
@@ -32,6 +33,15 @@ module.exports = ObjectFreeze({
3233
return o;
3334
});
3435
manifest = new Manifest(json, url);
36+
37+
// process.binding() is deprecated (DEP0111) and trivially allows bypassing
38+
// policies, so if policies are enabled, make this API unavailable.
39+
process.binding = function binding(_module) {
40+
throw new ERR_ACCESS_DENIED('process.binding');
41+
};
42+
process._linkedBinding = function _linkedBinding(_module) {
43+
throw new ERR_ACCESS_DENIED('process._linkedBinding');
44+
};
3545
},
3646

3747
get manifest() {
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
'use strict';
2+
3+
const assert = require('assert');
4+
5+
assert.throws(() => { process.binding(); }, {
6+
code: 'ERR_ACCESS_DENIED'
7+
});
8+
assert.throws(() => { process._linkedBinding(); }, {
9+
code: 'ERR_ACCESS_DENIED'
10+
});
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
"resources": {
3+
"./app.js": {
4+
"integrity": true,
5+
"dependencies": {
6+
"assert": true
7+
}
8+
}
9+
}
10+
}
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
common.requireNoPackageJSONAbove();
5+
6+
if (!common.hasCrypto)
7+
common.skip('missing crypto');
8+
9+
const fixtures = require('../common/fixtures');
10+
11+
const assert = require('node:assert');
12+
const { spawnSync } = require('node:child_process');
13+
14+
const dep = fixtures.path('policy', 'process-binding', 'app.js');
15+
const depPolicy = fixtures.path(
16+
'policy',
17+
'process-binding',
18+
'policy.json');
19+
const { status } = spawnSync(
20+
process.execPath,
21+
[
22+
'--experimental-policy', depPolicy, dep,
23+
],
24+
{
25+
stdio: 'inherit'
26+
},
27+
);
28+
assert.strictEqual(status, 0);

0 commit comments

Comments
 (0)