Skip to content

Instantly share code, notes, and snippets.

@arpitsingh1409
Created August 24, 2022 13:41
Show Gist options
  • Save arpitsingh1409/a7fea54bae7b3e84e0a3d0028daac953 to your computer and use it in GitHub Desktop.
Save arpitsingh1409/a7fea54bae7b3e84e0a3d0028daac953 to your computer and use it in GitHub Desktop.
My solution for the Spearbit Writing Exercise.

Delegatecall to a malicious contract

Severity: High

Context: Implementation.sol#L18

Since Implementation.sol is a contract without any access control implemented, anyone who has address for it can simply call it's functions. Hence it's possible for someone to make a delegate call directly from Implementation contract to a malicious contract which has function which has selfdestruct() functionality by directly calling the delegatecallContract(address a, bytes calldata _calldata) function of the Implementation contract, thus destorying Implementation contract and rendering the whole Wallet Protocol as useless. So an attacker:

  1. Will create a malicious contract. A very basic one could be:
    contract Attack{
    constructor () {}
        function attack() {
            selfdestruct();
        }
    }
    
  2. Will call delegatecallContract(address a, bytes calldata _calldata) function at the address of Implementation contract with parameters as address a = address of Attack contract and bytes calldata _calldata = function selector for "attack()".
  3. Since delegatecall will run the code in context of the caller, attack() would be executed in context of Implementation and hence destoying it.

Recommendation: We can make "Imeplentation.sol" a library instead of a contract. Since libraries are stateless, it would be perfect to make "Imeplementation.sol" as a library as it must execute the code in caller's context without any scope for storage collision. Also, libraries cannot be destoryed. Since libraries cannot receive/hold ether, we must unmark both of the functions as "payable".

library Implementation { //modified one word -- "contract" --> "library"

    function callContract(address a, bytes calldata _calldata) external returns (bytes memory) { 
    // removed word "payable"
        (bool success , bytes memory ret) =  a.call{value: msg.value}(_calldata);
        require(success);
        return ret;
    }

    function delegatecallContract(address a, bytes calldata _calldata) external returns (bytes memory) { 
    //removed word "payable"
        (bool success, bytes memory ret) =  a.delegatecall(_calldata);
        require(success);
        return ret;
    }
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment