Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 25d6799

Browse filesBrowse files
mcollinamarco-ippolito
mcollina
authored and
marco-ippolito
committed
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#796 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> CVE-ID: CVE-2026-21637
1 parent 37509c3 commit 25d6799
Copy full SHA for 25d6799

File tree

Expand file treeCollapse file tree

3 files changed

+442
-81
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

3 files changed

+442
-81
lines changed
Open diff view settings
Collapse file

‎lib/_tls_wrap.js‎

Copy file name to clipboardExpand all lines: lib/_tls_wrap.js
+87-70Lines changed: 87 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -235,39 +235,44 @@ function callALPNCallback(protocolsBuffer) {
235235
const handle = this;
236236
const socket = handle[owner_symbol];
237237

238-
const servername = handle.getServername();
238+
try {
239+
const servername = handle.getServername();
239240

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

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

250-
protocols.push(protocol.toString('ascii'));
251-
}
251+
protocols.push(protocol.toString('ascii'));
252+
}
252253

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

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

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

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

273278
function requestOCSP(socket, info) {
@@ -374,63 +379,75 @@ function onnewsession(sessionId, session) {
374379

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

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

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

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

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

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

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

436453
function onkeylog(line) {
Collapse file

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

Copy file name to clipboardExpand all lines: test/parallel/test-tls-alpn-server-client.js
+21-11Lines 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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.