Sparkn

Severity

Medium Risk

Relevant GitHub Links

https://github.com/Cyfrin/2023-08-sparkn/blob/main/src/ProxyFactory.sol#L105-L117

Summary

The owner of the ProxyFactory can accidentally provide a wrong implementation address while setting a new contest in the ProxyFactory contract. If a contest have an address of a smart contract implementation that doesn't have any function to distribute or get the funds out of the contract, the money will be stuck in the contest Proxy contract forever.

Vulnerability Details

There is no way to change the implementation address from a created contest in the ProxyFactory and once some sponsor sent money to the Proxy address, if the implementation doesn't have any function to distribute or move the funds, these will be there forever since the contract can only delegate call to the implementation as the Proxy have the address as an immutable.

The vulnerability happens when the owner of the ProxyFactory sets a contest with a wrong implementation address. Once the contest has been created in the setContest(address organizer, bytes32 contestId, uint256 closeTime, address implementation) function there is no way to change it, you would have to create a new contest with the right implementation address.

Impact

If the protocol had previously some other versions of an implementation for the Proxy to delegate call to, it is possible to mismatch the implementation address. If someone notices the error in the implementation address before sponsoring the contest it would be just as easy as create a new contest with the correct implementation address. However, if nobody notices it, if the Proxy contract gets funded the money will be stuck there forever.

Tools Used

Manual Review

Recommendations

Having the current and must updated implementation address as a storage variable would prevent from mismatching the address. This way, the owner would only have to pass the implementationAddress as a parameter to the setContest() function avoiding the possibility of commiting an error.
To change the implementationAddress would also be necessary this function: