# Report

## Funds would remain locked in the Proxy contract for perpetuity.

**Severity**: High

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

Attacker can call `delegatecallContract()` in the `Implementation` contract as it doesn't have any access restrictions that gives an opportunity to the attacker to `selfdestruct` the `Implementation` contract that leads to locking of funds within the `Proxy` contract forever as there would be no code exists at the Implementation contract address to execute any functionality.

Attacker can write a simple `Attack` contract, Something like below

contract Attack {
    function foo() external payable {
        address payable to = payable(msg.sender));
        selfdestruct(to);
    }
}

And call the `foo()` function using the `delegatecallContract()`, Due to the nature of the `delegatecall` it preserve the context (i.e storage, msg.sender) of the calling contract, It will execute the `selfdestruct()` in the context of the `Implementation` contract that leads to removal of the code from the chain for the next blocks.

**Recommendation**: The reason that anybody can call the `delegatecallContract()` function of the `Implementation` because there is a lack of the access ownership. So the solution is to have some kind of access lock which would restrict the direct usage of the `Implementation` contract function.

function delegatecallContract(address a, bytes calldata _calldata) payable external returns (bytes memory) {
        if (_implementation != address(0)) {
            error Error_NotAuthorized();
        }
        (bool success, bytes memory ret) =  a.delegatecall(_calldata);
        require(success);
        return ret;
    }

while during the deployment of the `Implementation` contract we can set `_implementation` address value to `address(0)` that will resist the direct execution of the `delegatecallContract()` while if the call is initiated from the `Proxy` then `_implementation` value it reads from the context of the `Proxy` contract.

Again this is not a viable solution as the whole delegate call based structure has a flaw because `owner` of the contract can itself overwrite the `implementation` address value by calling the `delegatecallContract()` from the `Proxy` contract which would either make Proxy non usable that lead of lock of funds for perpetuity or could change the `implementation` address to some attacker contract address which would would get funds stolen.

I recommend to remove the chaining of the `delegatecall` as it is very risky to use.