
Deconstructing the DAO Attack: A Brief Code Tour - bpierre
http://vessenes.com/deconstructing-thedao-attack-a-brief-code-tour/
======
RobertoG
I'm going to be a little polemic here.

It's this an attack?

It's not the problem that those technologies try to solve, to get rid of
subjectivity?. In a way, this could be interpreted as trying to be free of
politics.

If my understanding of what they are trying to accomplish here is correct, if
the system allow it, then, by definition, it's legal.

If you require a framework where something allowed by the code but with
unexpected consequences is illegal, you are again where you started.

~~~
wyager
You're being thoroughly downvoted, but I don't see anything wrong with your
argument. The person who pulled off this "hack" was following all the rules.
In fact, I could even see the argument that by undoing this transaction, the
people who started the DAO are in fact going back on their word. They promised
that they would build a system which would obey certain rules. By undoing this
transaction, they are no longer following those rules. Now, obviously there's
a strong argument for rolling back the transactions, but at this point
ethereum has lost any claim to objectivity or decentralization.

~~~
developer2
>> obviously there's a strong argument for rolling back the transactions

Absolutely not. Such an act would entirely destroy the credibility of the
entire system and its future. If _not_ rolling this back is detrimental to the
system, then it has already failed and should be dismantled. Who would
continue to use a blockchain that permits such a thing? It completely defeats
the purpose.

------
winteriscoming
The apparent typo is odd, IMO. I don't know the language in which that program
is written, but looking at that snippet, "Transfer" takes 3 arguments whereas
the "transfer" function takes 2 arguments. Isn't there any code review
involved when this makes into the codebase. Assuming there was some code
review and the reviewer just missed it (which is very much possible), a basic
unit test case would have easily caught this bug, isn't it? After all, the
test case would have checked for the balance etc... after execution of these
functions. Aren't there any test cases around this?

~~~
vessenes
I think after sleeping on it that I agree with you, this is more likely the
outcome of bad refactoring. You can read the code commits on github if you
like; I don't have them handy, unfortunately.

transfer is a very poorly named function, of that there is no doubt. And
Transfer is badly named as well.

transferAndLockTokens vs LogTransfer would be much, much better.

------
jacquesm
Whoever thought it was a good idea to have case sensitive function names where
the names are allowed to be identical but not identical in function?

Major fuck-up there.

That should have never passed the concept stage, nor the review stage.

Function names should describe what a function does.

~~~
rbobby
Case sensitivity in any programming language is crazy. I can't for the life of
me see a valid engineering principal that accepts IsTheOne() and istheone()
being different bits of code. Oh sure at a technical level the __computer
__has no problem with... the problem is restricted to those oh so error prone
humans.

Can anyone here honestly say that if they were doing a code review they'd
agree that solely a difference in case would get their approval?

And don't even get me started on filenames that only differ by case. If that's
a good idea then I think trailing whitespace should be significant too!!!

~~~
kartickv
I don't know why you've been downvoted, but anyone who disagrees with you
needs to explain in what situation it would make sense to have two different
functions or files with names differing only in case.

I upvoted you.

~~~
wnewman
I have used all-uppercase to make a distinction like class vs. instance in
variables (in case-sensitive languages in which the class might be an ordinary
held-in-a-variable value, like Javascript), and I might do it again. But it's
very unusual, and it's also the kind of practice that is more suitable for a
1KLOC project that will receive 100 hours of effort from a single maintainer
over its lifetime than for a bigger project with many maintainers and a highly
motivated community of attackers.

I don't think there was ever a case when I was tempted to name two functions
with different cases, but if I ever had to write a modest-sized 1-maintainer
system in which many functions came in exactly two different flavors, I might
be tempted. (Perhaps threadsafe locked vs. raw? or some C++-like distinction
between raw functions and closure-like class instances which can be used in a
function-call context? or raw functions vs. wrappers with the extra plumbing
required to let them be invoked from a scripting language?)

afterthought: And now that I think of it, in old C code I think I vaguely
remember working with macro vs. function implementations of the same operation
distinguished by capitalizing the name, and I don't think the name convention
was an urgent problem. C macros can breed various errors, but I think
bitbang_macro vs. bitbang would breed pretty much the same errors as BITBANG
vs. bitbang.

~~~
kartickv
In those situations, it would have been much more readable to have classFoo vs
foo, foo() vs nonThreadSafeFoo(), etc.

The bigger point is that while you can come up with creative ways to take
advantage of case-sensitivity, it's not that you would have missed it if the
language was case-insensitive. From that point of view, case-sensitivity has
no benefit, but only a cost: leads to irritating errors from the compiler, or
runtime errors in dynamically typed languages.

If something has no benefit, and only a cost, we should get rid of it.

------
seibelj
Bored computer scientists: analyze contracts that are less popular but still
have money attached to them and see if you can locate a bug. Apparently you
can get the money legally and the only way to stop you is to hard fork the
entire currency.

------
lordnacho
It reads like the debugging that you've done a million times if you are
working with ordinary, mutable, C-family code.

To start off with, you think you're developing a valuable skill in being able
to follow a call stack along with a bunch of variables. It certainly has its
uses, but after a while you come to wonder why it always ends up this way.
Sure, you can understand every bug eventually, but surely there's some reason
why you're constantly reasoning about the state of the variables and the order
of the function calls. You can write unit tests, but of course you can't think
of all the undesired behaviours beforehand.

The author does mention that something functional with strong types is
necessary. Probably a good idea.

As for the split function, there's a question of whether it is really
necessary. I wouldn't have thought an investment fund would need a split
function. An ordinary fund doesn't; if you're in, you're in for whatever
proportion that you've put in. Why not just make a second fund with the same
(reduced) code if you have some other group of investors? More functions =
bigger attack surface.

------
tdaltonc
It looks like this is a typo. The question is, "does that matter?" It seems to
me that either the code means what it says, or it doesn't. If a cabal of
humans are able to overrule the machines, then what is the point of Etherium?
Wouldn't we just be trading one group of authorities for another?

I want to see Etherium work, and it is extremely unfortunate that basically
THE first high profile smart contract has gone sideways. But I'm not sure if I
want there to be anything we can do about it. The whole point of this project
is that the blockchain is supposed to be like an indifferent force of nature.

------
harmegido
I'm curious, are there no tests for the code running this? It seems like some
of these bugs would be caught with them (especially the typo). And it seems
like insanity to not have them.

------
jcfrei
So, this review makes me wonder: Will the hard fork of the ethereum blockchain
be of any use at all? The DAO appears to be fundamentally flawed, are all the
coins there simply lost? What prevents an attacker from exploiting the DAO
after the hard fork? Or will they simply invalidate all the transactions that
went to the address of the DAO?

~~~
tdaltonc
As I understand it: The goal of the fork is to freeze the DAO account starting
just before the 'split' drain (at the blockchain level). And then figure out a
long term solution (possibly using another fork to return all ETH to the
original contributing accounts dissolving the DAO?).

The ability of a core of developers and miner to exercise their will over the
system like this is alarming though.

[https://blog.ethereum.org/2016/06/17/critical-update-re-
dao-...](https://blog.ethereum.org/2016/06/17/critical-update-re-dao-
vulnerability/)

------
winteriscoming
_This function will reduce user balances, before the vulnerable withdraw
function is called. So, instead of the logging function, we should have:

if (!transfer(0 , balances[msg.sender])) { throw; }

This would <snip>.... also reduce the tokens available to the user later on_

The more I think of the typo and the explanation about it in that article, the
more unclear I am about that whole code.

Keeping aside this hack, for a moment, if that is indeed a typo and instead it
should have called the lower case transfer method to really reduce the user
tokens, then does this mean that all these DAO contracts (or whatever the
right term is) that have been executed till date have been affected by this
and as such the entire eco-system state was already messed up before this
hack?

Nothing in that article suggests that this code flow, where this apparent typo
is present, is applicable only for this specific hack.

~~~
caf
No, because after the call to Transfer() and withdrawRewardFor(), it sets
their balance to zero:

    
    
      totalSupply -= balances[msg.sender];
      balances[msg.sender] = 0;
      paidOut[msg.sender] = 0;
    

So the bug is only exposed if the recipient of the reward transfers out the
balance during the execution of withdrawRewardFor().

In fact, to me it doesn't look like there's a typo at all there - the call to
Transfer() was fully intended just to log the burning of the tokens that will
be accomplished by the later lines setting the balance to zero. The bug is
simply that the balance isn't adjusted before the attacker gets a chance to
execute code.

~~~
htns
The top comment on /r/ethereum agrees with you on it not being a typo:
[https://www.reddit.com/r/ethereum/comments/4onbkj/deconstruc...](https://www.reddit.com/r/ethereum/comments/4onbkj/deconstructing_thedao_attack_a_brief_code_tour/)
. Hopefully in a few days we will have an authoritative explanation. Otherwise
this might become a HN urban legend.

------
amaks
The biggest problem I see with the code at
[https://github.com/slockit/DAO/](https://github.com/slockit/DAO/) is absense
of the unit tests. The code looks complex, how the hell can you make sure that
it's right?

The Solidity wiki doesn't have a single entry on testing either:

[https://github.com/ethereum/wiki/wiki/The-Solidity-
Programmi...](https://github.com/ethereum/wiki/wiki/The-Solidity-Programming-
Language)

~~~
graup
The repo you linked does have a "tests" directory. It contains "scenarios", so
not really unit tests.

You can't really unit-test contracts at the moment.

------
htns
I don't get how the transfer vs Transfer typo is supposed to have enabled a
_larger_ attack. The token balance is zeroed in the splitDAO function anyway,
and presumably the sanity checks in transfer are also redundant, or else there
would have been no need for recursion.

------
altern8tif
So all this hoo-ha is really just because of a capitalised "T"? If the code
were fixed, is the DAO model still valid in the real world?

(I'm a cryptocurrency noob... Some of technical stuff just went over my head)

~~~
kolinko
The real question is whether smart contracts can ever be safe enough so that
they have real world applications.

------
beachstartup
if you could 100% accurately and reliably model a contractual understanding in
any kind of language, it would have been done eons ago. human language (i'm
fond of english in particular), is extremely expressive.

but there's a reason we still have courts and judges. it's because language is
the tool through which humans express their desires and fears (computer
language included, because it is written by humans, so far), it's not a
machine.

~~~
tdaltonc
I don't think that there was any pressure to deeply formalize contrasts before
this because even if you did, there would still be recourse to a human judge
(not just a compiler). If one party didn't like the strict reading they could
sue.

The question that's playing out right now is "is there human judge behind the
code." If a set of humans (miners and developers) successfully overrule the
code, this will seriously undermine the whole premise of this project.

------
willtim
Modelling financial contracts in an imperative event-driven paradigm just
seems like an accident waiting to happen.

EDIT: To the downvoters, if you disagree, I would very much like to understand
why, please reply with a comment.

EDIT2: My position is that it is very difficult to reason about correctness
and maintain invariants in a highly imperative setting. IMHO, it would be more
desirable to use a declarative, or functional language. There are many
successful commercial applications of functional programming to financial
contracts, e.g. Lexifi, which is based on a subset of OCaml and various in-
house solutions developed by banks. Using a functional programming language
would also mean the code is far more amenable to formal methods and static
verification. One new startup I was very impressed with,
aestheticintegration.com, has had success rewriting financial algorithms in a
functional programming language in order to prove and verify numerous
correctness properties about them. It is a huge shame that folk do not reach
straight for the functional programming toolbox when faced with these types of
problems.

~~~
pron
I think you are placing far too much emphasis on language, especially as so
far there's little evidence that some programming approaches yield less
logical errors than others. I often see a similar sentiment expressed by
people who are familiar with functional programming but unfamiliar with
software verification, and may be unaware that nearly all verified software in
the industry is written in imperative languages. I know many FP people like to
believe that the local guarantees made by their chosen languages end up making
their programs more globally correct; it may well be true that some languages
make some errors easier to spot for humans (although we have little evidence
to suggest that this is indeed the case of FP). However, as far as _formal
verification_ is concerned, there is very little overlap between verified
software and FP.

Ensuring correctness -- i.e. conformance to a specification -- is, in general,
a task that requires effort -- by man and/or machine -- proportional only to
the complexity of the algorithm verified (i.e., the number of states reachable
by a TM implementing it) and almost not at all to some linguistic properties.
That is not to say that languages cannot prevent local errors that may be
catastrophic, but the difference between the best of languages and the worst
of languages when it comes to verifying spec-compliance is not that big.

> Using a functional programming language would also mean the code is far more
> amenable to formal methods and static verification

That is not true. If anything, imperative languages have better formal
verification tools than functional ones. And besides, for global program
"correctness" properties, the choice of a language more or less amenable to
formal reasoning may make a difference that is surprisingly small.

~~~
mbrock
I agree that functional vs imperative is a red herring in this case.

The Ethereum model is "imperative" anyway, so a functional DSL would probably
end up looking like some kind of monad.

That said, it seems interesting to use a language like Agda for writing a DSL
to EVM compiler, regardless of the exact nature of that DSL. Then you can have
machine checked compiler correction proofs and also prove theorems about DSL
programs.

~~~
pron
That would be interesting and even necessary. But why not use something much
easier than Agda, though not less powerful for this use case -- like TLA+ --
something that even "ordinary" engineers could use to verify their contracts
(not to mention that it supports model-checking in addition to deductive
proofs, which reduces the verification effort considerably)?

~~~
mbrock
Maybe that would be better. I admit that I don't know the first thing about
TLA+. Agda is what I learned about in school, and what my computer science
friends are excited about, and I was already interested in using it to write
verified compilers. Writing compilers with functional languages was also a
part of my university education. But that's just my background.

------
tinco
In addition to this the language is not explicit enough. Even standard Haskell
wouldn't be safe enough for a program that manages $250MM directly without
safe guards.

In addition to typesafety, it would have to be both explicitly typed at the
lowest possible granularity and annotated with pre and post conditions.

Not only did this code allow a bunch of tokens be transferred without
compensation, it left the whole accounting system in a corrupted state! One
that could have easily been detected and verified at any point.

This line:

    
    
        ```
            Transfer(msg.sender, 0, balances[msg.sender]);
        ```
    

Should have looked like this:

    
    
        ```
            // postcondition: balances[msg.sender] == 0
            Transfer(msg.sender, 0, balances[msg.sender]) :: Action<Transfer>;
    
        // Compiler result:
        // # Error 1: line XX expected type Action<Transfer> but instead got type Transfer
        // # Error 2: line XX violates postcondition, expected balances[msg.sender] == 0, got balances[msg.sender] != 0
        ```
    

And the whole function 'splitDao' should have a type annotation that
completely captures the possible actions it might encompass, and pre- and
postconditions that at the very least constrain the result of the function to
not corrupt the accounting of the DAO (i.e. it should still have as many
tokens as it believes it has).

It's nice that Vitalik Buterin is a genius, but it shows that this guy is only
22 and dropped out of university because anyone with a degree in computer
science knows about this stuff and its importance in high reliability systems.
He should have known the contract language should have had typechecked side
effect and he should have known proper pre- postconditions would have been the
least he should have done.

~~~
dang
> _It 's nice that Vitalik Buterin is a genius, but it shows that this guy is
> only 22 and dropped out of university because anyone with a degree in
> computer science knows about this stuff_

Personal attacks, which this crosses into, are not ok on Hacker News. Please
edit such stuff out of your comments here.

We detached this subthread from
[https://news.ycombinator.com/item?id=11928562](https://news.ycombinator.com/item?id=11928562)
and marked it off-topic.

~~~
tinco
Hi Dang, my apologies, I can no longer edit the comment, please feel free to
snip that paragraph out.

I guess I felt the need to accuse someone as I felt frustrated. News like this
really smears not only cryptocurrencies but software engineering in general. A
single typo cost these people $50M is what they claim, but the reality is that
the whole system was recklessly engineered. I guess I'm also saddened that so
much money was put behind something that was so clearly recklessly engineered.

Would it been a crazy idea just to invest a million or two of the $250M to pay
a Formal Methods team at any university in the world to give the language a
quick do-over?

------
fabled_giraffe
I know I'll get napalmed for saying this, but why is this all over HN? Why are
people obsessed with cryptocurrencies?

It's bad enough that those with sufficient computing speed/power can already
rip off the stock market; why does the world need another way for people to
rip each other off?

Since we've determined over history that some people will take advantage of
weakness for their own gain, no matter what system you create to transfer
goods, resources, services or value representing those, it will be exploited.
So- why even spend time on it?

If everyone who cares to maintains their own accounting data does so,
regardless of what technologies we are using, we really could just trade in
goods, services, and coin and completely get rid of all virtual currency.
Investments could go back to literally to providing coins, goods, resources,
or services to those that we'd like to support possibly in exchange for
reciprocity.

Since that won't happen right away, if you really want to secure your
financial future, major S&P 500 index tracking funds have been one of the
smartest things over its history to invest in:
[http://finance.yahoo.com/echarts?s=%5EGSPC+Interactive#symbo...](http://finance.yahoo.com/echarts?s=%5EGSPC+Interactive#symbol=%5EGSPC;range=my)

While some of you made money on bitcoin, many lost. Now the same game has been
played again with people believing that someone can write code to replace our
currency system and if they get in early enough, they'll be kings or queens.
Well- again, it didn't happen. What really is to gain from this fantasy world
of cryptocurrencies? It is harming more than it is helping, from what I see.

And if you downvote this, tell me why, otherwise I'll just assume you are with
those that want to exploit me and others.

~~~
xenophonf
Even though I disagree with some of your conclusions, I personally have no
problem with your comment except for the very first and very last sentences. I
generally downvote people who complain about submissions or about their
comments being downvoted, which I view as unproductive.

~~~
dhimes
I feel the last sentence, unfortunately, is relevant. Too many people now just
downvote without explanation, and that is not helpful to the conversation. In
something like this, which is a question disguised as a rant or vice versa, an
explicit request is, unfortunately, necessary today.

------
homero
Glad I never touched alts. My bitcoin are safe and increasing

