Sparkn

Summary

The protocol uses single step ownership transfers which, in the case of an erroneous input, can potentially leave the contract without an owner which severely affects several key areas.

Vulnerability Details

The ProxyFactory contract inherits OpenZeppelin's Ownable and implements several key protocol features using the onlyOwner modifier. Ownable uses single step ownership transfers, meaning that if a mistake is made when supplying the newOwner argument, there is no way to retrieve ownership.

It is best practice to implement two-step ownership transfers: the current owner submits a transaction to set the pending owner to a new address, and that address must submit a second transaction to accept ownership. This means that in the case of an erroneous input in the first transaction, all is not lost.

Impact

ProxyFactory may be left without an owner, preventing any new contests from being created and removing the ability to retrieve funds from contest contracts using deployProxyAndDistributeByOwner or distributeByOwner in the event of expiry.

Tools Used

Manual review

Recommendations

Use OpenZeppelin's Ownable2Step instead of Ownable.