Low level delegate call doesnt check if the contract code exist

Severity

High Risk

Relevant GitHub Links

https://github.com/Cyfrin/2023-08-sparkn/blob/0f139b2dc53905700dd29a01451b330f829653e9/src/Proxy.sol#L51C1-L64C6

Summary

Low level delegte call doesnt check if the contract code exist on address

Vulnerability Details

Delegate low level call has no ability to check if there's deployed contract code exist which delegating to ,So If proxy sending delegate call to empty address , call will success as everything went according to plan and this might lead to loss of funds cause in proxy factory there's no check if the implemantation contract is deployed.

Impact

The entire contest funds might stuck in proxy address cause of no implemantation existence check. Here's the POC for the scenario

 function testCanCallEmptyImplementationWithDlgtCall() public  {
        //can call empty implementation address and still delegate call succeeds!!
        address randomDistrbtr = makeAddr("ran");
        vm.startPrank(factoryAdmin);
        bytes32 randomId = keccak256(abi.encode("Jason", "001"));
        proxyFactory.setContest(organizer, randomId, block.timestamp + 8 days,  address(randomDistrbtr));
        vm.stopPrank();
        bytes32 salt = keccak256(abi.encode(organizer, randomId, address(randomDistrbtr)));
        address proxyAddress = proxyFactory.getProxyAddress(salt, address(randomDistrbtr));
        vm.startPrank(sponsor);
        MockERC20(jpycv2Address).transfer(proxyAddress, 10000 ether);
        vm.stopPrank();
        
        
        
        // before
        assertEq(MockERC20(jpycv2Address).balanceOf(user1), 0 ether);
        assertEq(MockERC20(jpycv2Address).balanceOf(stadiumAddress), 0 ether);
        bytes memory data = createData();

        vm.warp(9 days); // 9 days later
        vm.startPrank(organizer);
        proxyFactory.deployProxyAndDistribute(randomId, address(randomDistrbtr), data);
        vm.stopPrank();

        // after
/*      assertEq(MockERC20(jpycv2Address).balanceOf(user1), 9500 ether);
        assertEq(MockERC20(jpycv2Address).balanceOf(stadiumAddress), 500 ether); */
    }

This test will pass means delegatecall returns true from empty address...

Tools Used

Foundry test suite-Manuel Review

Recommendations

During an upgrade, check that the new logic contract has code. One solution is to use the extcodesize opcode. Alternatively, you can check for the existence of the target each time delegatecall is used. Here's usage for extcodesize

      uint codesz;
     assembly { 
        codesz := extcodesize(computedAddress) 
        }
    if(codesz ==0) revert ProxyFactory__ImplementationAddressEmpty();

Cyfrin / 2023-08-sparkn