search

cashshuffle / cashshuffle-electron-cash-plugin

Electron cash cash-shuffle plugin
27 stars 13 forks source link

Unlinkability broken due to missing equality check #25

Closed real-or-random closed 6 years ago

real-or-random commented 6 years ago

Peers need to check if there are duplicate ciphertexts after decryption. If not, attacks on unlinkability are possible. This check is missing here after this line: https://github.com/cashshuffle/cashshuffle-electron-cash-plugin/blob/bcd9d668dcde92649be78e445da5820ee79b7ffa/shuffle/coin_shuffle.py#L243

As co-author of the CoinShuffle paper, I recommend everybody not to use this plugin. It has not been audited properly and potentially contains more serious flaws. Moreover, the CoinShuffle protocol is outdated, CoinShuffle++ is better in every aspect. No coin mixing protocol should be used without very careful coin control mechanisms built in the client, because it's a huge cannon for anonymity.

zquestz commented 6 years ago

What would be the steps to have this formally reviewed? Would love to get this audited by more folks. I only wrote the server side, and have limited context on the plugin itself.

zquestz commented 6 years ago

So just to be clear, a plugin that is in development and not released is something you are actively going to tell people not to use? Well we didn't intend for it to be used either, that would be why we put out a dev release, to get reviews started, and fix bugs in the underlying implementation.

At no time have we said this is production ready. Just to be very clear.

real-or-random commented 6 years ago

I don't know your intentions and I see your point that there is no official release so far. The thing is: Neither the website nor the README mention that it is a development version. Instead, the README contains detailed installation instructions that not only advanced developers can use. And even advanced developers probably don't want to lose their coins. If your intention is that people should not use this currently, then this should be made very clear. Even better, it should be configured to only work on testnet.

If you need an audit: There are people who do security audits for money. Personally I'm not interested in doing an audit, because I think this old version of CoinShuffle is obsolete. There is CoinShuffle++, which is much faster and simpler. That also means it easier simpler to implement. There is no reason to use the old CoinShuffle.

zquestz commented 6 years ago

There is already effort underway to do CoinShuffle++. We did this first to become more familiar with the protocol. That was already our plan.

Since you worked on the original CoinShuffle, do you have suggestions on people who we should reach out to for an audit?

zquestz commented 6 years ago

Appreciate the feedback. I have gone ahead and updated the README to be very clear that this is a pre-release.

real-or-random commented 6 years ago

Okay, then you should know that CoinShuffle++ is pretty different from the old CoinShuffle. The biggest similarity is that it follows the same paradigm of "try to shuffle. if it fails, break anonymity and exclude the evil guy".

clifordsymack commented 6 years ago

duplicate ciphertexts are checked now