Skip to content

Commit f2cd3fc

Browse files
fix(sdk-core): validate password for UMKR share acceptance
1 parent 41375d2 commit f2cd3fc

3 files changed

Lines changed: 28 additions & 4 deletions

File tree

‎modules/sdk-core/src/bitgo/bitgoBase.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,5 +60,6 @@ export interface BitGoBase {
6060
options(url: string): BitGoRequest;
6161
setRequestTracer(reqTracer: IRequestTracer): void;
6262
url(path: string, version?: number): string;
63+
verifyPassword(params: { password: string }): Promise<boolean>;
6364
register(name: string, coin: CoinConstructor): void;
6465
}

‎modules/sdk-core/src/bitgo/wallet/wallets.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1358,8 +1358,11 @@ export class Wallets implements IWallets {
13581358
throw new Error('No valid wallet shares found to accept');
13591359
}
13601360

1361+
const hasSharesRequiringDecryption = walletShares.some(
1362+
(walletShare) => !walletShare.userMultiKeyRotationRequired && walletShare.keychain
1363+
);
13611364
let sharingKeychainPrv: string | undefined;
1362-
if (walletShares.some((walletShare) => !walletShare.userMultiKeyRotationRequired && walletShare.keychain)) {
1365+
if (hasSharesRequiringDecryption) {
13631366
const sharingKeychain = await this.bitgo.getECDHKeychain();
13641367
if (_.isUndefined(sharingKeychain.encryptedXprv)) {
13651368
throw new Error('encryptedXprv was not found on sharing keychain');
@@ -1369,6 +1372,8 @@ export class Wallets implements IWallets {
13691372
password: params.userLoginPassword,
13701373
input: sharingKeychain.encryptedXprv,
13711374
});
1375+
} else if (!(await this.bitgo.verifyPassword({ password: params.userLoginPassword }))) {
1376+
throw new Error('Incorrect password. Please try again.');
13721377
}
13731378
const newWalletPassphrase = params.newWalletPassphrase || params.userLoginPassword;
13741379
const webauthnInfo = params.webauthnInfo;

‎modules/sdk-core/test/unit/bitgo/wallet/walletsEncryptionVersion.ts‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ describe('Wallets - encryptionVersion threading', function () {
4242
.returns({ result: sinon.stub().resolves({ acceptedWalletShares: [], walletShareUpdateErrors: [] }) }),
4343
}),
4444
getECDHKeychain: sinon.stub().resolves({ encryptedXprv: 'encXprv' }),
45+
verifyPassword: sinon.stub().resolves(true),
4546
setRequestTracer: sinon.stub(),
4647
url: sinon.stub().returns('/test/url'),
4748
};
@@ -145,18 +146,34 @@ describe('Wallets - encryptionVersion threading', function () {
145146
assert.strictEqual(call.args[1], undefined);
146147
});
147148

148-
it('accepts a UMKR-only batch without fetching the ECDH keychain', async function () {
149-
mockBitGo.getECDHKeychain.resolves(new Error('missing'));
150-
149+
it('validates the login password without fetching the ECDH keychain for a UMKR-only batch', async function () {
151150
await wallets.bulkAcceptShare({
152151
walletShareIds: ['share-id'],
153152
userLoginPassword: 'login-password',
154153
});
155154

156155
assert.strictEqual(mockBitGo.getECDHKeychain.called, false);
156+
assert.deepStrictEqual(mockBitGo.verifyPassword.firstCall.args, [{ password: 'login-password' }]);
157157
assert.strictEqual(mockBitGo.put.called, true);
158158
});
159159

160+
it('rejects a UMKR-only batch when the login password is incorrect', async function () {
161+
mockBitGo.verifyPassword.resolves(false);
162+
163+
await assert.rejects(
164+
wallets.bulkAcceptShare({
165+
walletShareIds: ['share-id'],
166+
userLoginPassword: 'incorrect-password',
167+
}),
168+
{ message: 'Incorrect password. Please try again.' }
169+
);
170+
171+
assert.strictEqual(mockBitGo.getECDHKeychain.called, false);
172+
assert.strictEqual(mockBitGo.createEncryptionSession.called, false);
173+
assert.strictEqual(mockKeychains.create.called, false);
174+
assert.strictEqual(mockBitGo.put.called, false);
175+
});
176+
160177
it('still requires the ECDH keychain when the batch also holds a share with a keychain', async function () {
161178
mockBitGo.get.returns({
162179
result: sinon.stub().resolves({
@@ -187,6 +204,7 @@ describe('Wallets - encryptionVersion threading', function () {
187204
{ message: 'encryptedXprv was not found on sharing keychain' }
188205
);
189206
assert.strictEqual(mockBitGo.getECDHKeychain.calledOnce, true);
207+
assert.strictEqual(mockBitGo.verifyPassword.called, false);
190208
});
191209

192210
it('processes shares in batches of 16 to avoid WASM memory exhaustion', async function () {

0 commit comments

Comments
 (0)