Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
mcollina authored and BethGriggs committed Aug 9, 2021
1 parent 31d5773 commit 6c7fff6
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 1 deletion.
17 changes: 16 additions & 1 deletion lib/_tls_wrap.js
Expand Up @@ -1544,7 +1544,15 @@ function onConnectSecure() {
this.authorized = false;
this.authorizationError = verifyError.code || verifyError.message;

if (options.rejectUnauthorized) {
// rejectUnauthorized property can be explicitly defined as `undefined`
// causing the assignment to default value (`true`) fail. Before assigning
// it to the tlssock connection options, explicitly check if it is false
// and update rejectUnauthorized property. The property gets used by
// TLSSocket connection handler to allow or reject connection if
// unauthorized.
// This check is potentially redundant, however it is better to keep it
// in case the option object gets modified somewhere.
if (options.rejectUnauthorized !== false) {
this.destroy(verifyError);
return;
}
Expand Down Expand Up @@ -1629,6 +1637,13 @@ exports.connect = function connect(...args) {
signal: options.signal,
});

// rejectUnauthorized property can be explicitly defined as `undefined`
// causing the assignment to default value (`true`) fail. Before assigning
// it to the tlssock connection options, explicitly check if it is false
// and update rejectUnauthorized property. The property gets used by TLSSocket
// connection handler to allow or reject connection if unauthorized
options.rejectUnauthorized = options.rejectUnauthorized !== false;

tlssock[kConnectOptions] = options;

if (cb)
Expand Down
13 changes: 13 additions & 0 deletions test/parallel/test-tls-client-reject.js
Expand Up @@ -71,6 +71,19 @@ function rejectUnauthorized() {
servername: 'localhost'
}, common.mustNotCall());
socket.on('data', common.mustNotCall());
socket.on('error', common.mustCall(function(err) {
rejectUnauthorizedUndefined();
}));
socket.end('ng');
}

function rejectUnauthorizedUndefined() {
console.log('reject unauthorized undefined');
const socket = tls.connect(server.address().port, {
servername: 'localhost',
rejectUnauthorized: undefined
}, common.mustNotCall());
socket.on('data', common.mustNotCall());
socket.on('error', common.mustCall(function(err) {
authorized();
}));
Expand Down

0 comments on commit 6c7fff6

Please sign in to comment.