Sparkn

Summary

The Proxy.sol contract uses the delegatecall proxy
pattern. If the implementation contract (Distributor.sol) is incorrectly set-up or self-destructed, the contract
may not be able to detect failed executions.

Vulnerability Details

ProxyFactory.sol deploys the Proxy.sol for every contest. Proxy.sol delegate calls all the calls from the ProxyFactory.sol to the implementation contract (Distributor.sol). Proxy.sol uses simple delegatecall proxy pattern. However, it does not check whether the implementation contract (Distributor.sol) exists or not. The calls will return success even when the Distributor.sol has been incorrectly set-up or mistakenly selfdestruct. This will lead to unwanted consequences in the system.
From the solidity documentation:
The low-level functions call, delegatecall and staticcall return true as their first
return value if the account called is non-existent, as part of the design of the
EVM. Account existence must be checked prior to calling if needed.

Impact

Unwanted consequences in the system. May lead to confusion when it returns true to the calls from the ProxyFactory.sol but buyers and sellers do not get the desired result.

Tools Used

Manual review.

Recommendations

Have a contract existence check before the delegatecall.

fallback() external {
address implementation = _implementation;
require(implementation!= address(0));
assembly {
let ptr := mload(0x40)
calldatacopy(ptr, 0, calldatasize())
let result := delegatecall(gas(), implementation, ptr, calldatasize(), 0, 0)
let size := returndatasize()
returndatacopy(ptr, 0, size)

        switch result
        case 0 { revert(ptr, size) }
        default { return(ptr, size) }
    }
}

}