Skip to content

Commit 242aaa0

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 Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> CVE-ID: CVE-2023-32559 PR-URL: nodejs-private/node-private#459
1 parent 40c3958 commit 242aaa0

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
@@ -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
@@ -3340,6 +3343,7 @@ Node-API callbacks.
33403343
[from_arraybuffer]: buffer.md#static-method-bufferfromarraybuffer-byteoffset-length
33413344
[from_string_encoding]: buffer.md#static-method-bufferfromstring-encoding
33423345
[legacy `urlObject`]: url.md#legacy-urlobject
3346+
[policies]: permissions.md#policies
33433347
[static methods of `crypto.Certificate()`]: crypto.md#class-certificate
33443348
[subpath exports]: packages.md#subpath-exports
33453349
[subpath folder mappings]: packages.md#subpath-folder-mappings

‎doc/api/errors.md

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

682+
<a id="ERR_ACCESS_DENIED"></a>
683+
684+
### `ERR_ACCESS_DENIED`
685+
686+
A special type of error that is triggered whenever Node.js tries to get access
687+
to a resource restricted by the [policy][] manifest.
688+
For example, `process.binding`.
689+
682690
<a id="ERR_AMBIGUOUS_ARGUMENT"></a>
683691

684692
### `ERR_AMBIGUOUS_ARGUMENT`

‎lib/internal/errors.js

+3
Original file line numberDiff line numberDiff line change
@@ -934,6 +934,9 @@ module.exports = {
934934
//
935935
// Note: Node.js specific errors must begin with the prefix ERR_
936936

937+
E('ERR_ACCESS_DENIED',
938+
'Access to this API has been restricted. Permission: %s',
939+
Error);
937940
E('ERR_AMBIGUOUS_ARGUMENT', 'The "%s" argument is ambiguous. %s', TypeError);
938941
E('ERR_ARG_NOT_ITERABLE', '%s must be iterable', TypeError);
939942
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.