enable streaming full UTXOs for all types of inputs

[matejcik]

A security issue* in the design of BIP-143 allows an attacker to lie about segwit input amounts and get the user to pay an unexpectedly high transaction fee. The problem affects all HWW vendors.

We are fixing this by making Trezor require the full UTXO for all types of inputs, so we can validate that the input amount is correct.

To facilitate that, we need Electrum to provide full UTXOs for all input types. (This goes against the recommendation in BIP-174, saying that NON_WITNESS_UTXO should not be provided for SegWit inputs. Nevertheless, the resulting PSBT is still valid AFAICT.)

This PR enables that for Trezor, modifications for other HWW vendors will most likely also be required. Also I'm not sure whether this is a complete PR, perhaps my modifications to the data model are breaking something?

Without this modification, it will be impossible to sign SegWit transactions on Trezor firmwares starting with 1.9.1 and 2.3.1.

*) Details in our blogpost: https://blog.trezor.io/details-of-firmware-updates-for-trezor-one-version-1-9-1-and-trezor-model-t-version-2-3-1-1eba8f60f2dd

[SomberNight]

You are setting both PSBT_IN_NON_WITNESS_UTXO and PSBT_IN_WITNESS_UTXO.
Would it not be better to only set PSBT_IN_NON_WITNESS_UTXO?

Note that e.g. Bitcoin Core will outright reject the PSBT if both are set:
https://github.com/bitcoin/bitcoin/blob/234fabab90627eb9f93b03533c74f830b771f9ba/src/psbt.cpp#L164

[SomberNight]

(I will likely to do a follow-up commit as I think I might want more related code changed; I am mostly interested in your opinion)

[SomberNight]

Arghhh but if we only set PSBT_IN_NON_WITNESS_UTXO, we will fail the next sanity check:
https://github.com/bitcoin/bitcoin/blob/234fabab90627eb9f93b03533c74f830b771f9ba/src/psbt.cpp#L167
(for e.g. p2wsh, with PSBT_IN_WITNESS_SCRIPT set)

[SomberNight]

Well I don't think it is possible to pass the sanity checks imposed by existing Bitcoin Core.
That is, it is not possible to create a PSBT that contains all the info needed by a hardware wallet, that passes IsSane in Bitcoin Core.
I think the sane (:P) thing to do is to only include PSBT_IN_NON_WITNESS_UTXO.

@achow101 what is your opinion?

[achow101]

Yes, Core will need changes. I believe BIP 174 itself will need changes too.

I don't think it makes sense to have both UTXOs though.

[matejcik]

wrt setting both utxo types, my goal was to implement a minimum necessary change. i did not remove witness_utxo for fear of breaking some unrelated part of code that relies on it. i agree that it seems unnecessary, so as far as I'm concerned feel free to remove it

[andronoob]

@matejcik What's your thoughts on #5749?

Edit: IIRC, Trezor simply doesn't allow such transaction...

[matejcik]

@matejcik What's your thoughts on #5749?

Edit: IIRC, Trezor simply doesn't allow such transaction...

is Electrum allowing signing non-segwit inputs without validating the prevtx? If that's the case, then yes, this was never possible on Trezor. (And neither on other HWWs, I would presume. In the context of a stateless wallet it is simply too easy to attack.)
Personally I'd suggest disallowing that outright, unless you actively download and verify the utxos.

[andronoob]

@matejcik No... I meant that as far as I can remember, Trezor doesn't support signing transactions involving multiple payers, not multisig, just multiple inputs from multiple payers - I'm not quite sure on this however.

[andronoob]

Regarding the issue of omitting previous tx, Electrum did that due to the constrained data capacity of one single QR code, otherwise the data must be splitted into multiple parts, or encoded into some fountain code. Sadly BIP174 doesn't handle this kind of stuff at all.

(It's obvious that even taking such sacrifice/compromise of security, some large transactions still can't fit into one single QR code... In fact this has became an issue for Electrum for quite a long time)

BTW I think it's probably better for BIP174 to have a checksum, because one single bit flip could lead to tragic outcome, if I didn't misunderstand. Sure, this is already handled at the "transportation layer", but I think it's still meaningful to do this at the PSBT layer as well.

[matejcik]

@matejcik No... I meant that as far as I can remember, Trezor doesn't support signing transactions involving multiple payers, not multisig, just multiple inputs from multiple payers - I'm not quite sure on this however.

That is also correct. We are currently reevaluating this, see SLIP-19 draft for details both for why we didn't want this, and how we can allow it.

[andronoob]

@matejcik actually I not only want to confirm that Trezor doesn't allow signing multi-payer transactions, but also want to ask whether this exploit of BIP143 design flaw is also applicable to the multi-payer scenario, so that the actual risk would be probably much higher, since the attacker (as a malicious co-payer) would be able to directly profit.

[matejcik]

@matejcik actually I not only want to confirm that Trezor doesn't allow signing multi-payer transactions, but also want to ask whether this exploit of BIP143 design flaw is also applicable to the multi-payer scenario, so that the actual risk would be probably much higher, since the attacker (as a malicious co-payer) would be able to directly profit.

Presumably, yes. There is a variant of the attack that applies to coinjoins, which is described in the above linked SLIP-19. ISTM this would translate directly to a non-coinjoin multiparty payment.

I'm not sure what checks are in place for the UTXO verification, but how about this?

• let the victim have UTXO A with 1 BTC and UTXO B with 1 BTC
• craft a PSBT with inputs A = 1 BTC (attacker signature), B = 1 BTC (victim signature), outputs X = 1 BTC (common destination), Y = 0.5 BTC (attacker change), Z = 0.5 BTC (victim change)
  => victim confirms spending 0.5 BTC (paid 1 BTC and got back 0.5 as change)
• show error message, submit PSBT where A is victim signature and B is attacker signature
  => victim confirms seemingly same thing
  => attacker in fact paid 0 and victim paid 1.5 BTC

[matejcik]

hmm, perhaps a better example:

• let the victim have UTXO A with 1 BTC, B with 1 BTC, attacker has C with 1 BTC
• craft PSBT with inputs A = 1 BTC, B = 0.00001 BTC, C = 2 BTC, attacker change X = 2 BTC, victim change Y = 1 BTC
  => victim is confirming spend of 0.00001, with attacker putting in 2 and getting out 2
• switch around amounts for A and B
  => victim is confirming same thing
• attacker now has valid signatures for A and B
• create a valid signature for C = 1 BTC
  => final spend is victim paying 2 BTC and getting back 1 BTC, attacker is paying 1 BTC and getting back 2 BTC

[andronoob]

a better example

I think it's the case I had thought of.

craft a PSBT with inputs A = 1 BTC (attacker signature)

Oh this really scares me... Then it's not only the amounts, but also the ownership which could be tampered/manipulated by the attacker (it's just another way to decept the victim about the actual amount) - did I get what you meant?

---

I now wonder whether some similar attack is still viable even if the victim could validate full previous transaction data of all inputs belong to him? (Edit: sorry that I mistakenly put "belong to him" here) If so, #5749 would be much more tricky than what I had thought of... I think it's not that bad, but I'm not sure.

Also, I wonder whether it's safe to sign a transaction which only has one SegWit input belongs to the user himself (Edit: I forgot to put "with the ANYONECANPAY flag unset" here), without full previous tx data?

[matejcik]

Oh this really scares me... Then it's not only the amounts, but also the ownership which could be tampered/manipulated by the attacker (it's just another way to decept the victim about the actual amount) - did I get what you meant?

Yes, which is why i was wondering what sort of checks you do.
A HWW cannot tell if a prevtx belongs to it. Hence the necessity for proofs of (non)ownership. A stateful wallet like Electrum can know all UTXOs belonging to it, so this might be less of a concern. But if the only data you are using is a PSBT, I can just omit the BIP32 signing info from an input and claim that it is not yours to sign.

Depending on the signing scenario, you need to ascertain that:

• the prevtx you are signing has the correct amount (you need the full tx data for that, so that you can re-serialize with the provided amount and see if txhash matches)
• the prevtx you are signing does or doesn't belong to you as the signer

Also, I wonder whether it's safe to sign a transaction which only has one SegWit input belongs to the user himself, without full previous tx data?

That should be safe AFAICT. If the attacker lies about the amount on the only input, they can get you to provide an invalid signature. But there is no valid signature generated in the same step, so there is nothing the attacker can save for later.

[andronoob]

Sorry, I didn't describe what I meant correctly... I wrote:

I now wonder whether some similar attack is still viable even if the victim could validate full previous transaction data of all inputs belong to him?

which is not what I meant. I actually meant:

I now wonder whether some similar attack is still viable even if the victim could validate full previous transaction data of all inputs belong to him (just all the inputs, regardless whoever it belongs to)?

[andronoob]

Also, regarding the "only one SegWit input belongs to the user himself" scenario, I forgot to mention that the ANYONECANPAY flag must not be set - I'm not quite sure whether this flag matters either.

[SomberNight]

Note that when using QR codes to transport PSBTs, with segwit inputs, this attack is highly unrealistic.
E.g. imagine the traditional offline signing setup: offline PC with keys; online PC with xpub.

Online PC is infected. Crafts malicious PSBT, displays QR code 1.
User scans QR code 1 on offline PC, signs PSBT. Gets new QR code from offline PC.
User scans new QR code on online PC. Now online PC displays an error and has to convince the user to redo the whole flow with a new QR code, again...
Why would it need to create a new QR code... the user is bound to become suspicious.
If the user had saved the QR code in a file, or printed it onto paper, or took a picture of it with a phone, etc, why would they redo even that part for the new QR code?

Given that QR codes are not possible if we had to include prev txs, I still think we should keep using witness UTXOs there.

---

Btw regarding the example scenario in the Trezor blogpost:

The malware throws an error and tells the user to confirm the transaction again (e.g. “Uh, oh! Something went wrong. Please try again.”).

If the user can be convinced with any kind of ruse to complete all on-device prompts twice, irrespective of any sighash issue they can be tricked to double-pay. Last time I checked the Trezor (or any hw wallet I've tried for that matter) does not show the prevouts for the tx to be signed, so the second prompt can just use different inputs, and there you go, double-paid. To fix this, the device screen should show sufficient information for the user to check that the second tx would conflict with the first.

[andronoob]

If the user had saved the QR code in a file, or printed it onto paper, or took a picture of it with a phone, etc, why would they redo even that part for the new QR code?

Generally, the workflow is like: hot Electrum displays QR code, then cold Electrum scans it - no need to save the intermediate data (QR code) at all... However I agree that it's possible for the user to save the transaction, especially when the QR code is not available at all (if the transaction data is too large).

Why would it need to create a new QR code... the user is bound to become suspicious.

Generally, the user won't have to save the QR code/transaction data, so the user won't notice that it's actually a new QR code at all.

Even if he's requested to recreate unsigned transaction, he might not put too much consideration on this. "Try again, with much care this time" is common logic during troubleshooting.

Besides, there's probably an extreme situation: if the user don't/can't take effective actions to make the "hot" online machine clean (reinstalling the OS is obviously an annoying hassle, there's also nasty stuff like bootkit), it seems that the attack will eventually succeed still.

To fix this, the device screen should show sufficient information for the user to check that the second tx would conflict with the first.

It's true that the user has a chance to notice the discrepancy - however, I think it's just not easy. Besides, under the extreme scenario that the user performed unclean OS reinstall (bootkit/backdoored OS image/uncleared malware in non-OS partitions/etc), this probably won't work...

[andronoob]

In my opinion:

1. Regardless this security weakness, the problem that large transaction data can't fit into one single QR code needs to be fixed. I wish that some standard of multi-part PSBT or fountain-coded PSBT could solve this.

2. Once the QR code could handle large amount of data, just put every previous tx into the PSBT, and then it would be easy to validate them. To me it's very straightforward and effective.

[SomberNight]

To fix this, the device screen should show sufficient information for the user to check that the second tx would conflict with the first.

It's true that the user has a chance to notice the discrepancy - however, I think it's just not easy. Besides, under the extreme scenario that the user performed unclean OS reinstall (bootkit/backdoored OS image/uncleared malware in non-OS partitions/etc), this probably won't work...

Note that irrespective of this issue, and any sighash vulnerability, the user must check that the second tx conflicts with the first. There is nothing we can do (by e.g. putting stuff in the psbt) if they don't check.

[andronoob]

Note that irrespective of this issue, and any sighash vulnerability, the user must check that the second tx conflicts with the first. There is nothing we can do (by e.g. putting stuff in the psbt) if they don't check.

I have to agree, that the user must be very cautious when the signed transaction can't go through as ususal. However, I still think it's unpleasant for the user to check the inputs (previous txid/vout/amount/address etc) with his own eyeballs.

Edit: it's of course not the same case with this BIP143 vulnerability. To prevent simple double-payment the user just need to guarantee the two transactions share common inputs, so that these two transactions would conflict with each other. In the case of this BIP143 vulnerability, the user has to also pay much attention to the input values.

@matejcik Maybe there's some better approach, like, after signing, the HWW could ask the user whether the signed transaction has successfully went through as usual, if not, it would then keep the signed transaction, and then warn the user? (especially, tell the user not to factory-reset the device, and not to switch to another device? I have to admit that the malware could still scare the user with excuses like "prevent potential leakage ASAP", but I think this could be mitigated by education/guidance before the user starts to use the product)

Besides, this reminds me of potential human error of double-payment during manual full-RBF, and the signature capture attack exploiting BIP70.

As for Electrum, it seems that such approach could help as well, but I'm not quite sure.

[andronoob]

@SomberNight Anyway, I still think providing full previous transaction data is the correct fix to this BIP143 security issue. Maybe only after this issue being resolved we would be able to find some way to deal with other problems like double-payments.

[andronoob]

As long as the full previous transaction data is not provided to the offline device (regardless SegWit or not), the user would have to face a complex situation. To me the most straightforward fix is to provide such data to the offline device, only then the user's headache could be relieved a bit - to me double-payment is a much simpler situation than this sighash vulnerability...

[andronoob]

Oh, I have just realized that such approach of asking the user for the delivery result of the signed transaction is more likely to be a bad idea... Theoretically the compromised "hot" device could lie about anything, including the confirmation count.