
Zcoin implementation bug enabled attacker to create over 500K Zcoins - mbgaxyz
https://makebitcoingreatagain.wordpress.com/2017/02/18/is-the-zcoin-bug-in-checktransaction/#update6
======
yjgyhj
The point I'm about to make in this comment is so old and has been said so
many times that we all are tired of hearing it. But why do we use a language
like C++ to implement something where we don't wish to have bugs?

C++ is not memory-safe (no GC or whatever Rust does), it's not type safe (in
the ML sense), it relies on writing to memory a lot (instead of having pure
functions). Had this program been written in Ocaml or Haskell or a strongly
typed functional Lisp, this bug would have been impossible to make.

Why do we do this, still? Is it not irresponsible?

~~~
robertgraham
It's an absurd point.

ZCash was built upon the Bitcoin codebase. This inherits a lot of bad
decisions. Moral purity, demanding they start over again from scratch, just
isn't practical.

The bug in question could have been solved had the simply compiled with
minimal static analysis -- by which I mean -Wall.

C/C++ is memory safe if you turn on dynamic checking. Sure, it's twice as slow
as C/C++, but still tons faster than nonsense languages like Ocaml or Haskell.

~~~
im3w1l
> C/C++ is memory safe if you turn on dynamic checking.

Is anyone here doing this? Would be interesting to hear your experience.

~~~
jhasse
If valgrind counts, then yes.

~~~
kzrdude
I'll count valgrind if your program always runs in valgrind. Otherwise it
doesn't protect you _dynamically_.

------
WalterBright
The error is here:

[https://github.com/zcoinofficial/zcoin/blob/81a667867b5d8489...](https://github.com/zcoinofficial/zcoin/blob/81a667867b5d84893e4c225a8d73eddd78ac16c2/src/main.cpp#L963)

and the line of code:

    
    
        zccoinSpend.denomination == libzerocoin::ZQ_LOVELACE;
    

In other words, a statement with no effect. In D, such a line gives an error,
not a warning:

    
    
        Error: == has no effect in expression
    

You can force the statement to be accepted by casting it to void.

It's really past time for languages to not accept such code any more. Legacy
code needs to get fixed.

~~~
jacquesm
You're absolutely right (to my surprise), it gives:

    
    
        test.cpp: In function ‘int main()’:
        test.cpp:13:11: warning: statement has no effect [-Wunused-value]
                 a == b;
                   ^
    

But only after enabling a warning that is not on by default. -Wall and -Werror
should be the default.

~~~
WalterBright
It should be an error, not a warning (default or not). There are a number of
things like this that should just not be accepted anymore. Here's another one:

    
    
        if (condition);
            dothis();
    

No compiler should accept that, regardless of switch settings.

~~~
jacquesm
Fully agreed. There is absolutely no way that is something a programmer ever
intended. In fact I think that just

    
    
        if (condition);
    

Should already be enough to trigger the error. (Any 'if' statement without a
body).

~~~
spacemanmatt
There could be an important side-effect in condition. Isn't persistent state
fun!?

~~~
shawabawa3
Why have it in an if though?

~~~
spacemanmatt
I am not condoning it, so much as noting what goes wrong when it's _easy_ ;)

------
seibelj
> _If the attacker managed to liquidate their Zcoins at an average price of
> $1.25, they may have netted themselves upto USD 750,000._

If you are a skilled programmer / security expert / mathematician, crypto
currency exploit creation seems to be an extremely lucrative hobby.

~~~
goatsi
Indeed. Everything that crypto-currency touches gets an automatic bug bounty.
With rewards that are much nicer than limited editions t-shirts.

~~~
bbcbasic
Depends what side of the reward you are on.

~~~
elastic_church
the savant programmer obviously

------
TD-Linux
Note that this is unrelated to Zcash, the zk-SNARKS based cryptocurrency
perhaps more known to HN.

~~~
sigmar
People complained a lot for 10% of Zcash going to the founders (in the form of
a 20% draw on the first half of mining). But that value and expertise are used
to ensure bugs like this Zcoin attack do not occur.

~~~
JumpCrisscross
> _used to ensure bugs like this...do not occur_

Then compensate for that. Create a pool that pays out for each year without
vulnerabilities.

~~~
sigmar
If there are vulnerabilities, those vulnerabilities will be exploited. That
will reduce the currency value and the value of their 'founders reward'

~~~
lend000
Not if they cash out during "IPO." I don't like the idea of a
founder/developer pool, but if they are going to have one, there should be a
vesting period, such that the granted coins cannot be moved for X years after
receiving them. This will keep the incentive alive, and I think Ethereum would
have a lot more potential if something like this was employed.

~~~
jackgavigan
This is exactly what Zcash did. The Zcash Founders' Reward "vests" over four
years.

------
kondro
Im curious. Is this actually illegal? Is there any requirement on _how_ valid
crypto currency is created?

I understand the network could implement a fix and prevent future transactions
or even roll back old ones if there's enough desire, that's what democracies
are about right?

But was there anything illegal about creating and then trading them? A
rollback would hurt the exchanges, but if they didn't already have something
in their T&Cs about currencies needing to be created legitimately (and what
legitimately actually means), then is it anything other than morally wrong?

~~~
dangero
I recently spoke at an event where we talked about the crossover of
cryptocurrency and the law with a bunch of attorneys present. Many attorneys
spoke up during the Q and A to remind everyone that at the end of the day the
legality of anything will be decided by either a jury or a judge. It's going
to come down to the arguments both sides make, and I think any defense would
have a hard time explaining how what they did was legal in the face of the
CFAA. Analogies will be drawn to hacking of other systems, banks, etc. How
people feel about it culturally will also affect the outcome.

One important distinction to make is that it really is also going to depend on
the jurisdiction of those involved. Is the defendant in the US or somewhere
else? Who is prosecuting and what are the charges?

~~~
goodplay
Is there a recording of this discussion available online?

~~~
dangero
Unfortunately only a snippet that does not include the back and forth with the
audience.
[https://twitter.com/amyywan/status/829614419671871488](https://twitter.com/amyywan/status/829614419671871488)

------
tedunangst
From the blog post explaining the bug, [https://zcoin.io/language/en/zcoins-
zerocoin-bug-explained-i...](https://zcoin.io/language/en/zcoins-zerocoin-bug-
explained-in-detail/), how is that not a "useless statement has no side
effect" warning?

~~~
Sanddancer
It does. It seems like the devs are of the "it compiles, ship it" sort.
Building the source, I'm seeing all kinds of warnings -- the code trying to
return consts, warnings about sign differences, etc. Looks like lots of
potential places for naughtiness to occur.

~~~
martinpw
Indeed. I was hoping that the blog post would conclude with a section
describing how they had learned an important lesson and would now be much more
strict about dealing with this compiler warning in particular, and indeed
compiler warnings in general, but there was nothing like that. Which implies
it will quite likely happen again.

------
MrDosu
I always wonder how these "five conditions ANDed together in an if" make it
past code review in general (i know thats not where the actual bug was). I
don't care how brilliant a programmer you are, but mistakes in those are very
difficult to grasp.

~~~
jacquesm
That's the equivalent of a 32 branch switch statement in one line.

So only 31 other cases to test for, now let's hope they did all those tests.

~~~
dtran
I'm definitely not advocating for unnecessarily complex or long conditionals
on a single line or any other hard-to-grok code, but this comment reminded me
of something I just learned recently! Someone on my team introduced me to
property-based testing, which generates ranges of test cases that would
otherwise be very repetitive to write manually. We've used
[http://hypothesis.works/](http://hypothesis.works/) in a few places recently
and caught bugs that we might have missed if we had just written tests the
usual way.

~~~
jacquesm
Were you using their python version or the Java one?

------
prance
I'm not too familiar with cryptocurrency or generally blockchain
implementations, so I wonder: shouldn't it be normal that at least the
reference clients of any such protocol should be proven correct, and follow a
rigorous scheme accepting contributions?

I mean, all the mathematical rigor and proofs of the underlying theory and
protocols are basically useless if the rigor isn't carried over to at least
the reference implementations. (If people choose to use alternative, non-
proven clients, fine, that's their own decision and risk.)

~~~
rebuilder
That's not really how the whole scene works. There's little upfront demand for
such rigour, because the potential userbase for any given coin largely don't
know to demand it. Instead, poor implementations fail and either get discarded
or patched, and eventually, hopefully, whatever remains will be solid.

What I'm trying to say here is, the whole cryptocurrency field is much, much
messier than you seem to expect.

~~~
prance
Well, maybe I was indeed too optimistic about a field that's both open source
and has the word "crypto" in it :(

------
known
"Programming today is a race between software engineers striving to build
bigger and better idiot-proof programs, and the Universe trying to produce
bigger and better idiots. So far, the Universe is winning" \--Rick Cook

~~~
Sunset
The whole world runs on ducttape and bubblegum and it's a small miracle that
satellites aren't falling out of the sky and everything isn't on fire all the
time.

------
abrkn
I spent quite a bit of time and effort on Zcoin during the launch, including
setting up the first block explorer and mining with hundreds of EC2 instances
to the point where I possessed more than 30% of the money supply.

It was a disaster. Some founders started selling their Zcoin as soon as it was
listed on an exchange. The mining algorithm was changed several times.

It's a shame. Zerocoin, the underlying technology, is interesting.

------
SCAQTony
If I were to put a soundtrack to that article it would be "The B-52's, 1983
hit, "Legal Tender." The thievery in industry never stops.

------
10129838131
Waiting for: IBM/Maersk blockchain bug caused 548,000 containers to end up in
Lagos.

------
davidgerard
In which a premined altcoin scam is outscammed because the preminers were
sloppy.

------
hodder
Serious question... is this a crime?

------
alienjr
Central banks have similar bug with government money.

------
eruditely
Is this displaying in a weird way for anyone else? I half thought that this
was a bug or something, some attack(?).

~~~
Sunset
Works ok. Try Archive.is

