search

nochowderforyou / clams

Clam Project
MIT License
62 stars 58 forks source link

'change' output position isn't properly randomized #182

Closed dooglus closed 9 years ago

dooglus commented 9 years ago

Our wallet.cpp:

// Insert change txn at random position:
vector<CTxOut>::iterator position = wtxNew.vout.begin()+GetRandInt(wtxNew.vout.size());

Bitcoin's:

// Insert change txn at random position:
vector<CTxOut>::iterator position = txNew.vout.begin()+GetRandInt(txNew.vout.size()+1);

We're off-by-one.

For the most common case of a transaction with a single recipient and a single change output, the change is always the first output, and the real recipient is always the 2nd. This gives us poor privacy.

This was fixed in Bitcoin quite a long time ago, in ac7b8ea0864e925b0f5cf487be9acdf4a5d0c487 on Wed Jan 30 2013 ("Correctly randomize change output position").

Are we that far behind Bitcoin still? I thought we were much closer to being in sync with them.

dooglus commented 9 years ago

I merged Gavin's fix in 3685afc7c35ffba5cc99f2015090e210ad06e843.