Skip to content

Commit c357a39

Browse files
mcollinaRafaelGSS
authored andcommitted
tls: route callback exceptions through error handlers
Wrap pskCallback and ALPNCallback invocations in try-catch blocks to route exceptions through owner.destroy() instead of letting them become uncaught exceptions. This prevents remote attackers from crashing TLS servers or causing resource exhaustion. Fixes: https://hackerone.com/reports/3473882 PR-URL: nodejs-private/node-private#782 PR-URL: nodejs-private/node-private#790 CVE-ID: CVE-2026-21637
1 parent bdf5873 commit c357a39

File tree

3 files changed

+443
-81
lines changed

3 files changed

+443
-81
lines changed

lib/internal/tls/wrap.js

Lines changed: 87 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -233,39 +233,44 @@ function callALPNCallback(protocolsBuffer) {
233233
const handle = this;
234234
const socket = handle[owner_symbol];
235235

236-
const servername = handle.getServername();
236+
try {
237+
const servername = handle.getServername();
237238

238-
// Collect all the protocols from the given buffer:
239-
const protocols = [];
240-
let offset = 0;
241-
while (offset < protocolsBuffer.length) {
242-
const protocolLen = protocolsBuffer[offset];
243-
offset += 1;
239+
// Collect all the protocols from the given buffer:
240+
const protocols = [];
241+
let offset = 0;
242+
while (offset < protocolsBuffer.length) {
243+
const protocolLen = protocolsBuffer[offset];
244+
offset += 1;
244245

245-
const protocol = protocolsBuffer.slice(offset, offset + protocolLen);
246-
offset += protocolLen;
246+
const protocol = protocolsBuffer.slice(offset, offset + protocolLen);
247+
offset += protocolLen;
247248

248-
protocols.push(protocol.toString('ascii'));
249-
}
249+
protocols.push(protocol.toString('ascii'));
250+
}
250251

251-
const selectedProtocol = socket[kALPNCallback]({
252-
servername,
253-
protocols,
254-
});
252+
const selectedProtocol = socket[kALPNCallback]({
253+
servername,
254+
protocols,
255+
});
255256

256-
// Undefined -> all proposed protocols rejected
257-
if (selectedProtocol === undefined) return undefined;
257+
// Undefined -> all proposed protocols rejected
258+
if (selectedProtocol === undefined) return undefined;
258259

259-
const protocolIndex = protocols.indexOf(selectedProtocol);
260-
if (protocolIndex === -1) {
261-
throw new ERR_TLS_ALPN_CALLBACK_INVALID_RESULT(selectedProtocol, protocols);
262-
}
263-
let protocolOffset = 0;
264-
for (let i = 0; i < protocolIndex; i++) {
265-
protocolOffset += 1 + protocols[i].length;
266-
}
260+
const protocolIndex = protocols.indexOf(selectedProtocol);
261+
if (protocolIndex === -1) {
262+
throw new ERR_TLS_ALPN_CALLBACK_INVALID_RESULT(selectedProtocol, protocols);
263+
}
264+
let protocolOffset = 0;
265+
for (let i = 0; i < protocolIndex; i++) {
266+
protocolOffset += 1 + protocols[i].length;
267+
}
267268

268-
return protocolOffset;
269+
return protocolOffset;
270+
} catch (err) {
271+
socket.destroy(err);
272+
return undefined;
273+
}
269274
}
270275

271276
function requestOCSP(socket, info) {
@@ -372,63 +377,75 @@ function onnewsession(sessionId, session) {
372377

373378
function onPskServerCallback(identity, maxPskLen) {
374379
const owner = this[owner_symbol];
375-
const ret = owner[kPskCallback](owner, identity);
376-
if (ret == null)
377-
return undefined;
378380

379-
let psk;
380-
if (isArrayBufferView(ret)) {
381-
psk = ret;
382-
} else {
383-
if (typeof ret !== 'object') {
384-
throw new ERR_INVALID_ARG_TYPE(
385-
'ret',
386-
['Object', 'Buffer', 'TypedArray', 'DataView'],
387-
ret,
381+
try {
382+
const ret = owner[kPskCallback](owner, identity);
383+
if (ret == null)
384+
return undefined;
385+
386+
let psk;
387+
if (isArrayBufferView(ret)) {
388+
psk = ret;
389+
} else {
390+
if (typeof ret !== 'object') {
391+
throw new ERR_INVALID_ARG_TYPE(
392+
'ret',
393+
['Object', 'Buffer', 'TypedArray', 'DataView'],
394+
ret,
395+
);
396+
}
397+
psk = ret.psk;
398+
validateBuffer(psk, 'psk');
399+
}
400+
401+
if (psk.length > maxPskLen) {
402+
throw new ERR_INVALID_ARG_VALUE(
403+
'psk',
404+
psk,
405+
`Pre-shared key exceeds ${maxPskLen} bytes`,
388406
);
389407
}
390-
psk = ret.psk;
391-
validateBuffer(psk, 'psk');
392-
}
393408

394-
if (psk.length > maxPskLen) {
395-
throw new ERR_INVALID_ARG_VALUE(
396-
'psk',
397-
psk,
398-
`Pre-shared key exceeds ${maxPskLen} bytes`,
399-
);
409+
return psk;
410+
} catch (err) {
411+
owner.destroy(err);
412+
return undefined;
400413
}
401-
402-
return psk;
403414
}
404415

405416
function onPskClientCallback(hint, maxPskLen, maxIdentityLen) {
406417
const owner = this[owner_symbol];
407-
const ret = owner[kPskCallback](hint);
408-
if (ret == null)
409-
return undefined;
410418

411-
validateObject(ret, 'ret');
419+
try {
420+
const ret = owner[kPskCallback](hint);
421+
if (ret == null)
422+
return undefined;
423+
424+
validateObject(ret, 'ret');
425+
426+
validateBuffer(ret.psk, 'psk');
427+
if (ret.psk.length > maxPskLen) {
428+
throw new ERR_INVALID_ARG_VALUE(
429+
'psk',
430+
ret.psk,
431+
`Pre-shared key exceeds ${maxPskLen} bytes`,
432+
);
433+
}
412434

413-
validateBuffer(ret.psk, 'psk');
414-
if (ret.psk.length > maxPskLen) {
415-
throw new ERR_INVALID_ARG_VALUE(
416-
'psk',
417-
ret.psk,
418-
`Pre-shared key exceeds ${maxPskLen} bytes`,
419-
);
420-
}
435+
validateString(ret.identity, 'identity');
436+
if (Buffer.byteLength(ret.identity) > maxIdentityLen) {
437+
throw new ERR_INVALID_ARG_VALUE(
438+
'identity',
439+
ret.identity,
440+
`PSK identity exceeds ${maxIdentityLen} bytes`,
441+
);
442+
}
421443

422-
validateString(ret.identity, 'identity');
423-
if (Buffer.byteLength(ret.identity) > maxIdentityLen) {
424-
throw new ERR_INVALID_ARG_VALUE(
425-
'identity',
426-
ret.identity,
427-
`PSK identity exceeds ${maxIdentityLen} bytes`,
428-
);
444+
return { psk: ret.psk, identity: ret.identity };
445+
} catch (err) {
446+
owner.destroy(err);
447+
return undefined;
429448
}
430-
431-
return { psk: ret.psk, identity: ret.identity };
432449
}
433450

434451
function onkeylog(line) {

test/parallel/test-tls-alpn-server-client.js

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -253,25 +253,35 @@ function TestALPNCallback() {
253253
function TestBadALPNCallback() {
254254
// Server always returns a fixed invalid value:
255255
const serverOptions = {
256+
key: loadPEM('agent2-key'),
257+
cert: loadPEM('agent2-cert'),
256258
ALPNCallback: common.mustCall(() => 'http/5')
257259
};
258260

259-
const clientsOptions = [{
260-
ALPNProtocols: ['http/1', 'h2'],
261-
}];
261+
const server = tls.createServer(serverOptions);
262262

263-
process.once('uncaughtException', common.mustCall((error) => {
263+
// Error should be emitted via tlsClientError, not as uncaughtException
264+
server.on('tlsClientError', common.mustCall((error, socket) => {
264265
assert.strictEqual(error.code, 'ERR_TLS_ALPN_CALLBACK_INVALID_RESULT');
266+
socket.destroy();
265267
}));
266268

267-
runTest(clientsOptions, serverOptions, function(results) {
268-
// Callback returns 'http/5' => doesn't match client ALPN => error & reset
269-
assert.strictEqual(results[0].server, undefined);
270-
const allowedErrors = ['ECONNRESET', 'ERR_SSL_TLSV1_ALERT_NO_APPLICATION_PROTOCOL'];
271-
assert.ok(allowedErrors.includes(results[0].client.error.code), `'${results[0].client.error.code}' was not one of ${allowedErrors}.`);
269+
server.listen(0, serverIP, common.mustCall(() => {
270+
const client = tls.connect({
271+
port: server.address().port,
272+
host: serverIP,
273+
rejectUnauthorized: false,
274+
ALPNProtocols: ['http/1', 'h2'],
275+
}, common.mustNotCall());
272276

273-
TestALPNOptionsCallback();
274-
});
277+
client.on('error', common.mustCall((err) => {
278+
// Client gets reset when server handles error via tlsClientError
279+
const allowedErrors = ['ECONNRESET', 'ERR_SSL_TLSV1_ALERT_NO_APPLICATION_PROTOCOL'];
280+
assert.ok(allowedErrors.includes(err.code), `'${err.code}' was not one of ${allowedErrors}.`);
281+
server.close();
282+
TestALPNOptionsCallback();
283+
}));
284+
}));
275285
}
276286

277287
function TestALPNOptionsCallback() {

0 commit comments

Comments
 (0)