Summary

If the protocol's fee collector (STADIUM_ADDRESS) gets blacklisted, the token locked in the Proxy will be permanently frozen. Moreover, this vulnerability will affect all contests' Proxy contracts since they point to the same Implementation contract.

Vulnerability Details

The Distributor::_distribute() implements a core logic for distributing prizes (tokens) to contest winners and is also used for recovering stuck tokens. Upon distributing prizes, the function will transfer prizes (minus a commission fee) locked in the contest's Proxy to contest winners. After that, the function will transfer the commission fee to the protocol's fee collector (i.e., STADIUM_ADDRESS).

Upon recovering stuck tokens, the function will perform the same process. The recovered token (minus a commission fee) will be transferred to a rescue requestor, and then the function will send a commission fee to the fee collector.

The Sparkn protocol is supposed to whitelist major stablecoin tokens (e.g., JPYC, USDC, USDT, DAI, etc.) as permitted prize tokens. Almost all major stablecoin tokens implement the blacklist().

Suppose the fee collector (STADIUM_ADDRESS) gets blacklisted. In that case, the token transferred to the Proxy will be locked up permanently. The root cause of this vulnerability is that the STADIUM_ADDRESS is an immutable variable. Furthermore, the Proxy is also pointing to the immutable Implementation contract.

Even the stuck token's recovery process could not deal with this vulnerability since the recovery process also shares the same token transfer logic in the _distribute(), as described previously.

• The invocation of the _commissionTransfer() by the _distribute(): https://github.com/Cyfrin/2023-08-sparkn/blob/0f139b2dc53905700dd29a01451b330f829653e9/src/Distributor.sol#L154

• If the fee collector (STADIUM_ADDRESS) gets blacklisted, TX will always be reverted: https://github.com/Cyfrin/2023-08-sparkn/blob/0f139b2dc53905700dd29a01451b330f829653e9/src/Distributor.sol#L164

Impact

If the fee collector (STADIUM_ADDRESS) gets blacklisted, the token locked in the Proxy will be permanently frozen. Moreover, this vulnerability will affect all contests' Proxy contracts since they point to the same Implementation contract.

The likelihood is considered LOW. The impact is considered HIGH. Therefore, the severity is considered MEDIUM.

Someone might question: "Why would the fee collector get blacklisted?". The answer is not easy to determine. For example, the fee collector gets persecuted/discredited somehow, such as by sending tokens from Tornado Cash to it, resulting in the fee collector eventually getting blacklisted, unfortunately. Hence, relying on the assumption that this vulnerability will not happen is not the best notion exactly.

Tools Used

Manual Review

Recommendations

I recommend improving the _commissionTransfer() to get the fee collector's address from the ProxyFactory contract. The fee collector's address should be an updatable variable through a setter function by an owner (protocol admin).

If a current address gets blacklisted, the owner can change the fee collector to another address, preventing the users' funds from being permanently frozen.