I have a withdraw function in my contract that I invoked and I was expecting the ETH to be transferred to the calling account but the funds went to someone else. I am trying to figure out if my contract was compromised, if so how could that have happened. Here is the transaction where I lost my ETH https://etherscan.io/tx/0xf878a257d3fe3274c52d8d44cb0b9d21d44d2bc9d48780c8c2c811dffeaad750
My withdraw function looks like this
function withDraw() public {
msg.sender.transfer(address(this).balance);
}
The transaction you linked did not move any value. It was run on a contract with no balance at the time, and the underlying transfer moved 0 eth as can be seen in Internal Txns → advanced
TLDR this is not an upgradability issue. A transaction was made on 20-03-2021 00:55:17 UTC. This transaction used the underlying logic, which is not verified on etherscan. This transaction was send, using gas tokens, through 0x3a518964ff40ee733d30749a213d2e5c9ffb2b8c. The proxy behaved correctly, the underlying implementation might be vulnerable.
What is interesting is, this is an inactive contract, hasn’t seen activity in months. On 20th, I tried withdraw through truffle console, it failed because my gas params were not right. I fixed it and tried again after few mins and it worked. So, the contract was vulnerable all along but got drained just around the time I tried to withdraw. That is strange.
I also called owner() on the contract at 0xe21c2aeafcedaf5d2f7ac24126ac713f72b8152a and it returns 0x00 which is also strange. I am using Ownable as you can see in the contract gist, guessing the owner somehow wasn’t set even though my account deployed it?
I am just trying to understand so I don’t make this mistake in the future, any thoughts @Amxx@abcoathup?
This piece of code has an unprotected withDraw() function. Anyone can call it ... so anyone can drain the funds. It is not designed to hold funds safelly for any amount of time.
What probably happened is that, when you tried to do a withDraw(), and while your transaction was waiting to be mined, someone saw the transaction in the mempool, and realized they could run it before you and get the funds. That is called “frontrunning”, and there is very sophisticated tooling that does just that.
So in a way, by doing submitting the withDraw() transaction, you sent a notification that this mechanism exists. This information was used by someone to steal you funds. This is possible because your implementation does not check who is trying to withdraw.
I had to leave the funds there because the contract purchases ENS domains on behalf of users and needs to pay for it. The obvious mistake is withdraw is unprotected. But thanks for helping me make sense of it. I knew about frontrunning but didn’t know such sophisticated tooling existed. Thanks again for your help.
