Skip to content

Commit d4570fa

Browse files
tniessenRafaelGSS
authored andcommittedAug 8, 2023
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#460 CVE-ID: CVE-2023-32559
1 parent fe3abdf commit d4570fa

File tree

7 files changed

+73
-0
lines changed

7 files changed

+73
-0
lines changed
 

‎doc/api/deprecations.md

+4
Original file line numberDiff line numberDiff line change
@@ -2208,6 +2208,9 @@ Type: Documentation-only (supports [`--pending-deprecation`][])
22082208

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

2211+
While `process.binding()` has not reached End-of-Life status in general, it is
2212+
unavailable when [policies][] are enabled.
2213+
22112214
### DEP0112: `dgram` private APIs
22122215

22132216
<!-- YAML
@@ -3469,6 +3472,7 @@ In a future version of Node.js, [`message.headers`][],
34693472
[from_string_encoding]: buffer.md#static-method-bufferfromstring-encoding
34703473
[legacy URL API]: url.md#legacy-url-api
34713474
[legacy `urlObject`]: url.md#legacy-urlobject
3475+
[policies]: permissions.md#policies
34723476
[static methods of `crypto.Certificate()`]: crypto.md#class-certificate
34733477
[subpath exports]: packages.md#subpath-exports
34743478
[subpath imports]: packages.md#subpath-imports

‎doc/api/errors.md

+8
Original file line numberDiff line numberDiff line change
@@ -692,6 +692,14 @@ APIs _not_ using `AbortSignal`s typically do not raise an error with this code.
692692
This code does not use the regular `ERR_*` convention Node.js errors use in
693693
order to be compatible with the web platform's `AbortError`.
694694

695+
<a id="ERR_ACCESS_DENIED"></a>
696+
697+
### `ERR_ACCESS_DENIED`
698+
699+
A special type of error that is triggered whenever Node.js tries to get access
700+
to a resource restricted by the [policy][] manifest.
701+
For example, `process.binding`.
702+
695703
<a id="ERR_AMBIGUOUS_ARGUMENT"></a>
696704

697705
### `ERR_AMBIGUOUS_ARGUMENT`

‎lib/internal/errors.js

+3
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

+10
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() {
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+
});
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+
}
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)
Please sign in to comment.