mirror of
https://github.com/trezor/trezor-firmware.git
synced 2024-11-28 02:18:20 +00:00
fix(legacy): disallow using per-node paths
This commit is contained in:
parent
2322c6774d
commit
6548ca7921
@ -934,3 +934,27 @@ bool cryptoCosiVerify(const ed25519_signature signature, const uint8_t *message,
|
|||||||
res = ed25519_sign_open(message, message_len, pk_combined, signature);
|
res = ed25519_sign_open(message, message_len, pk_combined, signature);
|
||||||
return res == 0;
|
return res == 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
bool multisig_uses_single_path(const MultisigRedeemScriptType *multisig) {
|
||||||
|
if (multisig->pubkeys_count == 0) {
|
||||||
|
// Pubkeys are specified by multisig.nodes and multisig.address_n, in this
|
||||||
|
// case all the pubkeys use the same path
|
||||||
|
return true;
|
||||||
|
} else {
|
||||||
|
// Pubkeys are specified by multisig.pubkeys, in this case we check that all
|
||||||
|
// the pubkeys use the same path
|
||||||
|
for (int i = 0; i < multisig->pubkeys_count; i++) {
|
||||||
|
if (multisig->pubkeys[i].address_n_count !=
|
||||||
|
multisig->pubkeys[0].address_n_count) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
for (int j = 0; j < multisig->pubkeys[i].address_n_count; j++) {
|
||||||
|
if (multisig->pubkeys[i].address_n[j] !=
|
||||||
|
multisig->pubkeys[0].address_n[j]) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
@ -442,6 +442,17 @@ bool fsm_layoutPathWarning(void) {
|
|||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
bool fsm_layoutDifferentPathsWarning(void) {
|
||||||
|
layoutDialogSwipe(&bmp_icon_warning, _("Abort"), _("Continue"), NULL,
|
||||||
|
_("Ussing different paths"), _("for different XPUBs."),
|
||||||
|
NULL, _("Continue at your"), _("own risk!"), NULL);
|
||||||
|
if (!protectButton(ButtonRequestType_ButtonRequest_Warning, false)) {
|
||||||
|
fsm_sendFailure(FailureType_Failure_ActionCancelled, NULL);
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
#include "fsm_msg_coin.h"
|
#include "fsm_msg_coin.h"
|
||||||
#include "fsm_msg_common.h"
|
#include "fsm_msg_common.h"
|
||||||
#include "fsm_msg_crypto.h"
|
#include "fsm_msg_crypto.h"
|
||||||
|
@ -152,6 +152,7 @@ bool fsm_layoutSignMessage(const uint8_t *msg, uint32_t len);
|
|||||||
bool fsm_layoutVerifyMessage(const uint8_t *msg, uint32_t len);
|
bool fsm_layoutVerifyMessage(const uint8_t *msg, uint32_t len);
|
||||||
|
|
||||||
bool fsm_layoutPathWarning(void);
|
bool fsm_layoutPathWarning(void);
|
||||||
|
bool fsm_layoutDifferentPathsWarning(void);
|
||||||
bool fsm_checkCoinPath(const CoinInfo *coin, InputScriptType script_type,
|
bool fsm_checkCoinPath(const CoinInfo *coin, InputScriptType script_type,
|
||||||
uint32_t address_n_count, const uint32_t *address_n,
|
uint32_t address_n_count, const uint32_t *address_n,
|
||||||
bool has_multisig, MessageType message_type,
|
bool has_multisig, MessageType message_type,
|
||||||
|
@ -299,6 +299,27 @@ void fsm_msgGetAddress(const GetAddress *msg) {
|
|||||||
char desc[29] = {0};
|
char desc[29] = {0};
|
||||||
int multisig_index = 0;
|
int multisig_index = 0;
|
||||||
if (msg->has_multisig) {
|
if (msg->has_multisig) {
|
||||||
|
if (!multisig_uses_single_path(&(msg->multisig))) {
|
||||||
|
// An address that uses different derivation paths for different xpubs
|
||||||
|
// could be difficult to discover if the user did not note all the
|
||||||
|
// paths. The reason is that each path ends with an address index, which
|
||||||
|
// can have 1,000,000 possible values. If the address is a t-out-of-n
|
||||||
|
// multisig, the total number of possible paths is 1,000,000^n. This can
|
||||||
|
// be exploited by an attacker who has compromised the user's computer.
|
||||||
|
// The attacker could randomize the address indices and then demand a
|
||||||
|
// ransom from the user to reveal the paths. To prevent this, we require
|
||||||
|
// that all xpubs use the same derivation path.
|
||||||
|
if (config_getSafetyCheckLevel() == SafetyCheckLevel_Strict) {
|
||||||
|
fsm_sendFailure(
|
||||||
|
FailureType_Failure_DataError,
|
||||||
|
_("Using different paths for different xpubs is not allowed"
|
||||||
|
|
||||||
|
));
|
||||||
|
layoutHome();
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
fsm_layoutDifferentPathsWarning();
|
||||||
|
}
|
||||||
if (msg->multisig.has_pubkeys_order &&
|
if (msg->multisig.has_pubkeys_order &&
|
||||||
msg->multisig.pubkeys_order == MultisigPubkeysOrder_LEXICOGRAPHIC) {
|
msg->multisig.pubkeys_order == MultisigPubkeysOrder_LEXICOGRAPHIC) {
|
||||||
strlcpy(desc, "Multisig __ of __ (sorted):", sizeof(desc));
|
strlcpy(desc, "Multisig __ of __ (sorted):", sizeof(desc));
|
||||||
|
@ -2100,8 +2100,20 @@ static bool is_change_output(const TxInfo *tx_info,
|
|||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (txoutput->has_multisig && !check_change_multisig_fp(tx_info, txoutput)) {
|
if (txoutput->has_multisig) {
|
||||||
return false;
|
if (!(check_change_multisig_fp(tx_info, txoutput) &&
|
||||||
|
multisig_uses_single_path(&(txoutput->multisig)))) {
|
||||||
|
// An address that uses different derivation paths for different xpubs
|
||||||
|
// could be difficult to discover if the user did not note all the paths.
|
||||||
|
// The reason is that each path ends with an address index, which can have
|
||||||
|
// 1,000,000 possible values. If the address is a t-out-of-n multisig, the
|
||||||
|
// total number of possible paths is 1,000,000^n. This can be exploited by
|
||||||
|
// an attacker who has compromised the user's computer. The attacker could
|
||||||
|
// randomize the address indices and then demand a ransom from the user to
|
||||||
|
// reveal the paths. To prevent this, we require that all xpubs use the
|
||||||
|
// same derivation path.
|
||||||
|
return false;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
if (!check_change_multisig(tx_info, txoutput)) {
|
if (!check_change_multisig(tx_info, txoutput)) {
|
||||||
|
@ -437,7 +437,6 @@ def test_crw(client: Client):
|
|||||||
|
|
||||||
|
|
||||||
@pytest.mark.multisig
|
@pytest.mark.multisig
|
||||||
@pytest.mark.models(skip="legacy", reason="Not fixed")
|
|
||||||
def test_multisig_different_paths(client: Client):
|
def test_multisig_different_paths(client: Client):
|
||||||
nodes = [
|
nodes = [
|
||||||
btc.get_public_node(client, parse_path(f"m/45h/{i}"), coin_name="Bitcoin").node
|
btc.get_public_node(client, parse_path(f"m/45h/{i}"), coin_name="Bitcoin").node
|
||||||
|
@ -407,7 +407,6 @@ def test_multisig_mismatch_multisig_change(client: Client):
|
|||||||
|
|
||||||
|
|
||||||
# inputs match, change mismatches (second tries to be change but isn't)
|
# inputs match, change mismatches (second tries to be change but isn't)
|
||||||
@pytest.mark.models(skip="legacy", reason="Not fixed")
|
|
||||||
def test_multisig_mismatch_multisig_change_different_paths(client: Client):
|
def test_multisig_mismatch_multisig_change_different_paths(client: Client):
|
||||||
multisig_out2 = messages.MultisigRedeemScriptType(
|
multisig_out2 = messages.MultisigRedeemScriptType(
|
||||||
pubkeys=[
|
pubkeys=[
|
||||||
|
Loading…
Reference in New Issue
Block a user