search

Zondax / ledger-namada

Apache License 2.0
8 stars 10 forks source link

Check `shielded_hash` inside `Transfer`s #48

Open murisi opened 3 weeks ago

murisi commented 3 weeks ago

The v->transfer.shielded_hash field is used by the Namada protocol to identify which MASP Transaction object to extract from the Tx. The hardware wallet currently ignores the v->transfer.shielded_hash field both in printing and signing a MASP Transfer. (See https://github.com/Zondax/ledger-namada/blob/ca9da5e0129f676934809f58acffbd855d280664/app/src/parser_impl_txn.c#L527 .) This is problematic because a malicious client could potentially tamper with the field and get the hardware wallet to sign over the wrong shielded_hash.

If the v->transfer.shielded_hash field is present, we should ensure that it value equals SHA-256(0x04 || txid) where txid is the transaction ID of the MASP Transaction inside the MaspTx section. If there is no match here, the transaction is invalid and should not be printed nor be signable.

Less importantly, the Ledger app should confirm that the target_hash inside the masp_builder_section_t points to the MASP section that it provides the plaintexts for. This target_hash should also contain the value SHA-256(0x04 || txid) where txid is the transaction ID of the MASP Transaction inside the MaspTx section it points to.

chcmedeiros commented 3 weeks ago

Hey @murisi the txid needs to be computed has theblake2b((personalization || consensus_branch_id), header_hash, transparent_hash, sapling_hash) am I correct ?

murisi commented 2 weeks ago

Hey @murisi the txid needs to be computed has theblake2b((personalization || consensus_branch_id), header_hash, transparent_hash, sapling_hash) am I correct ?

Yes, exactly. Is the computed TxId not matching what's contained in the test vectors?

chcmedeiros commented 2 weeks ago

Unfortely yes. Do you think there is a way you can get me just a single testvector with the expected values of header_hash, transparent_hash, sapling_hash, blake2b((personalization || consensus_branch_id). So I can debug the different hashes I am computing with expected values?

murisi commented 2 weeks ago

Unfortely yes. Do you think there is a way you can get me just a single testvector with the expected values of header_hash, transparent_hash, sapling_hash, blake2b((personalization || consensus_branch_id). So I can debug the different hashes I am computing with expected values?

I see. The hash data is a little awkward to access, but I'll get your test vector by tomorrow.

murisi commented 2 weeks ago

Unfortely yes. Do you think there is a way you can get me just a single testvector with the expected values of header_hash, transparent_hash, sapling_hash, blake2b((personalization || consensus_branch_id). So I can debug the different hashes I am computing with expected values?

See attached some test vectors with the hashes. tx_digests.txt

chcmedeiros commented 2 weeks ago

Thanks!