Skip to content

Commit da89a17

Browse files
matejciktsusanka
authored andcommitted
all: add checks for prev_hash size
1 parent c15519f commit da89a17

File tree

3 files changed

+173
-0
lines changed

3 files changed

+173
-0
lines changed

core/src/apps/wallet/sign_tx/helpers.py

+3
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
from trezor.messages.TxRequest import TxRequest
1818

1919
from .signing import SigningError
20+
from .writers import TX_HASH_SIZE
2021

2122
from apps.common.coininfo import CoinInfo
2223

@@ -213,6 +214,8 @@ def sanitize_tx_input(tx: TransactionType, coin: CoinInfo) -> TxInputType:
213214
txi.script_type = InputScriptType.SPENDADDRESS
214215
if txi.sequence is None:
215216
txi.sequence = 0xFFFFFFFF
217+
if txi.prev_hash is None or len(txi.prev_hash) != TX_HASH_SIZE:
218+
raise SigningError(FailureType.DataError, "Provided prev_hash is invalid.")
216219
if txi.multisig and txi.script_type not in MULTISIG_INPUT_SCRIPT_TYPES:
217220
raise SigningError(
218221
FailureType.DataError, "Multisig field provided but not expected.",

legacy/firmware/signing.c

+6
Original file line numberDiff line numberDiff line change
@@ -588,6 +588,12 @@ void signing_init(const SignTx *msg, const CoinInfo *_coin,
588588
#define MIN(a, b) (((a) < (b)) ? (a) : (b))
589589

590590
static bool signing_validate_input(const TxInputType *txinput) {
591+
if (txinput->prev_hash.size != 32) {
592+
fsm_sendFailure(FailureType_Failure_ProcessError,
593+
_("Encountered invalid prevhash"));
594+
signing_abort();
595+
return false;
596+
}
591597
if (txinput->has_multisig &&
592598
(txinput->script_type != InputScriptType_SPENDMULTISIG &&
593599
txinput->script_type != InputScriptType_SPENDP2SHWITNESS &&
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,164 @@
1+
from hashlib import sha256
2+
from io import BytesIO
3+
4+
import pytest
5+
6+
from trezorlib import btc, messages
7+
from trezorlib.exceptions import TrezorFailure
8+
9+
from ..tx_cache import tx_cache
10+
11+
TXHASH_157041 = bytes.fromhex(
12+
"1570416eb4302cf52979afd5e6909e37d8fdd874301f7cc87e547e509cb1caa6"
13+
)
14+
15+
16+
def write_prefixed_bytes(io, data) -> None:
17+
assert len(data) < 253
18+
io.write(len(data).to_bytes(1, "little"))
19+
io.write(data)
20+
21+
22+
def serialize_input(tx_input) -> bytes:
23+
"""serialize for Bitcoin tx format"""
24+
b = BytesIO()
25+
if tx_input.prev_hash:
26+
b.write(tx_input.prev_hash[::-1])
27+
b.write(tx_input.prev_index.to_bytes(4, "little"))
28+
write_prefixed_bytes(b, tx_input.script_sig)
29+
b.write(tx_input.sequence.to_bytes(4, "little"))
30+
return b.getvalue()
31+
32+
33+
def serialize_bin_output(tx_output) -> bytes:
34+
b = BytesIO()
35+
b.write(tx_output.amount.to_bytes(8, "little"))
36+
write_prefixed_bytes(b, tx_output.script_pubkey)
37+
return b.getvalue()
38+
39+
40+
def serialize_tx(tx) -> bytes:
41+
b = BytesIO()
42+
b.write(tx.version.to_bytes(4, "little"))
43+
assert len(tx.inputs) < 253
44+
b.write(len(tx.inputs).to_bytes(1, "little"))
45+
for inp in tx.inputs:
46+
b.write(serialize_input(inp))
47+
assert len(tx.bin_outputs) < 253
48+
b.write(len(tx.bin_outputs).to_bytes(1, "little"))
49+
for outp in tx.bin_outputs:
50+
b.write(serialize_bin_output(outp))
51+
b.write(tx.lock_time.to_bytes(4, "little"))
52+
if tx.extra_data:
53+
b.write(tx.extra_data)
54+
return b.getvalue()
55+
56+
57+
def hash_tx(data: bytes) -> bytes:
58+
return sha256(sha256(data).digest()).digest()[::-1]
59+
60+
61+
def _check_error_message(value: bytes, model: str, message: str):
62+
if model != "1":
63+
assert message == "Provided prev_hash is invalid."
64+
return
65+
66+
# T1 has several possible errors
67+
if value is None:
68+
assert message.endswith("missing required field")
69+
elif len(value) > 32:
70+
assert message.endswith("bytes overflow")
71+
else:
72+
assert message.endswith("Encountered invalid prevhash")
73+
74+
75+
@pytest.mark.skip_ui
76+
@pytest.mark.parametrize("prev_hash", (None, b"", b"x", b"hello world", b"x" * 33))
77+
def test_invalid_prev_hash(client, prev_hash):
78+
inp1 = messages.TxInputType(
79+
address_n=[0],
80+
amount=123456789,
81+
prev_hash=prev_hash,
82+
prev_index=0,
83+
script_type=messages.InputScriptType.SPENDP2SHWITNESS,
84+
)
85+
out1 = messages.TxOutputType(
86+
address="mhRx1CeVfaayqRwq5zgRQmD7W5aWBfD5mC",
87+
amount=12300000,
88+
script_type=messages.OutputScriptType.PAYTOADDRESS,
89+
)
90+
91+
with pytest.raises(TrezorFailure) as e:
92+
btc.sign_tx(client, "Testnet", [inp1], [out1])
93+
_check_error_message(prev_hash, client.features.model, e.value.failure.message)
94+
95+
96+
@pytest.mark.skip_ui
97+
@pytest.mark.parametrize("prev_hash", (None, b"", b"x", b"hello world", b"x" * 33))
98+
def test_invalid_prev_hash_attack(client, prev_hash):
99+
# prepare input with a valid prev-hash
100+
inp1 = messages.TxInputType(
101+
address_n=[0],
102+
amount=123456789,
103+
prev_hash=b"\x00" * 32,
104+
prev_index=0,
105+
script_type=messages.InputScriptType.SPENDP2SHWITNESS,
106+
)
107+
out1 = messages.TxOutputType(
108+
address="mhRx1CeVfaayqRwq5zgRQmD7W5aWBfD5mC",
109+
amount=12300000,
110+
script_type=messages.OutputScriptType.PAYTOADDRESS,
111+
)
112+
113+
counter = 1
114+
115+
def attack_filter(msg):
116+
nonlocal counter
117+
118+
if not msg.tx.inputs:
119+
return msg
120+
121+
# on first attempt, send unmodified input
122+
if counter > 0:
123+
counter -= 1
124+
return msg
125+
126+
# on second request, send modified input
127+
msg.tx.inputs[0].prev_hash = prev_hash
128+
return msg
129+
130+
with client, pytest.raises(TrezorFailure) as e:
131+
client.set_filter(messages.TxAck, attack_filter)
132+
btc.sign_tx(client, "Testnet", [inp1], [out1])
133+
134+
# check that injection was performed
135+
assert counter == 0
136+
_check_error_message(prev_hash, client.features.model, e.value.failure.message)
137+
138+
139+
@pytest.mark.skip_ui
140+
@pytest.mark.parametrize("prev_hash", (None, b"", b"x", b"hello world", b"x" * 33))
141+
def test_invalid_prev_hash_in_prevtx(client, prev_hash):
142+
cache = tx_cache("Bitcoin")
143+
prev_tx = cache[TXHASH_157041]
144+
145+
# smoke check: replace prev_hash with all zeros, reserialize and hash, try to sign
146+
prev_tx.inputs[0].prev_hash = b"\x00" * 32
147+
tx_hash = hash_tx(serialize_tx(prev_tx))
148+
149+
inp0 = messages.TxInputType(address_n=[0], prev_hash=tx_hash, prev_index=0)
150+
out1 = messages.TxOutputType(
151+
address="1MJ2tj2ThBE62zXbBYA5ZaN3fdve5CPAz1",
152+
amount=1000,
153+
script_type=messages.OutputScriptType.PAYTOADDRESS,
154+
)
155+
btc.sign_tx(client, "Bitcoin", [inp0], [out1], prev_txes={tx_hash: prev_tx})
156+
157+
# attack: replace prev_hash with an invalid value
158+
prev_tx.inputs[0].prev_hash = prev_hash
159+
tx_hash = hash_tx(serialize_tx(prev_tx))
160+
inp0.prev_hash = tx_hash
161+
162+
with pytest.raises(TrezorFailure) as e:
163+
btc.sign_tx(client, "Bitcoin", [inp0], [out1], prev_txes={tx_hash: prev_tx})
164+
_check_error_message(prev_hash, client.features.model, e.value.failure.message)

0 commit comments

Comments
 (0)