From 9a977ed3bd622d32ff48e9cecc215c81c6415d44 Mon Sep 17 00:00:00 2001 From: Nick Guenther Date: Fri, 30 May 2025 20:22:08 -0400 Subject: [PATCH 1/3] Handle incorrect SASL s A malicious/misbehaving server can crash xmpp.js here by offering FAST but then responding to the `` with a `` instead of a `` or ``. That would cause xmpp.js to run ``` await mech.challenge(decode(element.text())); ``` but `mech.challenge` isn't defined for HT-SHA-256-NONE. --- packages/sasl2/index.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/sasl2/index.js b/packages/sasl2/index.js index 7cb819754..b05f5d055 100644 --- a/packages/sasl2/index.js +++ b/packages/sasl2/index.js @@ -46,6 +46,9 @@ async function authenticate({ if (element.getNS() !== NS) return; if (element.name === "challenge") { + if (!mech.challenge) { + throw new Error("${mech.name} does not support SASL challenges"); + } await mech.challenge(decode(element.text())); const resp = await mech.response(creds); await entity.send( From 4b8e2dad35e98cdae08e8d109269c050c1260024 Mon Sep 17 00:00:00 2001 From: kousu Date: Wed, 11 Jun 2025 11:10:58 -0400 Subject: [PATCH 2/3] Test incorrect SASL s during FAST --- packages/sasl-ht-sha-256-none/index.js | 4 ++ packages/sasl2/index.js | 10 +++-- packages/sasl2/test.js | 53 ++++++++++++++++++++++++++ 3 files changed, 63 insertions(+), 4 deletions(-) diff --git a/packages/sasl-ht-sha-256-none/index.js b/packages/sasl-ht-sha-256-none/index.js index 618794488..87acdae4d 100644 --- a/packages/sasl-ht-sha-256-none/index.js +++ b/packages/sasl-ht-sha-256-none/index.js @@ -9,6 +9,10 @@ Mechanism.prototype.Mechanism = Mechanism; Mechanism.prototype.name = "HT-SHA-256-NONE"; Mechanism.prototype.clientFirst = true; +Mechanism.prototype.challenge = async function challenge(_) { + throw new Error(`${this.name} does not support SASL challenges`); +} + Mechanism.prototype.response = async function response({ username, password }) { this.key = await crypto.subtle.importKey( "raw", diff --git a/packages/sasl2/index.js b/packages/sasl2/index.js index b05f5d055..6c3092834 100644 --- a/packages/sasl2/index.js +++ b/packages/sasl2/index.js @@ -46,9 +46,6 @@ async function authenticate({ if (element.getNS() !== NS) return; if (element.name === "challenge") { - if (!mech.challenge) { - throw new Error("${mech.name} does not support SASL challenges"); - } await mech.challenge(decode(element.text())); const resp = await mech.response(creds); await entity.send( @@ -118,6 +115,11 @@ export default function sasl2({ streamFeatures, saslFactory }, onAuthenticate) { async function done(credentials, mechanism, userAgent) { // Try fast + let err; + entity.on("error", (_err) => { + // catch internal/protocol errors in fast.auth() + err = _err; + }) const success = await fast.auth({ authenticate, entity, @@ -126,7 +128,7 @@ export default function sasl2({ streamFeatures, saslFactory }, onAuthenticate) { features, credentials, }); - if (success) return; + if (success || err) return; // fast.auth may mutate streamFeatures to request a token diff --git a/packages/sasl2/test.js b/packages/sasl2/test.js index 07c4ce805..bdbd2718c 100644 --- a/packages/sasl2/test.js +++ b/packages/sasl2/test.js @@ -224,3 +224,56 @@ test("use ANONYMOUS if username and password are not provided", async () => { const result = await promise(entity, "send"); expect(result.attrs.mechanism).toEqual("ANONYMOUS"); }); + +test("fail if server sends a challenge during FAST login", async () => { + const mech = "HT-SHA-256-NONE"; + + function onAuthenticate(authenticate, mechanisms, fast) { + expect(mechanisms).toEqual([]); + expect(fast.mechanism).toEqual(mech); + return authenticate( + { + token: { + token: "hai", + mechanism: fast.mechanism, + }, + }, + null, + userAgent, + ); + } + + const { entity } = mockClient({ credentials: onAuthenticate }); + + entity.mockInput( + + + + + {mech} + + + + , + ); + + expect(await promise(entity, "send")).toEqual( + + + bnVsbACNMNimsTBnxS04m8x7wgKjBHdDUL/nXPU4J4vqxqjBIg== + + {userAgent} + + , + ); + + entity.mockInput( + + aGVyZXNhYnVuY2hvZmJhZGNoYWxsZW5nZWRhdGEK + , + ); + + const error = await promise(entity, "error"); + expect(error instanceof Error).toBe(true); + expect(error.message).toBe("HT-SHA-256-NONE does not support SASL challenges"); +}); \ No newline at end of file From 1236d1d1876340ea70d261d4c850dbddd17bfbaf Mon Sep 17 00:00:00 2001 From: kousu Date: Wed, 11 Jun 2025 11:40:46 -0400 Subject: [PATCH 3/3] Newline --- packages/sasl2/test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/sasl2/test.js b/packages/sasl2/test.js index bdbd2718c..f608573c7 100644 --- a/packages/sasl2/test.js +++ b/packages/sasl2/test.js @@ -276,4 +276,4 @@ test("fail if server sends a challenge during FAST login", async () => { const error = await promise(entity, "error"); expect(error instanceof Error).toBe(true); expect(error.message).toBe("HT-SHA-256-NONE does not support SASL challenges"); -}); \ No newline at end of file +});