Warning on signing non-segwit input

[andronoob]

The current warning text only mentioned the risk of paying higher mining fee than expectation. However, to my understanding, the actual risk seems to be higher than that:

Under a scenario that a payment involves multiple parties. a malicious party could stealthily replace their own inputs with UTXOs of lower value, while the victims' inputs would be replaced with UTXOs of (corresponding) higher value, thus the victims would actually pay a (much) higher amount than expectation during this payment.

Edit 2020-06-05: After learning the details of the recently-disclosed security issue of BIP143, I have realized it's not the UTXOs would be replaced, but the amounts would be directly tampered by the attacker.

[SomberNight]

Yes, indeed you are right.
Will need to think a bit about this.

[andronoob]

I see that a PSBT_IN_NON_WITNESS_UTXO field is already defined in the PSBT BIP spec - is that field required? Then the wallet should simply refuse to sign any PSBT which does not contain the previous transaction data, right?

[andronoob]

If it's not a PSBT, I think the warning text could be improved like this:

You are taking the risk of paying an unexpected high amount on this transaction.
The actual pay-out amount (including mining fee) of this transaction is unknown, beause it contains at least one non-SegWit input to be signed, meanwhile the corresponding origin transaction data is missing for verification.
When signing non-SegWit input(s), it's always possible that the displayed values had already been tampered by a malicious party, in order to trick you into paying a much higher amount than expectation.
You may import the origin transaction(s) to ensure the input value could be verified, otherwise you would take the risks. Here are the TXID(s):
[TXIDs woud be listed here, should be an EditView with scrollbar]

It should be a dialog which provides two buttons of choice, one is Cancel, the other is Continue anyway.

[SomberNight]

Read top comment in the PSBT PR: #5721 (comment) There's no point in listing the input txids in a new dialog as they can already be seen in the Transaction Dialog itself. Further note that when all inputs are ismine, which is actually by far the typical case for now among Electrum users, then it's only about the mining fee (hence an attack is less incentivised). Also, in your phrasing you say the "pay-out amount" is unknown, not sure what you mean, but the outputs are certainly committed to. Perhaps we could just make sure that the input amounts are not shown; or show them but with the warning sign. (And perhaps only in the case when not all inputs are ismine) -------- Original Message --------

…

On 9 Nov 2019, 04:42, andronoob wrote: If it's not a PSBT, I think the warning text could be improved like this: > You are taking the risk of paying an unexpected high amount on this transaction. > The actual pay-out amount (including mining fee) of this transaction is unknown, beause it contains at least one non-SegWit input, meanwhile the corresponding origin transaction data is missing for verification. > When signing non-SegWit input(s), it's always possible that the displayed values had already been tampered by a malicious party, in order to trick you into paying a much higher amount than expectation. > You may import the origin transaction(s) to ensure the input value could be verified, otherwise you would take the risks. Here are the TXID(s): > [TXIDs woud be listed here, should be an EditView with scrollbar] It should be a dialog which provides two buttons of choice, one is Cancel, the other is Continue anyway. — You are receiving this because you commented. Reply to this email directly, [view it on GitHub](#5749?email_source=notifications&email_token=AG6K3XK46EK4EAL3A4AHQ53QSYWRVA5CNFSM4JKVB7EKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEDT44II#issuecomment-552062497), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/AG6K3XPYS5E2AQMBRSR2SS3QSYWRVANCNFSM4JKVB7EA).

[andronoob]

Read top comment in the PSBT PR: #5721 (comment)

Sorry... Now I learned that non-PSBT partial transactions will no longer be supported.

Also, in your phrasing you say the "pay-out amount" is unknown

I just meant the actual amount (including mining fee) paid by the user/victim. I don't know the proper phrasing.

Further note that when all inputs are ismine, which is actually by far the typical case for now among Electrum users, then it's only about the mining fee (hence an attack is less incentivised).

But Electrum supports multi-party signing of PSBT, right?

To my current understanding, it's actually a compromise, which sacrifice security in exchange for viability of encoding all PSBT data into one single QR code.

This just reminds me of #4959. IMHO, an animated QR code should be able to solve all these problems. However, I don't know if https://github.com/divan/txqr meets the requirements of Electrum project now.

Besides animated QR code, maybe audio-based data transfering is also possible to solve this.

[andronoob]

BTW, though I don't know informatics, I'm a bit concerned about reliability of animated QR code/audio-based data transfering, because a single bit-flip could cause permanent fund loss (like sending to a corrupted scriptPubKey, which is essentially a black hole), as far as I know.

[SomberNight]

In light of #6198 (and related) we will have to display a warning even for segwit inputs.

[ecdsa]

I believe it still makes sense to distinguish segwit from non-segwit inputs: What an attacker can do in the scenario of #6198 is very different from what they can do if we sign non-segwit inputs.

I don't think we should display the same message in both cases. The difficulty is how to explain #6198 in simple terms.

[andronoob]

What an attacker can do in the scenario of #6198 is very different from what they can do if we sign non-segwit inputs.

I on the contrary think it's probably not so different... The case of Electrum is better than Trezor in the sense that the transaction details, especially the amount of each input, are displayed to the user, therefore the user would be able to notice the discrepancy between two transactions (could be done on the offline device as well).

However the user may not pay much attention to it.

[andronoob]

If there's only one SegWit input, and the ANYONECANPAY flag is not set, it seems that the "signature covers amount" feature of SegWit can now work as intended... However the applicable scenario is obviously very limited.

[andronoob]

IMHO this should be fixed in the BIP174 standard (or some new BIP), with sequence number field (to split the data of one transaction into multiple parts) or fountain code etc, otherwise it would break interoperability.

[ecdsa]

I on the contrary think it's probably not so different..

It depends on the context. if you are signing a transaction with cold storage, which is the most common use case for this warning message, then it still makes sense to display a different message for non-segwit inputs than for segwit inputs.

If you are doing multi-party payments (the use case you mentioned) then the attack surface increases, and we probably need another warning message, more adapted to that situation.

The display of a warning is currently decided by tx.is_there_risk_of_burning_coins_as_fees(). it would need to consider the presence of multiple signers.

[andronoob]

In my opinion, because lack of full previous transaction data introduces risk/vulnerability, the most straightforward and effective fix is to feed/require full previous transaction data for all inputs (regardless whoever it belongs to). Irrespective to this issue, the problem of limited data capacity of one singe QR code needs to be fixed. I still haven't changed my mind on these points.

I have also learned a hypothetical "fee ransom" attack which allows the attacker to directly profit.

---

As for the similar double-payment/abused ANYONECANPAY problems (I'm still not quite sure whether ANYONECANPAY really matters?), I still haven't given up on the idea that the user should be more cautious about every transaction he had once signed, with the assistance of a "stateful" wallet.

Just like the case of a blockchain congestion, a user might try to double spend (manual full-RBF) the "stuck" UTXOs once again, in order to "unstuck" them. In this case the user also needs to guarantee the two transactions conflicts with each other. Besides, this awkward situation can last for days or even months.

I have realised it's tricky that a compromised online "hot" device can lie about anything, including the confirmation count. Maybe the offline wallet just needs to keep all signed transactions, as long as possible - this makes me uncomfortable, so that I have even came up with wilder ideas like SPV proofs (to reliably delete transactions in the offline wallet, both the confirmed one and any conflicting ones, so that normally the offline wallet should be "empty")... Ah, maybe it's just too odd, it shouldn't go that far.

Edit: sorry that I missed the point. The point should be "the user must ensure that two transactions conflict with each other, so that double-payment won't occur", which is obviously unrelated to confirmation count - Deleting a signed transaction would on the contrary bring back the risk of double-payment.

[SomberNight]

To reiterate, if we put full prev txs into the QR code, then there would be no QR code support for txs at all basically.

Note that if prev txs are missing but the scanning device is online, it will fetch them from the network.

Imagine a user who uses multisig with Electrum, let's say between a laptop and a phone, both online, using QR codes to transport txs in-between.
For this particular user the choice we make here (whether to include full prev txs) will translate whether their usecase dies. Note that even without prev txs in the QR code, this user is perfectly safe from this issue (again as they will fetch them from the network).

[andronoob]

To reiterate, if we put full prev txs into the QR code, then there would be no QR code support for txs at all basically.

If Electrum is facing obstacles on supporting animated QR code, I respect it. I just think it would be a temporary expedient, to be eventually replaced with real solution in the future.

Note that if prev txs are missing but the scanning device is online, it will fetch them from the network.

Yeah there's situations where omitting previous transaction data can work well, however there's also situations where the device is offline, or the device doesn't want to rely on Electrum servers (just like the case of Bitcoin Core without txindex, or even worse, with pruning enabled), so that omitting previous transaction data still expose the user to the vulnerability, although the actual risk is probably very low.

(Note that even it's a situation of pruning Bitcoin Core node, the scantxoutset RPC command can still provide block height of specified UTXO, which seemed to allow BIP157/158-like method to fetch back the full previous transaction data by requesting other peers for corresponding full block data. However, scantxoutset and fetching block would take some time to complete)

For this particular user the choice we make here (whether to include full prev txs) will translate whether their usecase dies.

I'm also afraid to say that maybe users who need to sign large transactions have been expecting animated QR code for quite a long time? AFAIK there're already some other wallets which implement multi-part (not animated though) QR code, like Bither.

Without QR code it's still possible to get the transaction transferred, with something like a USB thumb drive, amodem (I don't have experience on amodem, however I just think audio has the nature to be "broadcasted", which doesn't seem good) or bluetooth etc. It's just not as convenient as QR code.

Note that I used to think that QR code seemed to be almostly perfect, since other transferring channel like USB may introduce extra attack surface (like LNK vulnerability of MS Windows, or adb interface of an old Android phone) However I might overlooked risks of QR code like peeking, and something even more scary like BadBarcode. Despite this, I still think QR code is a good choice, since it's convenient and (probably) secure enough (since the sensitive offline device could still defend itself against BadBarcode, with Keyboard Simulation Mode dropped).

[andronoob]

I think some standard about animated QR code would be great. However if some wallet implement it before the corresponding standard, it's still not bad, at least not worse than the status-quo of fragmented seed format/derivation path, since partially signed/unsigned transaction data is transient thing, while a seed backup is persistent.

[SomberNight]

@andronoob You are of course right that these warning are less than ideal and ultimately the issue is the low capacity of QR codes. Hence, you are also right that maybe QR codes should be replaced with something else instead. However, QR codes are well supported on all platforms, for both parties (i.e. developer and user): have ready-to-use dependencies we can just pull in, and for the user there are all kinds of 3rd party apps that can decode them (even integrated as first-party into e.g. modern Android).

Any potential replacements for QR codes would need a significant amount of work :/
So while these warning texts might end up being an interim temporary solution only, we are not sure how long that "temporary" period might last.

[andronoob]

for the user there are all kinds of 3rd party apps that can decode them (even integrated as first-party into e.g. modern Android).

IMHO this may not be a good thing... It's not a surprise to see some 3rd party QR code app to submit the data online. Even a master public key is still considered to affect privacy, let alone the case of outright malicious QR code generator tampering the receiving address.