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

Update the DetectionBot of DoubleEntryPoint challenge and the validateInstance of the DoubleEntryPointFactory #745

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

Conversation

Nfire2103
Copy link

I will speak about the DoubleEntryPoint challenge, the one which use Forta.

Currently, this is the solution of this challenge:

    function handleTransaction(address user, bytes calldata msgData) public override {
        // Only the Forta contract can call this method
        require(msg.sender == address(fortaContract), "Unauthorized");
        fortaContract.raiseAlert(user);
        msgData;
    }

With this solution, the method raiseAlert is called all the time which makes revert the delegateTransfer method at each call. In my opinion, it makes no sense to have a method which reverts always.

According to me, the real exploit is to call the delegateTransfer method from the CryptoVault contract because DoubleEntryPoint is an underlying token. It can happend if someone call sweepToken method with LegacyToken address in parameter. The DetectionBot must prevent of this case. So this is my proposition of solution:

    function handleTransaction(address user, bytes calldata msgData) public override {
        // Only the Forta contract can call this method
        require(msg.sender == address(fortaContract), "Unauthorized");

        // Decode the parameters of the delegateTransfer method
        (, , address origSender) = abi.decode(
            msgData[4:],
            (address, uint256, address)
        );

        // The origSender mustn't be the CryptoVault
        // because DoubleEntryPoint is an underlying token,
        // if so raise an alert
        if (origSender == cryptoVaultContract)
            fortaContract.raiseAlert(user);
    }

With this solution, the method raiseAlert is called only when the vulnerability is exploited.

Now, to prevent someone to solve this challenge with a DetectionBot which raises an alert all the time. I also updated the validateInstance method of the DoubleEntryPointFactory. Before trying to sweep token, the validateInstance method will try to emulate a lambda transfer of a user, if the transfer reverts, the validateInstance fails.

I updated unit tests to test my code. I also deployed the new DoubleEntryPointFactory in local environment to test it through ethernaut. Everything is working!

Do I have to push the new build of the DoubleEntryPointFactory contract?

Also, DoubleEntryPoint challenge is the only one which reverts when the validateInstance fails. It doesn't come from of my code, it was already there. Do you want me to fix that? (It's 2 lines)

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.

1 participant