arpitsingh1409

Recommendation #2 for Spearbit Assignment

Recommendation: Since the msg.sender value do not change when a function is executed with delegatecall, we can create a state variable (named owner) in the Implementation contract at storage slot 0 and initialize it to the zero address. After this, we can add access control to the delegatecallContract function: require(msg.sender == owner); Now if someone calls the delegatecallContract function directly in the Implementation contract, the check would fail as no one has access to the zero account, however when the function is called from the proxy contract as a delegatecall, msg.sender would remain the same, but the owner state variable would be read from the stroage of the Proxy contract since it is a delegate call and hence the require statement will pass as the slot 0 storage of proxy contract has the address of the proxy contract owner stored.

// SPDX-License-Identifier: MIT
pragma solidity 0.8.10;
contract Implementation {

arpitsingh1409

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: