8000 tls: validate "rejectUnauthorized: undefined" · nodejs/node@1780bbc · GitHub
[go: up one dir, main page]
Skip to content

Commit 1780bbc

Browse files
mcollinaBethGriggs
mcollina
authored and
BethGriggs
committed
tls: validate "rejectUnauthorized: undefined"
Incomplete validation of rejectUnauthorized parameter (Low) If the Node.js https API was used incorrectly and "undefined" was passed in for the "rejectUnauthorized" parameter, no error was returned and connections to servers with an expired certificate would have been accepted. CVE-ID: CVE-2021-22939 Refs: https://nvd.nist.gov/vuln/detail/CVE-2021-22939 Refs: https://hackerone.com/reports/1278254 PR-URL: nodejs-private/node-private#276 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Akshay K <iit.akshay@gmail.com> Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Richard Lau <rlau@redhat.com>
1 parent 9cd1f53 commit 1780bbc

File tree

2 files changed

+29
-1
lines changed

2 files changed

+29
-1
lines changed

‎lib/_tls_wrap.js‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1516,7 +1516,15 @@ function onConnectSecure() {
15161516
this.authorized = false;
15171517
this.authorizationError = verifyError.code || verifyError.message;
15181518

1519-
if (options.rejectUnauthorized) {
1519+
// rejectUnauthorized property can be explicitly defined as `undefined`
1520+
// causing the assignment to default value (`true`) fail. Before assigning
1521+
// it to the tlssock connection options, explicitly check if it is false
1522+
// and update rejectUnauthorized property. The property gets used by
1523+
// TLSSocket conn 8000 ection handler to allow or reject connection if
1524+
// unauthorized.
1525+
// This check is potentially redundant, however it is better to keep it
1526+
// in case the option object gets modified somewhere.
1527+
if (options.rejectUnauthorized !== false) {
15201528
this.destroy(verifyError);
15211529
return;
15221530
}
@@ -1598,6 +1606,13 @@ exports.connect = function connect(...args) {
15981606
pskCallback: options.pskCallback,
15991607
});
16001608

1609+
// rejectUnauthorized property can be explicitly defined as `undefined`
1610+
// causing the assignment to default value (`true`) fail. Before assigning
1611+
// it to the tlssock connection options, explicitly check if it is false
1612+
// and update rejectUnauthorized property. The property gets used by TLSSocket
1613+
// connection handler to allow or reject connection if unauthorized
1614+
options.rejectUnauthorized = options.rejectUnauthorized !== false;
1615+
16011616
tlssock[kConnectOptions] = options;
16021617

16031618
if (cb)

‎test/parallel/test-tls-client-reject.js‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,19 @@ function rejectUnauthorized() {
7171
servername: 'localhost'
7272
}, common.mustNotCall());
7373
socket.on('data', common.mustNotCall());
74+
socket.on('error', common.mustCall(function(err) {
75+
rejectUnauthorizedUndefined();
76+
}));
77+
socket.end('ng');
78+
}
79+
80+
function rejectUnauthorizedUndefined() {
81+
console.log('reject unauthorized undefined');
82+
const socket = tls.connect(server.address().port, {
83+
servername: 'localhost',
84+
rejectUnauthorized: undefined
85+
}, common.mustNotCall());
86+
socket.on('data', common.mustNotCall());
7487
socket.on('error', common.mustCall(function(err) {
7588
authorized();
7689
}));

0 commit comments

Comments
 (0)
0