0) ZCoin is a fork of Bitcoin that uses a 4 year old academic research library, libzerocoin, to make anonymous payments using the Zerocoin protocol.
1) Unlike Zcash/Zerocash, the Zerocoin protocol has only fixed value coins.
2) To get multiple denominations, you have completely separate instances of the anonymous currency that just happen to live on the same blockchain as the other denominations.
3) Zerocoin has its own bitcoin like non anonymous base currency. Call it basecoin.
4) You spend basecoins to get zerocoins.
5) When you spend zerocoins, you get basecoins.
6) ZQ_WILLIAMSON and ZQ_PEDERSEN are denominations, worth 100 and 50 respectively, defined in libzerocoin.
So what went wrong?
When you convert a zerocoin into 100 basecoin, the ZCoin code forked from bitcoin checked if the coin was a valid instance of ZQ_PEDERSEN (worth 50 ) not ZQ_WILLIAMSON (worth 100). So you paid 50 for the zcoin,got it into the instance for ZQ_PEDERSEN, but got back 100. Free money.
Why did this happen? Well, it looks like in order to support the multiple denominations libzerocoin offers, the ZCoin developers wrote some code for one denomination and then duplicated it for each remaining denomination. There are five in total, ZQ_LOVELACE=1,ZQ_GOLDWASSER=10, ZQ_RACKOFF = 25, ZQ_PEDERSEN = 50,ZQ_WILLIAMSON = 100.
But on the last one, ZQ_PEDERSEN was not changed to ZQ_WILLIAMSON in a few places. This caused the bug.
Caveat: I have nothing to do with ZCoin. However, I am an author of the zerocoin protocol, libzerocoin, the zerocash protocol, and am involved with Zcash.
Just to clarify, the code that was duplicated per denomination is not part of libzerocoin itself, it's in main.cpp. I'm not sure who wrote it; it may or may not have been part of the academic prototype Ian refers to. In any case, this amount of duplication (in security-critical code, no less) should never have passed the necessary code review to release a cryptocurrency. Also note that there are still unexplained differences between the copied code branches after the security fix.
(In contrast, Zcash did have duplicated code in the prototype we inherited, but we rewrote that entirely well before the Zcash launch.)
[Edit: I confirmed that the duplicated validation code in main.cpp was not present in libzerocoin. Some of the code in main.cpp including some stale comments, appears to have been pasted from https://github.com/Zerocoin/libzerocoin/blob/master/Tutorial... , but that tutorial code does not have the bug. So it appears that it was introduced by the Moneta/Zcoin developers.]
Any idea why they would describe the code error as "a single additional character in code"? It looks like about 10 characters or so based on your link. There are also some other code changes associated with that commit
Another major bug caused by copy+paste. I seem to remember a security researcher article months (years?) ago that identified this theme, showed a way to grep a codebase for likely c+p errors and found a load of bugs in real production code that had remained hidden for years. I think I landed there from HN, but my google-fu is failing me now, can anyone else remember it?
Probably not what you mean, but this (https://news.ycombinator.com/item?id=12853211) submission about the PVS-Studio static analyzer also shows a bunch of copy+paste errors being found.
This isn't a subtle or difficult-to-find case. It's a case of "why the heck would anyone write code like that, in any language, in the first place?" The only language-level abstraction needed to avoid this particular kind of duplicated code, is a loop.
(Just to be clear, this is about Zcoin, not Zcash/Zerocash. The two are completely different)
The fix is here. https://github.com/zcoinofficial/zcoin/commit/33796c839f7d4d... What happened?
First, some stylized facts about ZCoin:
0) ZCoin is a fork of Bitcoin that uses a 4 year old academic research library, libzerocoin, to make anonymous payments using the Zerocoin protocol.
1) Unlike Zcash/Zerocash, the Zerocoin protocol has only fixed value coins.
2) To get multiple denominations, you have completely separate instances of the anonymous currency that just happen to live on the same blockchain as the other denominations.
3) Zerocoin has its own bitcoin like non anonymous base currency. Call it basecoin.
4) You spend basecoins to get zerocoins.
5) When you spend zerocoins, you get basecoins.
6) ZQ_WILLIAMSON and ZQ_PEDERSEN are denominations, worth 100 and 50 respectively, defined in libzerocoin.
So what went wrong?
When you convert a zerocoin into 100 basecoin, the ZCoin code forked from bitcoin checked if the coin was a valid instance of ZQ_PEDERSEN (worth 50 ) not ZQ_WILLIAMSON (worth 100). So you paid 50 for the zcoin,got it into the instance for ZQ_PEDERSEN, but got back 100. Free money.
Why did this happen? Well, it looks like in order to support the multiple denominations libzerocoin offers, the ZCoin developers wrote some code for one denomination and then duplicated it for each remaining denomination. There are five in total, ZQ_LOVELACE=1,ZQ_GOLDWASSER=10, ZQ_RACKOFF = 25, ZQ_PEDERSEN = 50,ZQ_WILLIAMSON = 100.
But on the last one, ZQ_PEDERSEN was not changed to ZQ_WILLIAMSON in a few places. This caused the bug.
Caveat: I have nothing to do with ZCoin. However, I am an author of the zerocoin protocol, libzerocoin, the zerocash protocol, and am involved with Zcash.