Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Activity
  • Feb 28 2019 19:45
    @Arachnid banned @merkle_tree_twitter
  • Feb 17 2019 00:56
    @jpitts banned @Aquentson_twitter
  • Apr 09 2018 04:14
    @holiman banned @JennyJennywren_twitter
  • Oct 21 2017 19:12
    @Arachnid banned @Musk11
chriseth
@chriseth
The slither code to check for constantinople reentrancy looks rather complicated to me. If we base a decision on the fact that it finds no vulnerable contracts, it should be taken into account that it might have missed something.
Michael - Blurpesec
@blurpesec
I don't know enough about solidity to verify
chriseth
@chriseth
In general, I would rather base a decision on the fact that this has not been mentioned on the EIP at all and thus was not discussed.
Dan Guido
@dguido
@ptsankov_twitter : yes, Slither works on Solidity, not EVM. We won't be able to modify Echidna or Manticore in time for the fork (our two EVM-based tools).
Ghost
@ghost~594787f3d73408ce4f685c42
@hahnmichaelf I feel like the attacking contract in that example is wrong
Martin Holst Swende
@holiman
The reddit post is wrong, it's the entier transaction
Petar Tsankov
@ptsankov_twitter
what we can do at ChainSecurity is run on all deployed contracts at EVM and list who may be affected, but of course it will be impossible to check if they are exploitable by staring at the EVM
Péter Szilágyi
@karalabe
OK, I checked against a live Rinkeby and a fake mainnet node (@holiman pls double check):
  • Downgrading to Geth 1.8.19 before the fork will indefinitely postpone the fork (verified with a new mainnet node).
  • Downgrading to Geth 1.8.19 after the fork will indefinitely postpone the fork, roll back to block 7079999 and sync from there (verified with an already forked Rinkeby node, fast sync resumption worked too).
  • Running Geth 1.8.20 with --override.constantinople=9999999 before the fork will postpone the the fork to block 9999999 (verified with a new mainnet node).
  • Running Geth 1.8.20 with --override.constantinople=9999999 after the fork will postpone the the fork to block 9999999, roll back to block 7079999 and sync from there (verified with an already forked Rinkeby node, fast sync resumption worked too).
  • Mist and non-power users cannot reasonably be expected to do either, so we need a hotfix release too in case we postpone.
I can prep the release in a couple of hours if need be
5chdn
@5chdn
Parity need to release anyways, so it makes sense to just communicate the latest version numbers
Micah Zoltu
@MicahZoltu
Agreed, for the average user "download latest, here is a link" is much easier than CLI options. Downgrading isn't too bad, though can be slightly more confusing than "get latest".
Taylor Monahan
@tayvano

It sounds like regardless, geth and parity need to push new releases if we want to delay the fork.

It is possible for node operators running geth to run geth with a specific flag if they don't want to update

Péter Szilágyi
@karalabe
Yes, we'll definitely push releases too if we decide to postpone
Just listing the options here
Corey Petty
@corpetty
Do we have a current odds of postponing. Are we leaning on a given side?
Péter Szilágyi
@karalabe
I'd say the odds are 50-50% with an error rate of about 50% (i.e. no idea) ;P
Taylor Monahan
@tayvano
I believe people are taking an hour or so to try to determine probability of risk before making that decision (while geth / parity teams are preparing for delay). my impression is about 50/50 right now as well
Corey Petty
@corpetty
yeah, waiting on verification from blockchain scans, wasn't sure what was the threshold of exploitability that pushes us to postpone/not-postpone
Michael - Blurpesec
@blurpesec
Its mostly just comparing risk. What risk is there to postponing at the last minute (network split, or worse - doublespends).
Jochem Brouwer
@jochem-brouwer
My two cents: if the attack is verified then the hard fork should be delayed. Many developers have assumed in the past that transfer and send are safe against re-entrancies. It is just time until contracts are found which can be attacked. This is not fair and will reduce trust in the Ethereum network. If it is implied (in many documents) that these functions are re-entrancy safe and that suddenly these are unsafe against these attacks this will reduce trust in the network a lot since the way how things work apparently can change over time.
Bruno Škvorc
@Swader
^ this. Never mind custom made multisigs by Bob or Alice, if things like Compound.Finance or MakerDAO's CDPs suffer because of this, these enormously successful and growing DeFi projects will be left with no choice than to say "Okay, guess the logic of the program can change, we can't make it work here, let's leave". (illustrative example, those are probably safe). We definitely don't want another DAO or several.
Danno Ferrin
@shemnon
Don’t most DeFi contracts have a pause button? how many have burned their pause keys?
Taylor Monahan
@tayvano
On the call there is talks of using a collaborative sheet for analysis of smart contracts so people aren't duplicating work. I’m not 100% of what the column headings should be but here’s the sheet that we can all use (feel free to be opinionated on what the headings should be): https://docs.google.com/spreadsheets/d/1GS98k8YosBsqV1UVq57vX-PTOUAcCCcRjJ847H8Y4hw/edit?usp=sharing
Jochem Brouwer
@jochem-brouwer
I have run the ChainSecurity tests on ropsten. They pass
Aron
@cobordism
@jochem-brouwer can you elaborate? What does that transaction show?
Jochem Brouwer
@jochem-brouwer
I'm trying to verify the contracts
This is the test from the ChainSecurity truffle
Geoff Hayes
@hayesgm
FYI, Compound Finance avoids transfer and send and opts for wrapped ERC-20 instead, so we're safe there. That said, I agree it could be difficult for contracts that did depend on the limited gas stipend.
Jochem Brouwer
@jochem-brouwer
@hayesgm I agree that even with transfer and send people should not have re-entrancy attacks available even if that doesnt work due to gas. But, I think most people have always had in the back in their heads that it is never possible to change storage using transfer or send which is hence now possible
I guess it's not even limited to one slot but actually multiple since every storage slot write which is dirty is 200 gas
I am pretty sure that if we check etherscan we can find some contracts which hold a small amount of eth which are now suddenly drainable due to the hard fork
Nick Johnson
@Arachnid
Well bugger. It's worth thinking about the larger implications here; do we really want to be stuck with requiring all state modifying operations to cost more than the gas stipend? That puts a serious crimp in any optimisation effort, not just this one.
It's definitely something people use at least as a safety net, though, if not more than that.
Micah Zoltu
@MicahZoltu
@Arachnid The current plan is to avoid that conversation right now.
Aron
@cobordism
@jochem-brouwer I was just confused about what does it mean for the test to "pass".
Jochem Brouwer
@jochem-brouwer
I strongly suggest that the fork is delayed because this would honestly raise a lot of confusion in the development community. If it is strongly implied some things are not possible but due to a hard fork it is suddenly possible that is really discomforting and reduces a lot of trust. Still want to mention the Core Dev team has done an amazing job but I also want to thank ChainSecurity for raising this point now (although its really short notice) - there now is some time to fix this.
@homotopycolimit it confirms that a re-entrancy attack is possible on constantinople using transfer/send
Micah Zoltu
@MicahZoltu
We don't have enough time to make a decision other than "to Constantinople or not" right now, not enough time to discuss the long-term ramifications and solutions.
Nick Johnson
@Arachnid
One simple analysis to find potentially vulnerable contracts is just to look for anything that makes a transfer or send and then changes state or makes a less-gas-constrained external call.
That'll have lots of false positives, but it's a start.
Matthias Egli
@MatthiasEgli_twitter
This is finding tons of contracts
Nick Johnson
@Arachnid
@MicahZoltu Fair enough.
Taylor Monahan
@tayvano
@hayesgm which compound address(es) did you check? I'll add them to the working list
Geoff Hayes
@hayesgm
@tayvano 0x3fda67f7583380e67ef93072294a7fac882fd7e7 is our core Money Market contract. I can list out the rest, though not a single contract of ours uses send() or transfer()
Taylor Monahan
@tayvano
Thanks
Jochem Brouwer
@jochem-brouwer
What about an auction contract where the price of an auction is updated /after/ the transfer? This would be attackable