Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CRITICIAL SECURITY VULNERABILITY -- Hotfix #17

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

owocki
Copy link
Contributor

@owocki owocki commented Jul 8, 2018

There is a critical security vulnerability in the production smart contract for YGE deployed at 0x8bcaadc84fd3bdea3cc5dd534cd85692a820a692

If you scroll through the smart contract, you can see that there is an attacker who keeps draining the funds of the smart contract every time there is new ETH in there.

this is a fix for what i think is the vulmerability, though i am not completely sure. i believe the attacker is creating new transfers where they claim an _amount that is higher than msg.value, thereby tricking the smart contract into thinking it received a trasnfer higher than it actually did, and then exploiting that in the receive flow by withdrawing more than they deposited

IMPORTANT - it is inadvisable to use YGE in any non-permissioned environment until a full security audit is performed on it.

dimoncoin and others added 5 commits November 10, 2017 18:50
@owocki
Copy link
Contributor Author

owocki commented Jul 8, 2018

24fa3d3 warns users not to use the system until we can do a security audit

@Dobrokhvalov
Copy link

Hi @owocki

I was able to reproduce the hack - https://etherscan.io/tx/0xf0712c5b5c675d39711d4c0bfc622f2060845310a6341159c4a968d68a94f7e1

The current deployed contract allows to withdraw it's ether to an ephemeral address -

_owner.transfer(_fee_amount);

As there were no checks for the fee amount, caller was able to withdraw any fee to any address calling newTransfer function.

I think you have fixed the vulnerability by adding fee amount check -

require(_fee_amount < msg.value);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants