Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Zcoin implementation bug enabled attacker to create over 500K Zcoins

makebitcoingreatagain.wordpress.com

Post reply on HN
Print view View original

131–140 of 223 posts

Re: Zcoin implementation bug enabled attacker to create over 500K Zcoins

#131

Post by yjgyhj »

petters wrote: post #96
yjgyhj wrote: post #71

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 Ocam…

As far as I can tell, this bug has nothing to do with memory-safety. And I don't think C++ is to blame here. It's just a lot of untested code. Surely even Ocaml and Haskell need tests.

Yes, this bug is not about memory safety - but having the state-writing operator so easy at hand was probably what lead to the mistake in the first place. In my GP comment I went on to rant about other classes of bugs which are obsolete since the 1970s, but still happen.

Re: Zcoin implementation bug enabled attacker to create over 500K Zcoins

#132

Post by caf »

lmm wrote: post #102
easytiger wrote: post #101

Earlier quoted context omitted.

This bug had nothing to do with c++ and its ergonomics, let alone it's safety. > Unfortunately when populating the denomination member variable, an equality operator was used instead of assignment, resulting in the denomination always being zero, as set in the class constructor. So = Vs ==

That is absolutely a bug in C++'s ergonomics. Languages that use pascal-style := for assignment would not have this problem. Languages that don't have the surprising value behaviour of = would not have this problem. Hell, even as crude a language as Java would not have this problem because it doesn't allow an integer to implicitly decay to a boolean. Building with warnings on would also have caught it, but the fact t…

This doesn't have anything to do with the value behaviour of =. The statement in question was intended to be a simple assignment, throwing away the value of the = operator and only using its side-effect. Instead an equality operator was used, which produced no side-effect.

So if you want to blame anything, blame the concept of expression statements, I guess?

Re: Zcoin implementation bug enabled attacker to create over 500K Zcoins

#133

Post by caf »

yjgyhj wrote: post #130
easytiger wrote: post #101

Earlier quoted context omitted.

This bug had nothing to do with c++ and its ergonomics, let alone it's safety. > Unfortunately when populating the denomination member variable, an equality operator was used instead of assignment, resulting in the denomination always being zero, as set in the class constructor. So = Vs ==

Yes it is. Assignment is another word for writing state. If your programming environment doesn't include a way to set state, you will never do it. Why is doing the dangerous task of writing state so easy you just have to hit one key to do it in C++? Let's collectively put our seat belts on, colleagues!

The bug here was a lack of writing state!

Re: Zcoin implementation bug enabled attacker to create over 500K Zcoins

#134

Post by joosters »

simias wrote: post #107
jacquesm wrote: post #72

Earlier quoted context omitted.

> Other people might be using different compilers that emit warnings for things yours didn't. You don't want the build to fail for those people. Actually, you do. That way those things will be fixed and hopefully your inclusion of their fixes is a well motivated PR away. Switching -Werror off should be a decision made with great care and understanding of what's going on under the hood. If there are platform specific…

I want to agree with you but in practice I can't really imagine switching -Werror outside of dev builds. The problem is that different compilers have different warnings and if your code is meant to be portable it's going to be a serious pain to avoid all the warnings all the time. Having compilation break for a user because of a spurious warning due to a different compiler (or different compiler version) would make i…

warning: comparison is always true due to limited range of data type

Surely this is a serious problem on 16 bit builds; the comparison implies that larger values are expected to be stored in this variable, the warning is a strong hint that the (size of the) types are incorrect.

Re: Zcoin implementation bug enabled attacker to create over 500K Zcoins

#135

Post by Sanddancer »

TeMPOraL wrote: post #124
ifdefdebug wrote: post #113

Earlier quoted context omitted.

Didn't try, but I could imagine that running -Wall over that codebase produces huge amounts of noise rendering it practically useless, until somebody gets to fix all warnings which are not bugs.

Doing C/C++ without -Wall, and/or not continuously fixing the things it shows[0], is pretty insane to me. The compiler is there to help you. -- [0] - even if by ignoring them, when you're absolutely sure what you're doing.

Yep. I'd add that if it's a warning you're absolutely certain of, use the warning control pragmas to suppress them, and document why the warning's suppressed. Keep the compiler output clean so you can see where you need to examine.

Re: Zcoin implementation bug enabled attacker to create over 500K Zcoins

#137

Post by rebuilder »

prance wrote: post #69

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…

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.

Re: Zcoin implementation bug enabled attacker to create over 500K Zcoins

#138

Post by yjgyhj »

riprowan wrote: post #120
yjgyhj wrote: post #71

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 Ocam…

What language can be better peer-reviewed?

A purely functional statically typed one, like OCaml or Haskell, because there would be so much less to review, and an insane amount of the reviewing work is automatic.

Zero bugs around state, zero bugs around memory mgmt, zero bugs about error conditions not being handled, zero bugs due to some data being expected but not being written, etc.

Re: Zcoin implementation bug enabled attacker to create over 500K Zcoins

#139

Post by TeMPOraL »

Sanddancer wrote: post #135
TeMPOraL wrote: post #124

Earlier quoted context omitted.

Doing C/C++ without -Wall, and/or not continuously fixing the things it shows[0], is pretty insane to me. The compiler is there to help you. -- [0] - even if by ignoring them, when you're absolutely sure what you're doing.

Yep. I'd add that if it's a warning you're absolutely certain of, use the warning control pragmas to suppress them, and document why the warning's suppressed. Keep the compiler output clean so you can see where you need to examine.

Exactly. That's what I meant by "ignore"; "suppress" was the word I was looking for.

Re: Zcoin implementation bug enabled attacker to create over 500K Zcoins

#140

Post by yjgyhj »

caf wrote: post #132
lmm wrote: post #102

Earlier quoted context omitted.

That is absolutely a bug in C++'s ergonomics. Languages that use pascal-style := for assignment would not have this problem. Languages that don't have the surprising value behaviour of = would not have this problem. Hell, even as crude a language as Java would not have this problem because it doesn't allow an integer to implicitly decay to a boolean. Building with warnings on would also have caught it, but the fact t…

This doesn't have anything to do with the value behaviour of =. The statement in question was intended to be a simple assignment, throwing away the value of the = operator and only using its side-effect. Instead an equality operator was used, which produced no side-effect. So if you want to blame anything, blame the concept of expression statements, I guess?

You can rightfully blame having state setting be part of your language. It is easier to program without state than with it, just like it's easier to juggle 2 balls than 3. Without state (or with the absolutely minimum of it) you can not make mistakes with it. Also, if your state-setting operator is super similar to your equality-checking operator, you can fall into this trap (like the devs in question did).

Programming without state is something that has helped me personally make my code much better. I produce less bugs by far since I stopped relying on state for so many things. A lot of micro-optimisations are not doable for me. In exchange I guess I get to run things in parallell very easily. But mostly, I get fewer bugs, which makes my life and the lives of my customers better.

Post reply on HN