Encrypted payment ID is decrypted once per transaction and shared across unrelated accounts

[mahnunchik]

ownership_test::operator() declares payment_id once for the whole transaction:

monero-lws/src/util/ownership_test.cpp

Line 77 in 1e48ef2

std::pair<std::uint8_t, db::output::payment_id_> payment_id;

then loops over every registered account:

monero-lws/src/util/ownership_test.cpp

Line 110 in 1e48ef2

for (account& user : users)

and every output:

monero-lws/src/util/ownership_test.cpp

Line 169 in 1e48ef2

for (std::size_t index = 0; index < tx.vout.size(); ++index)

Decryption only happens once, guarded by !payment_id.first:

monero-lws/src/util/ownership_test.cpp

Lines 267 to 271 in 1e48ef2

	if (extra_nonce && !payment_id.first && cryptonote::get_encrypted_payment_id_from_tx_extra_nonce(extra_nonce->nonce, payment_id.second.short_))
	{
	payment_id.first = sizeof(crypto::hash8);
	lws::decrypt_payment_id(payment_id.second.short_, active_derivation);
	}

using whichever (account, output) pair matches first. The resulting payment_id.second is
then attached to every account's own output record for that transaction, not just the one it was
actually encrypted for:

monero-lws/src/util/ownership_test.cpp

Line 291 in 1e48ef2

payment_id.second,

Effect: in a transaction that pays multiple accounts (e.g. one recipient with an encrypted/
integrated-address payment ID, other recipients being unrelated regular destinations), whichever
account happens to be scanned first "wins" the correctly-decrypted payment ID - every other
account that received an output in that same transaction gets that same (wrong, foreign) value
recorded against their own output, instead of nothing/their own correct value.

Repro: send one transaction with several destinations across different accounts, where one
destination is an integrated address. Check get_address_txs for the other (non-integrated)
recipients - they show a payment_id that isn't theirs.

Fix direction: payment_id needs to be decrypted per (account, output) that actually owns it,
not once globally per transaction - it can't be hoisted above the for (account& user : users) /
per-output loops the way it is now.

[vtnerd]

Good catch, this will have to be back ported all the way to 0.3 too I think. Not sure how I'm going to fix this yet.

[vtnerd]

The biggest issue seems to be that the payment_id can be wrong if both sender and receiver are in the account list. There really isn't much of a leak, because everyone seeing the payment id was involved in the transaction anyway.