(Bug) Can send multiple postcards to the same address

[phahulin]

• What is it? (leave one option)
  • (Bug) report (i.e. something doesn't work as it should)

Malicious user calls /prepareRegTx endpoint N times, server generates N different confirmation codes and returns N different session keys sk_1, ..., sk_N. However, user only sends 1 tx to the contract, then uses this tx hash to send N calls to /notifyRegTx with different sk_i. This will result in server sending N postcards to the same address

[phahulin]

The best solution here is to decouple postcards processing from user interaction - periodically fetch blocks from blockchain in background and search for LogAddressRegistered events, then read tx data from blockchain and send postcards. On client side, after tx is mined periodically check with server if it was processed.

But this requires code refactoring, maybe there's a simpler solution.

[phahulin]

@fvictorio Here's another suggestion from auditors: in session, keep not only wallet and confirmation code, but also signOutput https://github.com/poanetwork/poa-popa/blob/e259cec1fcfcfdff30a52bffb395d845c774855b/web-dapp/routes/prepare_reg_tx.js#L50

Since each session generates a new confirmation code, signOutput is different for each session + signOutput is passed to the tx https://github.com/poanetwork/poa-popa/blob/e259cec1fcfcfdff30a52bffb395d845c774855b/web-dapp/routes/prepare_reg_tx.js#L56-L64

so malicious user won't be able to reuse the same tx on different sessions. When server fetches tx from the network https://github.com/poanetwork/poa-popa/blob/e259cec1fcfcfdff30a52bffb395d845c774855b/web-dapp/server-lib/get_tx_receipt.js it can decode tx data, recover signOutput and compare it with the one in session (this is not very easy).

I think this solution is a good one.

[fvictorio]

That sounds good.

I was thinking about a different solution, but I'm not really sure about it: instead of using a random session key, use a combination of the wallet and the form data to generate it (probably the hash of all this). That would mean that the same wallet for a given address can only have one item in the db.

But I'm not really sure about it. Can the attacker generate other valid session keys with different form data, and use them in notifyRegTx? What happens if some user accidentally sends the same transaction twice, is there a chance that this causes a bad result for a benign user?

So I'll better try saving the signOutput as the auditors proposed and kept this idea as an alternative solution.

[fvictorio]

I re-read your comment, and I think I don't understand something.

The confirmation code is also unique for each session, and the transaction has the sha3 of the confirmation code. So why not just checking that the sha3cc in the transaction matches the confirmation code in the db?

It's pretty much the same idea, but without having to save signOutput.

[phahulin]

I think you're right, but let me check with the auditors. Actually, initially they suggested to reconstruct full tx.data based on abi-encoded parameters from https://github.com/poanetwork/poa-popa/blob/e259cec1fcfcfdff30a52bffb395d845c774855b/web-dapp/routes/prepare_reg_tx.js#L56-L64 I think any combination that includes at least signOutput or sha3cc will work, we should select the easiest one to implement. But let me check first.

[phahulin]

@fvictorio you're right - can use signOutput or confirmationCodePlain