Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Mnemonic + Password handling security #150

Merged
merged 2 commits into from Jul 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 14 additions & 7 deletions scripts/global.js
Expand Up @@ -10,7 +10,7 @@
decryptWallet,
getNewAddress,
getDerivationPath,
LegacyMasterKey,

Check warning on line 13 in scripts/global.js

View workflow job for this annotation

GitHub Actions / ESLint

scripts/global.js#L13

'LegacyMasterKey' is defined but never used (@typescript-eslint/no-unused-vars)
} from './wallet.js';
import { getNetwork, HistoricalTxType } from './network.js';
import {
Expand Down Expand Up @@ -1378,8 +1378,10 @@
* Imports a wallet using the GUI input, handling decryption via UI
*/
export async function guiImportWallet() {
const fEncrypted =
doms.domPrivKey.value.length >= 128 && isBase64(doms.domPrivKey.value);
// Important: These fields will be wiped by importWallet();
const strPrivKey = doms.domPrivKey.value;
const strPassword = doms.domPrivKeyPassword.value;
const fEncrypted = strPrivKey.length >= 128 && isBase64(strPrivKey);

// If we are in testnet: prompt an import
if (cChainParams.current.isTestnet) return importWallet();
Expand All @@ -1388,8 +1390,6 @@
if (!(await hasEncryptedWallet()) && !fEncrypted) return importWallet();

// If we don't have a DB wallet and the input is ciphered:
const strPrivKey = doms.domPrivKey.value;
const strPassword = doms.domPrivKeyPassword.value;
if (!(await hasEncryptedWallet()) && fEncrypted) {
const strDecWIF = await decrypt(strPrivKey, strPassword);
if (!strDecWIF || strDecWIF === 'decryption failed!') {
Expand All @@ -1407,6 +1407,9 @@
encWif: strPrivKey,
});
}
// Destroy residue import data
doms.domPrivKey.value = '';
doms.domPrivKeyPassword.value = '';
return;
}
}
Expand Down Expand Up @@ -1443,6 +1446,8 @@
createAlert('success', ALERTS.NEW_PASSWORD_SUCCESS, [], 5500);

$('#encryptWalletModal').modal('hide');
doms.domEncryptPasswordFirst.value = '';
doms.domEncryptPasswordSecond.value = '';

doms.domWipeWallet.hidden = false;
}
Expand Down Expand Up @@ -1727,10 +1732,12 @@
html: `${strHTML}<input type="password" id="restoreWalletPassword" placeholder="Wallet password" style="text-align: center;">`,
})
) {
// Fetch the password from the prompt, and immediately destroy the prompt input
const domPassword = document.getElementById('restoreWalletPassword');
const strPassword = domPassword.value;
domPassword.value = '';

// Attempt to unlock the wallet with the provided password
const strPassword = document.getElementById(
'restoreWalletPassword'
).value;
if (await decryptWallet(strPassword)) {
doms.domRestoreWallet.hidden = true;
doms.domWipeWallet.hidden = false;
Expand Down
4 changes: 4 additions & 0 deletions scripts/wallet.js
Expand Up @@ -584,7 +584,7 @@
fRaw = false,
isHardwareWallet = false,
skipConfirmation = false,
fSavePublicKey = false,

Check warning on line 587 in scripts/wallet.js

View workflow job for this annotation

GitHub Actions / ESLint

scripts/wallet.js#L587

'fSavePublicKey' is assigned a value but never used (@typescript-eslint/no-unused-vars)
fStartup = false,
} = {}) {
const strImportConfirm =
Expand Down Expand Up @@ -841,6 +841,10 @@
doms.domMnemonicModalButton.onclick = () => {
res(doms.domMnemonicModalPassphrase.value);
$('#mnemonicModal').modal('hide');

// Wipe the mnemonic displays of sensitive data
doms.domMnemonicModalContent.innerText = '';
doms.domMnemonicModalPassphrase.value = '';
};
$('#mnemonicModal').modal('show');
});
Expand Down