8 ms·

List of the top smart contract vulnerabilities

93 points by wepple 9y ago

aiCeivi9 9y ago
The timeline for the first one is just sad.
jjallen 9y ago
I'll never forget this pull request. Just having these two `internal` keywords there would have saved $150 million dollars. https://github.com/paritytech/parity/pull/6102/files#diff-8ea4aa7c2ba715c683bc764337f51585 https://github.com/paritytech/parity/pull/6102/files#diff-8e... A lot of people think "smart contracts" are inherently insecure. I personally believe that most of the "hacks" were very preventable, if those who had written the code slowed down to think about what they were doing.
- zitterbewegung 9y ago
  From the article they address your point. https://dasp.co/#item-10 https://dasp.co/#item-10 A lot of "hacks" are preventable in hindsight though. I think the real issue is that we are just beginning to secure smart contracts and its more about the process than the person who made a mistake. The tooling for Solidity is alright but most of the EVM and the languages associated with it are in their infancy. Formal Verification might be a solution here but we also have to consider the developer ergonomics of writing secure smart contract code.
  - deleted 9y ago
    [deleted]
- CJefferson 9y ago
  This reminds me of 'Poya-Yoke', which is (as I understand it) about making accidents less likely by design. Functions should be carefully chosen to be externally visible, so why not make internal the default. When designing contracts, the language should be making every possible effort to be safe by design.
  - jjallen 9y ago
    Totally agree about making `internal` the default visibility for Solidity/EVM functions. Should also require visibility to be declared (right now just compiler warnings happen).
  - pmoriarty 9y ago
    Related to this is the maxim to "make it easy to do the right thing, and difficult to do the wrong thing". When applied to programming, a first step towards that could be as simple as using a safer language, where it's more difficult to shoot oneself in the foot, and where all sorts of errors are prevented by default.
- cortesoft 9y ago
  Every single mistake in the world is preventable under your definition, though; every single mistake has an alternative action that was a non-mistake, and if only the people had thought more and chosen the non-mistake course of action the mistake would have been prevented. If a ‘mistake’ has no alternative that would have prevented it, it is no longer a mistake and it is instead a tragedy. This doesn’t help us prevent the mistake in the future, though. You can’t prevent bugs and vulnerabilities in code by just mandating that everyone think more before acting. This is why we have a rule during our learning reviews at work following an incident that the action item can’t be, “be more careful next time.” That is not an action item, that is magical thinking. We have a saying at my work - “plan for a world where we are just as stupid tomorrow as we are today”
  - thaumasiotes 9y ago
    > every single mistake has an alternative action that was a non-mistake This is only true for a very narrow view of what a "mistake" is. Classical tragedy is more inclined to the view that in some situations, there will be at least one mistake somewhere no matter what choices people make. In a more contemporary source, there's an episode of Friends in which the backstory is that, years ago, Phoebe married a gay ice dancer so that he could get a green card, and also because she was secretly in love with him. He comes back to ask for a divorce because he's learned that he isn't gay after all and he's fallen in love with another girl. This is painful for Phoebe, and she asks "Do you think, if you'd realized you weren't gay earlier, that we could have..." "Never mind, I don't think there's any answer you could give that would make me feel better."
- specialist 9y ago
  How wrong am I to equate "smart contracts" with a general purpose protocol negotiation thingie? Like TLS handshakes, but with code vs a spec? Or like a two party constraint satisfaction solver?
  - jobigoud 9y ago
    A smart contract can have persistent state, so it's more than just code it's also data. You can have a notion of users and privileges baked in the contract for example.
- ssdd 9y ago
  Its mostly these minute details that gets overlooked during code reviews which leads to bigger problems. Bigger design mistakes rarely happen.
seanwilson 9y ago
What ever happened with the frozen funds caused by the Parity multi-sig wallet bug? Is there a plan to unfreeze them somehow?
- jjallen 9y ago
  Original recovery proposal: https://github.com/Musiconomi/EIPs/blob/78abec4751c877e7a12a879787b2576cf5349b2b/EIPS/eip-867.md https://github.com/Musiconomi/EIPs/blob/78abec4751c877e7a12a...
  - seanwilson 9y ago
    What progress is being made though? This was a huge amount of money that was frozen. I would have thought something would have been done by now.
    - DennisP 9y ago
      At the moment there are no plans. There were some attempts to design an "upgrade" that would make the funds recoverable while benefiting the platform, but they had dangerous side effects. A lot of people opposed a targeted recovery fork, for reasons including: - It's not such a large amount that it's a systemic risk. - The hack was arguably enabled by negligence; the contract was changed after its last security audit, hacked, changed again and still didn't get a new security audit, and only after that the funds were frozen. Strong incentives to be more careful are probably good. Forking every time somebody's negligent could get messy. - The DAO hack involve an attack that was new to most people in the community, and even the tutorial code on ethereum.org was vulnerable to similar hacks. These hacks were more in the nature of simple oversights, enabled by overly complicated code. Good auditors would probably have found them. - The largest loss of funds was to the entity that made the contract (or a related entity anyway). That entity also said they had plenty of other money for their project. - Most of the remaining losses were to ICOs, who should have gotten competent advice to avoid this contract (given the first hack and lack of audit), and who've demonstrated fundraising ability so could conceivably get bailed out by their own investors. - Despite heavy criticism from certain quarters about Ethereum's supposed lack of immutability (after the DAO hack), immutability actually is a strong community value. Some people supported the DAO fix on the grounds that it was early days, but feel that the network is more mature now.
hritzdorf 9y ago
The new version of securify, which will soon be available at https://securify.ch https://securify.ch, catches 7 our of the 9 mentioned issues automatically and is free. I am one of the tool's authors. We went on to found ChainSecurity (https://chainsecurity.com https://chainsecurity.com) in order to deepen our research and understanding of smart contract security. Feel free to ask any questions about securify or ChainSecurity.
- seanwilson 9y ago
  Do you have any views on Solidity as a contract language and how suitable it is for being formally verified? Does it make any properties you want to check difficult or impossible to verify? My experience with formal verification is that the further a language is away from being a pure functional language with easy to check termination, the more difficult you make life for yourself. You really want a smart contract language that was specifically created to be easy to formally verify rather than trying to tack on formal verification later.
  - ptsankov1 9y ago
    Note that tools like Securify work directly on the EVM, and are therefore agnostic to the high-level language the contract is written in. The EVM itself is not as amenable to formal analysis (no types, explicit function calls, etc.).
- snissn 9y ago
  A bit confusing how to parse the audit. For example the output of this contract: https://etherscan.io/address/0xb6ed7644c69416d67b522e20bc294a9a9b405b31#code https://etherscan.io/address/0xb6ed7644c69416d67b522e20bc294... it's a bit opaque what lines of code you are warning of
  - hritzdorf 9y ago
    Thank you for the feedback. We will add some extra information. In this particular examples there seem to be no warnings. However, as I said the new version is coming soon.
asynchrony 9y ago
Please correct me if I'm wrong, but isn't it the nature of a turing machine (decentralized or not) to produce an endless series of "unforeseen" exploits?
- piracy1 9y ago
  I find that an interesting thesis but I don't quite understand how it's true. Could you expand on that?
  - asynchrony 9y ago
    Based on the halting problem, a provably complete analysis of the behavior of some (most?) programs in a turing-complete language is impossible. To put it another way, it is impossible to be sure that there isn't another vulnerability after you've patched the last one. I don't own any crypto assets myself, but I think that using a non-turing-complete scripting language for the bitcoin blockchain was a wise decision rather than a lack of vision.
    - cortesoft 9y ago
      The halting problem doesn't say that you can't do a complete analysis of a PARTICULAR program to see if it halts, it just says you can't have a general algorithm that would do this for ALL programs. Since we are trying to analyze particular programs, you aren't going to be limited by the halting problem.
      - asynchrony 9y ago
        If it's possible to do a complete analysis of any particular program, shouldn't it follow that it's possible to do a complete analysis of all programs? If only a subset of all programs can be completely analyzed as particular programs, what are the boundaries of that set?
        cortesoft 9y ago
jeffreyrogers 9y ago
I heard someone say that smart contracts don't eliminate the need for a trusted party, they just make that trusted party a program, since most people don't fully understand what these contracts are saying. At least when you get a lawyer to write-up a contract the lawyer is usually familiar with the law, and how similar contracts are interpreted by the court system.
homakov 9y ago
1. front running has nothing to do with TOCTOU/race conditions. the real race condition example would be reentry attack actually. 2. #9 has nothing to do with "off-chain issues". Off chain has very clear context and refers to layer 2 solutions for scaling. No need to call a website bug "off chain issue".

93 points by wepple 9y ago

The timeline for the first one is just sad.

I'll never forget this pull request. Just having these two `internal` keywords there would have saved $150 million dollars. https://github.com/paritytech/parity/pull/6102/files#diff-8ea4aa7c2ba715c683bc764337f51585 https://github.com/paritytech/parity/pull/6102/files#diff-8e... A lot of people think "smart contracts" are inherently insecure. I personally believe that most of the "hacks" were very preventable, if those who had written the code slowed down to think about what they were doing.

zitterbewegung 9y ago
From the article they address your point. https://dasp.co/#item-10 https://dasp.co/#item-10 A lot of "hacks" are preventable in hindsight though. I think the real issue is that we are just beginning to secure smart contracts and its more about the process than the person who made a mistake. The tooling for Solidity is alright but most of the EVM and the languages associated with it are in their infancy. Formal Verification might be a solution here but we also have to consider the developer ergonomics of writing secure smart contract code.
- deleted 9y ago
  [deleted]
CJefferson 9y ago
This reminds me of 'Poya-Yoke', which is (as I understand it) about making accidents less likely by design. Functions should be carefully chosen to be externally visible, so why not make internal the default. When designing contracts, the language should be making every possible effort to be safe by design.
- jjallen 9y ago
  Totally agree about making `internal` the default visibility for Solidity/EVM functions. Should also require visibility to be declared (right now just compiler warnings happen).
- pmoriarty 9y ago
  Related to this is the maxim to "make it easy to do the right thing, and difficult to do the wrong thing". When applied to programming, a first step towards that could be as simple as using a safer language, where it's more difficult to shoot oneself in the foot, and where all sorts of errors are prevented by default.
cortesoft 9y ago
Every single mistake in the world is preventable under your definition, though; every single mistake has an alternative action that was a non-mistake, and if only the people had thought more and chosen the non-mistake course of action the mistake would have been prevented. If a ‘mistake’ has no alternative that would have prevented it, it is no longer a mistake and it is instead a tragedy. This doesn’t help us prevent the mistake in the future, though. You can’t prevent bugs and vulnerabilities in code by just mandating that everyone think more before acting. This is why we have a rule during our learning reviews at work following an incident that the action item can’t be, “be more careful next time.” That is not an action item, that is magical thinking. We have a saying at my work - “plan for a world where we are just as stupid tomorrow as we are today”
- thaumasiotes 9y ago
  > every single mistake has an alternative action that was a non-mistake This is only true for a very narrow view of what a "mistake" is. Classical tragedy is more inclined to the view that in some situations, there will be at least one mistake somewhere no matter what choices people make. In a more contemporary source, there's an episode of Friends in which the backstory is that, years ago, Phoebe married a gay ice dancer so that he could get a green card, and also because she was secretly in love with him. He comes back to ask for a divorce because he's learned that he isn't gay after all and he's fallen in love with another girl. This is painful for Phoebe, and she asks "Do you think, if you'd realized you weren't gay earlier, that we could have..." "Never mind, I don't think there's any answer you could give that would make me feel better."
specialist 9y ago
How wrong am I to equate "smart contracts" with a general purpose protocol negotiation thingie? Like TLS handshakes, but with code vs a spec? Or like a two party constraint satisfaction solver?
- jobigoud 9y ago
  A smart contract can have persistent state, so it's more than just code it's also data. You can have a notion of users and privileges baked in the contract for example.
ssdd 9y ago
Its mostly these minute details that gets overlooked during code reviews which leads to bigger problems. Bigger design mistakes rarely happen.

seanwilson 9y ago

What ever happened with the frozen funds caused by the Parity multi-sig wallet bug? Is there a plan to unfreeze them somehow?

jjallen 9y ago
Original recovery proposal: https://github.com/Musiconomi/EIPs/blob/78abec4751c877e7a12a879787b2576cf5349b2b/EIPS/eip-867.md https://github.com/Musiconomi/EIPs/blob/78abec4751c877e7a12a...
- seanwilson 9y ago
  What progress is being made though? This was a huge amount of money that was frozen. I would have thought something would have been done by now.
  - DennisP 9y ago
    At the moment there are no plans. There were some attempts to design an "upgrade" that would make the funds recoverable while benefiting the platform, but they had dangerous side effects. A lot of people opposed a targeted recovery fork, for reasons including: - It's not such a large amount that it's a systemic risk. - The hack was arguably enabled by negligence; the contract was changed after its last security audit, hacked, changed again and still didn't get a new security audit, and only after that the funds were frozen. Strong incentives to be more careful are probably good. Forking every time somebody's negligent could get messy. - The DAO hack involve an attack that was new to most people in the community, and even the tutorial code on ethereum.org was vulnerable to similar hacks. These hacks were more in the nature of simple oversights, enabled by overly complicated code. Good auditors would probably have found them. - The largest loss of funds was to the entity that made the contract (or a related entity anyway). That entity also said they had plenty of other money for their project. - Most of the remaining losses were to ICOs, who should have gotten competent advice to avoid this contract (given the first hack and lack of audit), and who've demonstrated fundraising ability so could conceivably get bailed out by their own investors. - Despite heavy criticism from certain quarters about Ethereum's supposed lack of immutability (after the DAO hack), immutability actually is a strong community value. Some people supported the DAO fix on the grounds that it was early days, but feel that the network is more mature now.

hritzdorf 9y ago

The new version of securify, which will soon be available at https://securify.ch https://securify.ch, catches 7 our of the 9 mentioned issues automatically and is free. I am one of the tool's authors. We went on to found ChainSecurity (https://chainsecurity.com https://chainsecurity.com) in order to deepen our research and understanding of smart contract security. Feel free to ask any questions about securify or ChainSecurity.

seanwilson 9y ago
Do you have any views on Solidity as a contract language and how suitable it is for being formally verified? Does it make any properties you want to check difficult or impossible to verify? My experience with formal verification is that the further a language is away from being a pure functional language with easy to check termination, the more difficult you make life for yourself. You really want a smart contract language that was specifically created to be easy to formally verify rather than trying to tack on formal verification later.
- ptsankov1 9y ago
  Note that tools like Securify work directly on the EVM, and are therefore agnostic to the high-level language the contract is written in. The EVM itself is not as amenable to formal analysis (no types, explicit function calls, etc.).
snissn 9y ago
A bit confusing how to parse the audit. For example the output of this contract: https://etherscan.io/address/0xb6ed7644c69416d67b522e20bc294a9a9b405b31#code https://etherscan.io/address/0xb6ed7644c69416d67b522e20bc294... it's a bit opaque what lines of code you are warning of
- hritzdorf 9y ago
  Thank you for the feedback. We will add some extra information. In this particular examples there seem to be no warnings. However, as I said the new version is coming soon.

asynchrony 9y ago

Please correct me if I'm wrong, but isn't it the nature of a turing machine (decentralized or not) to produce an endless series of "unforeseen" exploits?

piracy1 9y ago
I find that an interesting thesis but I don't quite understand how it's true. Could you expand on that?
- asynchrony 9y ago
  Based on the halting problem, a provably complete analysis of the behavior of some (most?) programs in a turing-complete language is impossible. To put it another way, it is impossible to be sure that there isn't another vulnerability after you've patched the last one. I don't own any crypto assets myself, but I think that using a non-turing-complete scripting language for the bitcoin blockchain was a wise decision rather than a lack of vision.
  - cortesoft 9y ago
    The halting problem doesn't say that you can't do a complete analysis of a PARTICULAR program to see if it halts, it just says you can't have a general algorithm that would do this for ALL programs. Since we are trying to analyze particular programs, you aren't going to be limited by the halting problem.
    - asynchrony 9y ago
      If it's possible to do a complete analysis of any particular program, shouldn't it follow that it's possible to do a complete analysis of all programs? If only a subset of all programs can be completely analyzed as particular programs, what are the boundaries of that set?
      - cortesoft 9y ago

jeffreyrogers 9y ago

I heard someone say that smart contracts don't eliminate the need for a trusted party, they just make that trusted party a program, since most people don't fully understand what these contracts are saying. At least when you get a lawyer to write-up a contract the lawyer is usually familiar with the law, and how similar contracts are interpreted by the court system.

homakov 9y ago

1. front running has nothing to do with TOCTOU/race conditions. the real race condition example would be reentry attack actually. 2. #9 has nothing to do with "off-chain issues". Off chain has very clear context and refers to layer 2 solutions for scaling. No need to call a website bug "off chain issue".