arno
/
trezor-firmware
mirror of https://github.com/trezor/trezor-firmware.git
1
0
Fork 0
Code Issues Releases Wiki Activity

refactor(legacy): Refactor compile_input_script_sig().

Browse Source
pull/1926/head
Andrew Kozlik 3 years ago committed by Andrew Kozlik
parent 9a051df127
commit 6fbbd14f8c
5 changed files with 85 additions and 86 deletions
Show Stats Download Patch File Download Diff File

142
legacy/firmware/signing.c
Unescape View File

@ -656,16 +656,13 @@ bool check_change_bip32_path(const TxInfo *tx_info,
}
static bool fill_input_script_sig(TxInputType *tinput) {
memcpy(&node, &root, sizeof(HDNode));
if (hdnode_private_ckd_cached(&node, tinput->address_n,
tinput->address_n_count, NULL) == 0) {
// Failed to derive private key
return false;
}
if (hdnode_fill_public_key(&node) != 0) {
// Failed to derive public key
fsm_sendFailure(FailureType_Failure_ProcessError,
_("Failed to derive public key."));
signing_abort();
return false;
}
if (tinput->has_multisig) {
tinput->script_sig.size = compile_script_multisig(coin, &(tinput->multisig),
tinput->script_sig.bytes);
@ -675,33 +672,24 @@ static bool fill_input_script_sig(TxInputType *tinput) {
tinput->script_sig.size =
compile_script_sig(coin->address_type, hash, tinput->script_sig.bytes);
}
return tinput->script_sig.size > 0;
}
bool compile_input_script_sig(TxInputType *tinput) {
if (!info.multisig_fp_mismatch) {
// check that this is still multisig
uint8_t h[32] = {0};
if (!tinput->has_multisig ||
cryptoMultisigFingerprint(&(tinput->multisig), h) == 0 ||
memcmp(info.multisig_fp, h, 32) != 0) {
// Transaction has changed during signing
return false;
}
}
if (info.in_address_n_count != BIP32_NOCHANGEALLOWED) {
// check that input address didn't change
size_t count = tinput->address_n_count;
if (count < 2 || count != info.in_address_n_count ||
0 != memcmp(info.in_address_n, tinput->address_n,
(count - 2) * sizeof(uint32_t))) {
return false;
}
if (tinput->script_sig.size == 0) {
fsm_sendFailure(FailureType_Failure_ProcessError,
_("Failed to compile input."));
signing_abort();
return false;
}
return true;
}
static bool derive_node(TxInputType *tinput) {
if (!coin_path_check(coin, tinput->script_type, tinput->address_n_count,
tinput->address_n, tinput->has_multisig,
CoinPathCheckLevel_BASIC)) {
if (config_getSafetyCheckLevel() == SafetyCheckLevel_Strict) {
fsm_sendFailure(FailureType_Failure_DataError, _("Forbidden key path"));
signing_abort();
return false;
}
@ -711,11 +699,21 @@ bool compile_input_script_sig(TxInputType *tinput) {
if (!protectButton(ButtonRequestType_ButtonRequest_UnknownDerivationPath,
false)) {
fsm_sendFailure(FailureType_Failure_ActionCancelled, NULL);
layoutHome();
signing_abort();
return false;
}
}
return fill_input_script_sig(tinput);
memcpy(&node, &root, sizeof(HDNode));
if (hdnode_private_ckd_cached(&node, tinput->address_n,
tinput->address_n_count, NULL) == 0) {
fsm_sendFailure(FailureType_Failure_ProcessError,
_("Failed to derive private key."));
signing_abort();
return false;
}
return true;
}
static bool tx_info_init(TxInfo *tx_info, uint32_t inputs_count,
@ -1190,6 +1188,34 @@ static bool tx_info_add_input(TxInfo *tx_info, const TxInputType *txinput) {
return true;
}
static bool tx_info_check_input(TxInfo *tx_info, TxInputType *tinput) {
if (!tx_info->multisig_fp_mismatch) {
// check that this is still multisig
uint8_t h[32] = {0};
if (!tinput->has_multisig ||
cryptoMultisigFingerprint(&(tinput->multisig), h) == 0 ||
memcmp(tx_info->multisig_fp, h, 32) != 0) {
fsm_sendFailure(FailureType_Failure_ProcessError,
_("Transaction has changed during signing"));
signing_abort();
return false;
}
}
if (tx_info->in_address_n_count != BIP32_NOCHANGEALLOWED) {
// check that input address didn't change
size_t count = tinput->address_n_count;
if (count < 2 || count != tx_info->in_address_n_count ||
0 != memcmp(tx_info->in_address_n, tinput->address_n,
(count - 2) * sizeof(uint32_t))) {
fsm_sendFailure(FailureType_Failure_ProcessError,
_("Transaction has changed during signing"));
signing_abort();
return false;
}
}
return true;
}
static bool tx_info_add_output(TxInfo *tx_info,
const TxOutputBinType *tx_bin_output) {
// Add output to BIP-143/BIP-341 hashOutputs.
@ -1221,7 +1247,7 @@ static void tx_info_finish(TxInfo *tx_info) {
}
}
static bool signing_check_input(TxInputType *txinput) {
static bool signing_add_input(TxInputType *txinput) {
// hash all input data to check it later (relevant for fee computation)
tx_input_check_hash(&hasher_check, txinput);
@ -1319,7 +1345,7 @@ static bool is_change_output(const TxInfo *tx_info,
return check_change_bip32_path(tx_info, txoutput);
}
static bool signing_check_output(TxOutputType *txoutput) {
static bool signing_add_output(TxOutputType *txoutput) {
// Phase1: Check outputs
// add it to hash_outputs
// ask user for permission
@ -1429,7 +1455,7 @@ static bool save_signature(TxInputType *txinput) {
return true;
}
static bool signing_check_orig_input(TxInputType *orig_input) {
static bool signing_add_orig_input(TxInputType *orig_input) {
if (!fill_input_script_pubkey(coin, &root, orig_input)) {
fsm_sendFailure(FailureType_Failure_ProcessError,
_("Failed to derive scriptPubKey"));
@ -1491,10 +1517,7 @@ static bool signing_check_orig_input(TxInputType *orig_input) {
// (aka BIP-143 script code), which is what our code expects here in order
// to properly compute the legacy transaction digest or BIP-143 transaction
// digest.
if (!fill_input_script_sig(orig_input)) {
fsm_sendFailure(FailureType_Failure_ProcessError,
_("Failed to derive public key."));
signing_abort();
if (!derive_node(orig_input) || !fill_input_script_sig(orig_input)) {
return false;
}
@ -1516,7 +1539,7 @@ static bool signing_check_orig_input(TxInputType *orig_input) {
return true;
}
static bool signing_check_orig_output(TxOutputType *orig_output) {
static bool signing_add_orig_output(TxOutputType *orig_output) {
// Compute scriptPubKey.
TxOutputBinType orig_bin_output;
if (compile_output(coin, amount_unit, &root, orig_output, &orig_bin_output,
@ -2091,10 +2114,9 @@ static bool signing_sign_segwit_input(TxInputType *txinput) {
signing_abort();
return false;
}
if (!compile_input_script_sig(txinput)) {
fsm_sendFailure(FailureType_Failure_ProcessError,
_("Failed to compile input"));
signing_abort();
if (!tx_info_check_input(&info, txinput) || !derive_node(txinput) ||
!fill_input_script_sig(txinput)) {
return false;
}
@ -2194,7 +2216,7 @@ void signing_txack(TransactionType *tx) {
switch (signing_stage) {
case STAGE_REQUEST_1_INPUT:
if (!signing_validate_input(&tx->inputs[0]) ||
!signing_check_input(&tx->inputs[0])) {
!signing_add_input(&tx->inputs[0])) {
return;
}
@ -2304,7 +2326,7 @@ void signing_txack(TransactionType *tx) {
return;
case STAGE_REQUEST_1_ORIG_INPUT:
if (!signing_validate_input(tx->inputs) ||
!signing_check_orig_input(tx->inputs)) {
!signing_add_orig_input(tx->inputs)) {
return;
}
@ -2313,7 +2335,7 @@ void signing_txack(TransactionType *tx) {
return;
case STAGE_REQUEST_2_OUTPUT:
if (!signing_validate_output(&tx->outputs[0]) ||
!signing_check_output(&tx->outputs[0])) {
!signing_add_output(&tx->outputs[0])) {
return;
}
tx_weight += tx_output_weight(coin, &tx->outputs[0]);
@ -2327,7 +2349,7 @@ void signing_txack(TransactionType *tx) {
return;
case STAGE_REQUEST_2_ORIG_OUTPUT:
if (!signing_validate_output(tx->outputs) ||
!signing_check_orig_output(tx->outputs)) {
!signing_add_orig_output(tx->outputs)) {
return;
}
@ -2571,10 +2593,9 @@ void signing_txack(TransactionType *tx) {
// check inputs are the same as those in phase 1
tx_input_check_hash(&hasher_check, tx->inputs);
if (idx2 == idx1) {
if (!compile_input_script_sig(&tx->inputs[0])) {
fsm_sendFailure(FailureType_Failure_ProcessError,
_("Failed to compile input"));
signing_abort();
if (!tx_info_check_input(&info, &tx->inputs[0]) ||
!derive_node(&tx->inputs[0]) ||
!fill_input_script_sig(&tx->inputs[0])) {
return;
}
memcpy(&input, &tx->inputs[0], sizeof(input));
@ -2671,10 +2692,9 @@ void signing_txack(TransactionType *tx) {
signing_abort();
return;
}
if (!compile_input_script_sig(&tx->inputs[0])) {
fsm_sendFailure(FailureType_Failure_ProcessError,
_("Failed to compile input"));
signing_abort();
if (!tx_info_check_input(&info, &tx->inputs[0]) ||
!derive_node(&tx->inputs[0]) ||
!fill_input_script_sig(&tx->inputs[0])) {
return;
}
if (!tx->inputs[0].has_amount) {
@ -2711,10 +2731,9 @@ void signing_txack(TransactionType *tx) {
} else if (tx->inputs[0].script_type ==
InputScriptType_SPENDP2SHWITNESS &&
!tx->inputs[0].has_multisig) {
if (!compile_input_script_sig(&tx->inputs[0])) {
fsm_sendFailure(FailureType_Failure_ProcessError,
_("Failed to compile input"));
signing_abort();
if (!tx_info_check_input(&info, &tx->inputs[0]) ||
!derive_node(&tx->inputs[0]) ||
!fill_input_script_sig(&tx->inputs[0])) {
return;
}
// fixup normal p2pkh script into witness 0 p2wpkh script for p2sh
@ -2827,10 +2846,9 @@ void signing_txack(TransactionType *tx) {
coin->overwintered, info.version_group_id, info.timestamp);
ti.version |= (DECRED_SERIALIZE_WITNESS_SIGNING << 16);
ti.is_decred = true;
if (!compile_input_script_sig(&tx->inputs[0])) {
fsm_sendFailure(FailureType_Failure_ProcessError,
_("Failed to compile input"));
signing_abort();
if (!tx_info_check_input(&info, &tx->inputs[0]) ||
!derive_node(&tx->inputs[0]) ||
!fill_input_script_sig(&tx->inputs[0])) {
return;
}

7
tests/device_tests/test_msg_signtx.py
Unescape View File

@ -965,12 +965,7 @@ class TestMsgSigntx:
)
assert exc.value.code == messages.FailureType.ProcessError
if client.features.model == "1":
assert exc.value.message.endswith("Failed to compile input")
else:
assert exc.value.message.endswith(
"Transaction has changed during signing"
)
assert exc.value.message.endswith("Transaction has changed during signing")
def test_spend_coinbase(self, client):
inp1 = messages.TxInputType(

8
tests/device_tests/test_msg_signtx_invalid_path.py
Unescape View File

@ -61,12 +61,8 @@ class TestMsgSigntxInvalidPath:
with pytest.raises(TrezorFailure) as exc:
btc.sign_tx(client, "Litecoin", [inp1], [out1], prev_txes=TX_CACHE_MAINNET)
if client.features.model == "1":
assert exc.value.code == proto.FailureType.ProcessError
assert exc.value.message.endswith("Failed to compile input")
else:
assert exc.value.code == proto.FailureType.DataError
assert exc.value.message.endswith("Forbidden key path")
assert exc.value.code == proto.FailureType.DataError
assert exc.value.message.endswith("Forbidden key path")
# Adapted from TestMsgSigntx.test_one_one_fee,
# only changed the coin from Bitcoin to Litecoin and set safety checks to prompt.

7
tests/device_tests/test_msg_signtx_segwit.py
Unescape View File

@ -340,12 +340,7 @@ class TestMsgSigntxSegwit:
with pytest.raises(TrezorFailure) as exc:
btc.sign_tx(client, "Testnet", [inp1], [out1, out2], prev_txes=TX_API)
assert exc.value.code == proto.FailureType.ProcessError
if client.features.model == "1":
assert exc.value.message.endswith("Failed to compile input")
else:
assert exc.value.message.endswith(
"Transaction has changed during signing"
)
assert exc.value.message.endswith("Transaction has changed during signing")
def test_attack_mixed_inputs(self, client):
TRUE_AMOUNT = 123456789

7
tests/device_tests/test_multisig.py
Unescape View File

@ -333,9 +333,4 @@ class TestMultisig:
# 01000000000101396e2c107427f9eaece56a37539983adb8efd52b067c3d4567805fc8f3f7bffb01000000171600147a876a07b366f79000b441335f2907f777a0280bffffffff02e8030000000000001976a914e7c1345fc8f87c68170b3aa798a956c2fe6a9eff88ac703a0f000000000017a914a1261837f1b40e84346b1504ffe294e402965f2687024830450221009ff835e861be4e36ca1f2b6224aee2f253dfb9f456b13e4b1724bb4aaff4c9c802205e10679c2ead85743119f468cba5661f68b7da84dd2d477a7215fef98516f1f9012102af12ddd0d55e4fa2fcd084148eaf5b0b641320d0431d63d1e9a90f3cbd0d540700000000
assert exc.value.code == proto.FailureType.ProcessError
if client.features.model == "1":
assert exc.value.message.endswith("Failed to compile input")
else:
assert exc.value.message.endswith(
"Transaction has changed during signing"
)
assert exc.value.message.endswith("Transaction has changed during signing")

Write Preview
Loading…
Cancel
Save