Skip to content

Commit 20591b0

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#796 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> CVE-ID: CVE-2026-21637
1 parent ac03075 commit 20591b0

File tree

3 files changed

+442
-81
lines changed

3 files changed

+442
-81
lines changed

lib/_tls_wrap.js

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

237-
const servername = handle.getServername();
237+
try {
238+
const servername = handle.getServername();
238239

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;
240+
// Collect all the protocols from the given buffer:
241+
const protocols = [];
242+
let offset = 0;
243+
while (offset < protocolsBuffer.length) {
244+
const protocolLen = protocolsBuffer[offset];
245+
offset += 1;
245246

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

249-
protocols.push(protocol.toString('ascii'));
250-
}
250+
protocols.push(protocol.toString('ascii'));
251+
}
251252

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

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

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-
}
261+
const protocolIndex = protocols.indexOf(selectedProtocol);
262+
if (protocolIndex === -1) {
263+
throw new ERR_TLS_ALPN_CALLBACK_INVALID_RESULT(selectedProtocol, protocols);
264+
}
265+
let protocolOffset = 0;
266+
for (let i = 0; i < protocolIndex; i++) {
267+
protocolOffset += 1 + protocols[i].length;
268+
}
268269

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

272277
function requestOCSP(socket, info) {
@@ -373,63 +378,75 @@ function onnewsession(sessionId, session) {
373378

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

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

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

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

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

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

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

435452
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)