Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upSecurify fails to detect the DAO reentrancy #106
Comments
|
If this can help to debug, here a "simplified" version of the DAO reentrancy, which is also not detected by Securify: contract Wallet{
address owner = msg.sender;
function cashOut(address account, uint amount) external{
require(owner == msg.sender);
account.call.value(amount)();
}
function () payable{
}
}
contract DAO{
Wallet w;
mapping(address => uint) balances;
constructor(){
w = new Wallet();
}
function buy() payable{
balances[msg.sender] += msg.value;
w.transfer(msg.value);
}
function bug() external{
w.cashOut(msg.sender, balances[msg.sender]);
balances[msg.sender] = 0;
}
} |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I have checked the following
soufflebinary is available, output when runningsouffle:solc --versionsolc --bin-runtime MyContract.solTested from the master branch aef12a3
Steps to reproduce
Hi,
The latest version of securify (aef12a3) fails to find the DAO reentrancy bug.
The run reports multiple potential reentrancies, but does not report the one responsible for the DAO hack.
As a recall (see http://hackingdistributed.com/2016/06/18/analysis-of-the-dao-exploit/), the bug lies in the function
withdrawRewardFor, a malicious attacker controlling_accountcan re-enter beforepaidOutis updated during the call topayOut:This was tested with Solidity 0.4.25, on this modified version of the DAO:
The modifications are:
The reentrancy results from Securify are:
But the reentrancy used during the hack is not detected.
Please let me know if I am missing something, or if you need more information.
Thanks!