From 27b4481f10f4a8b1b45f0e7bdf70f48d5ec8ec37 Mon Sep 17 00:00:00 2001 From: Pranav Jain Date: Mon, 26 Oct 2026 13:53:24 -0600 Subject: [PATCH 1/4] fix(sdk-api): reject non-ccm mode before v1 SJCL decrypt SJCL reads the cipher mode from the envelope and implements the weak OCB2 mode. BitGo only wrote CCM, so refuse any other explicit mode before SJCL runs, in both the browser and Node paths. An absent mode still defaults to CCM. Ticket: WCN-3831 --- modules/sdk-api/src/encrypt.ts | 18 +++++++++++++++++ modules/sdk-api/test/unit/decryptV1.ts | 27 ++++++++++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/modules/sdk-api/src/encrypt.ts b/modules/sdk-api/src/encrypt.ts index f635711b8f6..5852a5c21a2 100644 --- a/modules/sdk-api/src/encrypt.ts +++ b/modules/sdk-api/src/encrypt.ts @@ -87,6 +87,23 @@ function isIterCapViolation(err: unknown): boolean { return err instanceof Error && /iter:\s*expected integer|iter out of range/i.test(err.message); } +/** + * SJCL reads the cipher mode from the envelope and also implements the weak OCB2 + * mode. BitGo only ever wrote CCM, so any other explicit mode is refused before + * SJCL sees it. An absent mode is SJCL's default (CCM) and is allowed. + */ +function assertCcmMode(ciphertext: string): void { + let mode: unknown; + try { + mode = JSON.parse(ciphertext)?.mode; + } catch { + return; // not JSON: let the decoder report it + } + if (mode !== undefined && mode !== 'ccm') { + throw new Error('v1 envelope: unsupported mode, expected ccm'); + } +} + /** * v1 decrypt with an SJCL safety net. * @@ -117,6 +134,7 @@ export async function decryptV1WithFallback( native: (pw: string, ct: string) => Promise = decryptV1, isBrowser: boolean = isBrowserRuntime() ): Promise { + assertCcmMode(ciphertext); if (isBrowser) { try { parseV1Envelope(ciphertext); diff --git a/modules/sdk-api/test/unit/decryptV1.ts b/modules/sdk-api/test/unit/decryptV1.ts index a5b8a5917db..114cb7fe8f8 100644 --- a/modules/sdk-api/test/unit/decryptV1.ts +++ b/modules/sdk-api/test/unit/decryptV1.ts @@ -221,5 +221,32 @@ describe('decryptV1 (native, SJCL-free)', () => { const ct = await encrypt(password, plaintext, { encryptionVersion: 1 }); await assert.rejects(() => decryptV1WithFallback('wrongPassword', ct, decryptV1, true)); }); + + it('rejects a non-ccm mode before SJCL runs', async () => { + const envelope = JSON.parse(await encrypt(password, plaintext, { encryptionVersion: 1 })); + envelope.mode = 'ocb2'; + await assert.rejects( + () => decryptV1WithFallback(password, JSON.stringify(envelope), decryptV1, true), + /unsupported mode/ + ); + }); + + it('still decrypts a legacy envelope without a v field', async () => { + const envelope = JSON.parse(await encrypt(password, plaintext, { encryptionVersion: 1 })); + delete envelope.v; + const result = await decryptV1WithFallback(password, JSON.stringify(envelope), decryptV1, true); + assert.strictEqual(result, plaintext); + }); + }); + + describe('cipher mode screen (node runtime)', () => { + it('rejects a non-ccm mode instead of falling back to SJCL', async () => { + const envelope = JSON.parse(await encrypt(password, plaintext, { encryptionVersion: 1 })); + envelope.mode = 'ocb2'; + await assert.rejects( + () => decryptV1WithFallback(password, JSON.stringify(envelope), decryptV1, false), + /unsupported mode/ + ); + }); }); }); From 622de58e35f08d29bfaf779ccb3c6f2c91428f9c Mon Sep 17 00:00:00 2001 From: Pranav Jain Date: Mon, 26 Oct 2026 13:53:56 -0600 Subject: [PATCH 2/4] fix(sdk-core): cap iter and mode in EdDSA v1 recovery decrypt Without a bitgo instance the recovery helpers called sjcl.decrypt directly, skipping the 100,000 iteration cap the main decrypt path enforces. Screen iter and mode before SJCL runs. Signatures are unchanged. Ticket: WCN-3831 --- .../src/bitgo/utils/tss/eddsa/eddsaMPCv2.ts | 38 +++++++++++++++++-- .../unit/bitgo/utils/tss/eddsa/eddsaMPCv2.ts | 30 +++++++++++++++ 2 files changed, 65 insertions(+), 3 deletions(-) diff --git a/modules/sdk-core/src/bitgo/utils/tss/eddsa/eddsaMPCv2.ts b/modules/sdk-core/src/bitgo/utils/tss/eddsa/eddsaMPCv2.ts index 411a5802877..2fd76feb9e9 100644 --- a/modules/sdk-core/src/bitgo/utils/tss/eddsa/eddsaMPCv2.ts +++ b/modules/sdk-core/src/bitgo/utils/tss/eddsa/eddsaMPCv2.ts @@ -1381,6 +1381,38 @@ export class EddsaMPCv2Utils extends BaseEddsaUtils { // #endregion } +/** + * Upper bound on PBKDF2 iterations accepted from a v1 envelope. Mirrors the cap in + * sdk-api (V1_MAX_ITER); BitGo only ever wrote 10,000. + */ +const LEGACY_V1_MAX_ITER = 100_000; + +/** + * v1-only decrypt used when no `bitgo` instance is available. SJCL reads `iter` and + * `mode` from the envelope and has no upper bound on either, so a tampered keycard + * could stall the caller with a huge PBKDF2 run or select the weak OCB2 mode. Refuse + * both before SJCL sees the envelope. Absent fields take SJCL's safe defaults. + */ +function decryptLegacyV1(walletPassphrase: string | undefined, ciphertext: string): string { + let envelope: { iter?: unknown; mode?: unknown } | undefined; + try { + envelope = JSON.parse(ciphertext); + } catch { + // not JSON: let SJCL report it + } + const iter = envelope?.iter; + if ( + iter !== undefined && + !(Number.isInteger(iter) && (iter as number) >= 1 && (iter as number) <= LEGACY_V1_MAX_ITER) + ) { + throw new Error(`v1 envelope: iter out of range, expected an integer between 1 and ${LEGACY_V1_MAX_ITER}`); + } + if (envelope?.mode !== undefined && envelope.mode !== 'ccm') { + throw new Error('v1 envelope: unsupported mode, expected ccm'); + } + return sjcl.decrypt(walletPassphrase, ciphertext); +} + /** * Detect whether an encrypted keycard contains MPCv1 signing material. * @@ -1417,7 +1449,7 @@ export async function isEddsaMpcV1SigningMaterial( ): Promise { const decrypted = bitgo ? await bitgo.decrypt({ input: encryptedKeyShare, password: walletPassphrase }) - : sjcl.decrypt(walletPassphrase, encryptedKeyShare); + : decryptLegacyV1(walletPassphrase, encryptedKeyShare); return parseEddsaMpcV1Material(decrypted) !== null; } @@ -1446,7 +1478,7 @@ export async function getEddsaMpcV2RecoveryKeySharesFromReducedKey( ): Promise<{ reduced: MPSTypes.EddsaReducedKeyShare; vrfKeyShare?: Buffer }> => { const decrypted = bitgo ? await bitgo.decrypt({ input: encryptedKey, password: walletPassphrase }) - : sjcl.decrypt(walletPassphrase, encryptedKey); + : decryptLegacyV1(walletPassphrase, encryptedKey); const parsed = parseMpcV2KeyShareEnvelope(decrypted); let reduced: MPSTypes.EddsaReducedKeyShare; try { @@ -1566,7 +1598,7 @@ export async function getEddsaSigningMaterial( try { decrypted = bitgo ? await bitgo.decrypt({ input: normalized, password: walletPassphrase }) - : sjcl.decrypt(walletPassphrase, normalized); + : decryptLegacyV1(walletPassphrase, normalized); } catch (e) { throw new Error(`Error decrypting user keychain: ${e instanceof Error ? e.message : String(e)}`); } diff --git a/modules/sdk-core/test/unit/bitgo/utils/tss/eddsa/eddsaMPCv2.ts b/modules/sdk-core/test/unit/bitgo/utils/tss/eddsa/eddsaMPCv2.ts index dd46d159e32..e4074e60ef1 100644 --- a/modules/sdk-core/test/unit/bitgo/utils/tss/eddsa/eddsaMPCv2.ts +++ b/modules/sdk-core/test/unit/bitgo/utils/tss/eddsa/eddsaMPCv2.ts @@ -393,6 +393,36 @@ describe('EdDSA MPS DSG helper functions', async () => { }); }); +describe('legacy v1 decrypt without a bitgo instance', () => { + const PASSPHRASE = 'test-passphrase'; + const tamper = (envelope: Record, change: Record): string => + JSON.stringify({ ...envelope, ...change }); + const valid = JSON.parse(sjcl.encrypt(PASSPHRASE, 'plaintext')); + + it('rejects an iter above the cap before running PBKDF2', async () => { + const start = Date.now(); + const tampered = tamper(valid, { iter: 100_001 }); + await assert.rejects(EDDSAUtils.isEddsaMpcV1SigningMaterial(tampered, PASSPHRASE), /iter out of range/); + await assert.rejects(EDDSAUtils.getEddsaSigningMaterial(tampered, PASSPHRASE), /iter out of range/); + await assert.rejects( + EDDSAUtils.getEddsaMpcV2RecoveryKeySharesFromReducedKey(tampered, tampered, PASSPHRASE), + /iter out of range/ + ); + assert.ok(Date.now() - start < 200, 'must reject before any KDF work'); + }); + + it('rejects a non-ccm mode', async () => { + await assert.rejects( + EDDSAUtils.isEddsaMpcV1SigningMaterial(tamper(valid, { mode: 'ocb2' }), PASSPHRASE), + /unsupported mode/ + ); + }); + + it('still decrypts an envelope with the default iter and mode', async () => { + assert.strictEqual(await EDDSAUtils.isEddsaMpcV1SigningMaterial(JSON.stringify(valid), PASSPHRASE), false); + }); +}); + describe('getEddsaMPCv2RecoveryKeyShares', () => { const walletPassphrase = 'testPass'; From c58ce6485872f14dcf517eed1393a5784614f8a2 Mon Sep 17 00:00:00 2001 From: Pranav Jain Date: Mon, 26 Oct 2026 14:07:49 -0600 Subject: [PATCH 3/4] fix(sdk-core): share v1 envelope screening with sdk-api Move the iter cap and ccm-only mode check into one sdk-core helper used by the EdDSA recovery fallback and sdk-api decrypt, so the two cannot drift. Drop a flaky timing assertion and cover non-integer iter and non-object input in tests. Ticket: WCN-3831 --- modules/sdk-api/src/decryptV1.ts | 4 +- modules/sdk-api/src/encrypt.ts | 20 +--------- modules/sdk-core/src/bitgo/utils/index.ts | 1 + .../src/bitgo/utils/legacyV1Envelope.ts | 37 +++++++++++++++++++ .../src/bitgo/utils/tss/eddsa/eddsaMPCv2.ts | 30 ++------------- .../unit/bitgo/utils/tss/eddsa/eddsaMPCv2.ts | 19 +++++++++- 6 files changed, 63 insertions(+), 48 deletions(-) create mode 100644 modules/sdk-core/src/bitgo/utils/legacyV1Envelope.ts diff --git a/modules/sdk-api/src/decryptV1.ts b/modules/sdk-api/src/decryptV1.ts index 24c3dc86b0f..06f316e30d4 100644 --- a/modules/sdk-api/src/decryptV1.ts +++ b/modules/sdk-api/src/decryptV1.ts @@ -1,4 +1,4 @@ -import { base64String, boundedInt, decodeWithCodec } from '@bitgo/sdk-core'; +import { base64String, boundedInt, decodeWithCodec, LEGACY_V1_MAX_ITER } from '@bitgo/sdk-core'; import { createDecipheriv, pbkdf2 } from 'crypto'; import * as t from 'io-ts'; import { promisify } from 'util'; @@ -8,7 +8,7 @@ import { promisify } from 'util'; * v1 envelopes use 10,000; this cap is 10x that. Envelope validation enforces * it up front before any KDF work runs. */ -export const V1_MAX_ITER = 100_000; +export const V1_MAX_ITER = LEGACY_V1_MAX_ITER; /** * io-ts codec for a v1 (SJCL) envelope. diff --git a/modules/sdk-api/src/encrypt.ts b/modules/sdk-api/src/encrypt.ts index 5852a5c21a2..a882075e6d6 100644 --- a/modules/sdk-api/src/encrypt.ts +++ b/modules/sdk-api/src/encrypt.ts @@ -1,6 +1,7 @@ import * as sjcl from '@bitgo/sjcl'; import { randomBytes } from 'crypto'; +import { assertSafeLegacyV1Envelope } from '@bitgo/sdk-core'; import { decryptV1, parseV1Envelope } from './decryptV1'; import { decryptV2, encryptV2 } from './encryptV2'; @@ -87,23 +88,6 @@ function isIterCapViolation(err: unknown): boolean { return err instanceof Error && /iter:\s*expected integer|iter out of range/i.test(err.message); } -/** - * SJCL reads the cipher mode from the envelope and also implements the weak OCB2 - * mode. BitGo only ever wrote CCM, so any other explicit mode is refused before - * SJCL sees it. An absent mode is SJCL's default (CCM) and is allowed. - */ -function assertCcmMode(ciphertext: string): void { - let mode: unknown; - try { - mode = JSON.parse(ciphertext)?.mode; - } catch { - return; // not JSON: let the decoder report it - } - if (mode !== undefined && mode !== 'ccm') { - throw new Error('v1 envelope: unsupported mode, expected ccm'); - } -} - /** * v1 decrypt with an SJCL safety net. * @@ -134,7 +118,7 @@ export async function decryptV1WithFallback( native: (pw: string, ct: string) => Promise = decryptV1, isBrowser: boolean = isBrowserRuntime() ): Promise { - assertCcmMode(ciphertext); + assertSafeLegacyV1Envelope(ciphertext); if (isBrowser) { try { parseV1Envelope(ciphertext); diff --git a/modules/sdk-core/src/bitgo/utils/index.ts b/modules/sdk-core/src/bitgo/utils/index.ts index 0a4f026543b..f4716aa5e1a 100644 --- a/modules/sdk-core/src/bitgo/utils/index.ts +++ b/modules/sdk-core/src/bitgo/utils/index.ts @@ -13,5 +13,6 @@ export * from './notEmpty'; export * from './wallet'; export * from './messageTypes'; export * from './siws'; +export * from './legacyV1Envelope'; export { openpgpUtils }; diff --git a/modules/sdk-core/src/bitgo/utils/legacyV1Envelope.ts b/modules/sdk-core/src/bitgo/utils/legacyV1Envelope.ts new file mode 100644 index 00000000000..8250d16c95a --- /dev/null +++ b/modules/sdk-core/src/bitgo/utils/legacyV1Envelope.ts @@ -0,0 +1,37 @@ +/** + * Upper bound on PBKDF2 iterations accepted from a legacy v1 (SJCL) envelope. + * BitGo only ever wrote 10,000; this cap is 10x that. + */ +export const LEGACY_V1_MAX_ITER = 100_000; + +/** + * Refuse hostile legacy v1 envelopes before SJCL sees them. SJCL reads `iter` and `mode` + * from the envelope and has no upper bound on the iteration count, so a tampered envelope + * could stall the caller with a huge PBKDF2 run or select the weak OCB2 mode. Absent + * `iter` and `mode` take SJCL's defaults. Input that is not a JSON object is left for the + * decoder to report. + * + * @param ciphertext serialized v1 envelope + * @throws if `iter` is not an integer between 1 and LEGACY_V1_MAX_ITER, or `mode` is not `ccm` + */ +export function assertSafeLegacyV1Envelope(ciphertext: string): void { + let envelope: { iter?: unknown; mode?: unknown } | null | undefined; + try { + envelope = JSON.parse(ciphertext); + } catch { + return; + } + if (typeof envelope !== 'object' || envelope === null) { + return; + } + const { iter, mode } = envelope; + if ( + iter !== undefined && + !(Number.isInteger(iter) && (iter as number) >= 1 && (iter as number) <= LEGACY_V1_MAX_ITER) + ) { + throw new Error(`v1 envelope: iter out of range, expected an integer between 1 and ${LEGACY_V1_MAX_ITER}`); + } + if (mode !== undefined && mode !== 'ccm') { + throw new Error('v1 envelope: unsupported mode, expected ccm'); + } +} diff --git a/modules/sdk-core/src/bitgo/utils/tss/eddsa/eddsaMPCv2.ts b/modules/sdk-core/src/bitgo/utils/tss/eddsa/eddsaMPCv2.ts index 2fd76feb9e9..15c441139ba 100644 --- a/modules/sdk-core/src/bitgo/utils/tss/eddsa/eddsaMPCv2.ts +++ b/modules/sdk-core/src/bitgo/utils/tss/eddsa/eddsaMPCv2.ts @@ -36,6 +36,7 @@ import { } from '../../../tss/eddsa/eddsaMPCv2'; import { getInitializedMpcInstance } from '../../../tss/eddsa/eddsa'; import { generateGPGKeyPair } from '../../opengpgUtils'; +import { assertSafeLegacyV1Envelope } from '../../legacyV1Envelope'; import { MPCv2PartiesEnum } from '../ecdsa/typesMPCv2'; import { CustomEddsaMPCv2SigningRound1GeneratingFunction, @@ -1382,34 +1383,11 @@ export class EddsaMPCv2Utils extends BaseEddsaUtils { } /** - * Upper bound on PBKDF2 iterations accepted from a v1 envelope. Mirrors the cap in - * sdk-api (V1_MAX_ITER); BitGo only ever wrote 10,000. - */ -const LEGACY_V1_MAX_ITER = 100_000; - -/** - * v1-only decrypt used when no `bitgo` instance is available. SJCL reads `iter` and - * `mode` from the envelope and has no upper bound on either, so a tampered keycard - * could stall the caller with a huge PBKDF2 run or select the weak OCB2 mode. Refuse - * both before SJCL sees the envelope. Absent fields take SJCL's safe defaults. + * v1-only decrypt used when no `bitgo` instance is available. Hostile envelopes are refused + * before SJCL runs. */ function decryptLegacyV1(walletPassphrase: string | undefined, ciphertext: string): string { - let envelope: { iter?: unknown; mode?: unknown } | undefined; - try { - envelope = JSON.parse(ciphertext); - } catch { - // not JSON: let SJCL report it - } - const iter = envelope?.iter; - if ( - iter !== undefined && - !(Number.isInteger(iter) && (iter as number) >= 1 && (iter as number) <= LEGACY_V1_MAX_ITER) - ) { - throw new Error(`v1 envelope: iter out of range, expected an integer between 1 and ${LEGACY_V1_MAX_ITER}`); - } - if (envelope?.mode !== undefined && envelope.mode !== 'ccm') { - throw new Error('v1 envelope: unsupported mode, expected ccm'); - } + assertSafeLegacyV1Envelope(ciphertext); return sjcl.decrypt(walletPassphrase, ciphertext); } diff --git a/modules/sdk-core/test/unit/bitgo/utils/tss/eddsa/eddsaMPCv2.ts b/modules/sdk-core/test/unit/bitgo/utils/tss/eddsa/eddsaMPCv2.ts index e4074e60ef1..97b4f62e456 100644 --- a/modules/sdk-core/test/unit/bitgo/utils/tss/eddsa/eddsaMPCv2.ts +++ b/modules/sdk-core/test/unit/bitgo/utils/tss/eddsa/eddsaMPCv2.ts @@ -32,6 +32,7 @@ import { getEddsaSigningMaterial, isEddsaSigningMaterial, signEddsaMpcV2RecoveryTx, + assertSafeLegacyV1Envelope, } from '../../../../../../src'; import { getSignatureShareRoundOne, @@ -400,7 +401,6 @@ describe('legacy v1 decrypt without a bitgo instance', () => { const valid = JSON.parse(sjcl.encrypt(PASSPHRASE, 'plaintext')); it('rejects an iter above the cap before running PBKDF2', async () => { - const start = Date.now(); const tampered = tamper(valid, { iter: 100_001 }); await assert.rejects(EDDSAUtils.isEddsaMpcV1SigningMaterial(tampered, PASSPHRASE), /iter out of range/); await assert.rejects(EDDSAUtils.getEddsaSigningMaterial(tampered, PASSPHRASE), /iter out of range/); @@ -408,7 +408,16 @@ describe('legacy v1 decrypt without a bitgo instance', () => { EDDSAUtils.getEddsaMpcV2RecoveryKeySharesFromReducedKey(tampered, tampered, PASSPHRASE), /iter out of range/ ); - assert.ok(Date.now() - start < 200, 'must reject before any KDF work'); + }); + + it('rejects an iter that is not an integer within range', async () => { + for (const iter of ['10000', 1.5, 0, -1]) { + await assert.rejects( + EDDSAUtils.isEddsaMpcV1SigningMaterial(tamper(valid, { iter }), PASSPHRASE), + /iter out of range/, + `iter ${JSON.stringify(iter)} must be rejected` + ); + } }); it('rejects a non-ccm mode', async () => { @@ -418,6 +427,12 @@ describe('legacy v1 decrypt without a bitgo instance', () => { ); }); + it('leaves non-object input for the decoder to report', () => { + for (const input of ['not json', 'null', '"a string"', '42']) { + assert.doesNotThrow(() => assertSafeLegacyV1Envelope(input), `input ${input} must pass through`); + } + }); + it('still decrypts an envelope with the default iter and mode', async () => { assert.strictEqual(await EDDSAUtils.isEddsaMpcV1SigningMaterial(JSON.stringify(valid), PASSPHRASE), false); }); From 154d18a0dc46214c26a7cbb3f0f3646460a6f5bb Mon Sep 17 00:00:00 2001 From: Pranav Jain Date: Mon, 26 Oct 2026 14:12:15 -0600 Subject: [PATCH 4/4] fix(sdk-core): screen the envelope SJCL actually decodes SJCL strips all whitespace before decoding, so a key like "it er" is iter to SJCL but unknown to JSON.parse, which let a hostile iter or mode past the guards. Read the envelope with sjcl.json.decode instead, in the shared helper used by sdk-api and the EdDSA recovery fallback. Ticket: WCN-3831 --- modules/sdk-api/test/unit/decryptV1.ts | 20 +++++++++++++++++++ .../src/bitgo/utils/legacyV1Envelope.ts | 16 ++++++++------- .../unit/bitgo/utils/tss/eddsa/eddsaMPCv2.ts | 19 ++++++++++++++++-- 3 files changed, 46 insertions(+), 9 deletions(-) diff --git a/modules/sdk-api/test/unit/decryptV1.ts b/modules/sdk-api/test/unit/decryptV1.ts index 114cb7fe8f8..2f2ae5e499f 100644 --- a/modules/sdk-api/test/unit/decryptV1.ts +++ b/modules/sdk-api/test/unit/decryptV1.ts @@ -231,6 +231,26 @@ describe('decryptV1 (native, SJCL-free)', () => { ); }); + it('rejects a hostile iter or mode hidden by whitespace in the key', async () => { + const ct = await encrypt(password, plaintext, { encryptionVersion: 1 }); + const spacedIter = ct.replace(/"iter":\d+/, '"it er":3000000'); + const spacedMode = ct.replace('"mode":"ccm"', '"mo de":"ocb2"'); + assert.notStrictEqual(spacedIter, ct); + assert.notStrictEqual(spacedMode, ct); + for (const isBrowser of [true, false]) { + const start = Date.now(); + await assert.rejects( + () => decryptV1WithFallback(password, spacedIter, decryptV1, isBrowser), + /iter out of range/ + ); + await assert.rejects( + () => decryptV1WithFallback(password, spacedMode, decryptV1, isBrowser), + /unsupported mode/ + ); + assert.ok(Date.now() - start < 2000, 'must not run the inflated PBKDF2'); + } + }); + it('still decrypts a legacy envelope without a v field', async () => { const envelope = JSON.parse(await encrypt(password, plaintext, { encryptionVersion: 1 })); delete envelope.v; diff --git a/modules/sdk-core/src/bitgo/utils/legacyV1Envelope.ts b/modules/sdk-core/src/bitgo/utils/legacyV1Envelope.ts index 8250d16c95a..8ce18fa0008 100644 --- a/modules/sdk-core/src/bitgo/utils/legacyV1Envelope.ts +++ b/modules/sdk-core/src/bitgo/utils/legacyV1Envelope.ts @@ -1,3 +1,5 @@ +import * as sjcl from '@bitgo/sjcl'; + /** * Upper bound on PBKDF2 iterations accepted from a legacy v1 (SJCL) envelope. * BitGo only ever wrote 10,000; this cap is 10x that. @@ -8,22 +10,22 @@ export const LEGACY_V1_MAX_ITER = 100_000; * Refuse hostile legacy v1 envelopes before SJCL sees them. SJCL reads `iter` and `mode` * from the envelope and has no upper bound on the iteration count, so a tampered envelope * could stall the caller with a huge PBKDF2 run or select the weak OCB2 mode. Absent - * `iter` and `mode` take SJCL's defaults. Input that is not a JSON object is left for the - * decoder to report. + * `iter` and `mode` take SJCL's defaults. + * + * The check reads the envelope with SJCL's own decoder, not JSON.parse. SJCL strips all + * whitespace before decoding, so a key such as "it er" is `iter` to SJCL but unknown to + * JSON.parse. Input SJCL cannot decode is left for `sjcl.decrypt` to report. * * @param ciphertext serialized v1 envelope * @throws if `iter` is not an integer between 1 and LEGACY_V1_MAX_ITER, or `mode` is not `ccm` */ export function assertSafeLegacyV1Envelope(ciphertext: string): void { - let envelope: { iter?: unknown; mode?: unknown } | null | undefined; + let envelope: { iter?: unknown; mode?: unknown }; try { - envelope = JSON.parse(ciphertext); + envelope = sjcl.json.decode(ciphertext); } catch { return; } - if (typeof envelope !== 'object' || envelope === null) { - return; - } const { iter, mode } = envelope; if ( iter !== undefined && diff --git a/modules/sdk-core/test/unit/bitgo/utils/tss/eddsa/eddsaMPCv2.ts b/modules/sdk-core/test/unit/bitgo/utils/tss/eddsa/eddsaMPCv2.ts index 97b4f62e456..a5ff5c3879e 100644 --- a/modules/sdk-core/test/unit/bitgo/utils/tss/eddsa/eddsaMPCv2.ts +++ b/modules/sdk-core/test/unit/bitgo/utils/tss/eddsa/eddsaMPCv2.ts @@ -411,13 +411,18 @@ describe('legacy v1 decrypt without a bitgo instance', () => { }); it('rejects an iter that is not an integer within range', async () => { - for (const iter of ['10000', 1.5, 0, -1]) { + for (const iter of ['10000', 0]) { await assert.rejects( EDDSAUtils.isEddsaMpcV1SigningMaterial(tamper(valid, { iter }), PASSPHRASE), /iter out of range/, `iter ${JSON.stringify(iter)} must be rejected` ); } + + // SJCL's decoder only accepts non-negative integers, so these never reach PBKDF2. + for (const iter of [1.5, -1]) { + await assert.rejects(EDDSAUtils.isEddsaMpcV1SigningMaterial(tamper(valid, { iter }), PASSPHRASE)); + } }); it('rejects a non-ccm mode', async () => { @@ -427,7 +432,17 @@ describe('legacy v1 decrypt without a bitgo instance', () => { ); }); - it('leaves non-object input for the decoder to report', () => { + it('screens the envelope SJCL decodes, not the one JSON.parse sees', () => { + // SJCL strips all whitespace before decoding, so these keys are iter and mode to SJCL. + const base = JSON.stringify(valid); + const spacedIter = base.replace(/"iter":\d+/, '"it er":3000000'); + const spacedMode = base.replace('"mode":"ccm"', '"mo de":"ocb2"'); + assert.strictEqual(JSON.parse(spacedIter).iter, undefined); + assert.throws(() => assertSafeLegacyV1Envelope(spacedIter), /iter out of range/); + assert.throws(() => assertSafeLegacyV1Envelope(spacedMode), /unsupported mode/); + }); + + it('leaves input SJCL cannot decode for sjcl.decrypt to report', () => { for (const input of ['not json', 'null', '"a string"', '42']) { assert.doesNotThrow(() => assertSafeLegacyV1Envelope(input), `input ${input} must pass through`); }