Sparkn

CodeFox Inc.
DeFiFoundryProxy
15,000 USDC
View results
Previous Next
Submission Details
Severity: low
Valid

If the STADIUM_ADDRESS got blacklisted by token contract, the fee and all the proxy functionality will be permanently frozen which lead to lock all the funds inside the proxy forever.

castleChain

Summary

if the STADIUM_ADDRESS got blacklisted by the token it will be impossible to get the fee tokens from the proxy contract and all the functions of the proxy will be frozen and revert all time which will lead to a loss of funds for the protocol and users .

Vulnerability Details

Some tokens (e.g. USDC, USDT) have a contract level admin controlled address blocklist. If an address is blocked, then transfers to and from that address are forbidden , so if the STADIUM_ADDRESS get blocklisted for any reason this will lead to freeze all the functions the responsible for distribution of the reward which lead to lock all the funds and fee tokens inside the proxy contract , which is a huge loss of funds .

and if the protocol tried to deploy a new implementation contract ,all the locked funds will keep locked because the implementation can only be set once inside the proxy contract .

Poc

in the Distributor contract, the function distribute() call the internal function _commissionTransfer() which send the fee tokens to the STADIUM_ADDRESS , as shown here
https://github.com/Cyfrin/2023-08-sparkn/blob/0f139b2dc53905700dd29a01451b330f829653e9/src/Distributor.sol#L116-L156

function _distribute(address token, address[] memory winners, uint256[] memory percentages, bytes memory data)
internal
{
// token address input check
if (token == address(0)) revert Distributor__NoZeroAddress();
if (!_isWhiteListed(token)) {
revert Distributor__InvalidTokenAddress();
}
// winners and percentages input check
if (winners.length == 0 || winners.length != percentages.length) revert Distributor__MismatchedArrays();
uint256 percentagesLength = percentages.length;
uint256 totalPercentage;
for (uint256 i; i < percentagesLength;) {
totalPercentage += percentages[i];
unchecked {
++i;
}
}
// check if totalPercentage is correct
if (totalPercentage != (10000 - COMMISSION_FEE)) {
revert Distributor__MismatchedPercentages();
}
IERC20 erc20 = IERC20(token);
uint256 totalAmount = erc20.balanceOf(address(this));
​
​
// if there is no token to distribute, then revert
if (totalAmount == 0) revert Distributor__NoTokenToDistribute();
​
​
uint256 winnersLength = winners.length; // cache length
for (uint256 i; i < winnersLength;) {
uint256 amount = totalAmount * percentages[i] / BASIS_POINTS;
erc20.safeTransfer(winners[i], amount);
unchecked {
++i;
}
}
​
​
// send commission fee as well as all the remaining tokens to STADIUM_ADDRESS to avoid dust remaining
--> _commissionTransfer(erc20);
emit Distributed(token, winners, percentages, data);
}

sending the fee tokens to the STADIUM_ADDRESS here :
https://github.com/Cyfrin/2023-08-sparkn/blob/0f139b2dc53905700dd29a01451b330f829653e9/src/Distributor.sol#L163-L165

function _commissionTransfer(IERC20 token) internal {
--> token.safeTransfer(STADIUM_ADDRESS, token.balanceOf(address(this)));
}

if the function _commissionTransfer() reverts the distribute() will also revert which prevent the winners from getting their rewards and also prevent the owner from rescuing the tokens , which cause the all funds to be locked forever .

Impact

this vulnerability will prevent the winners from getting their rewards , and the protocol from taking the fee ,and also prevent the owner from rescuing the tokens , which cause the all funds to be locked forever .

Tools Used

manual review

Recommendations

there are two possible mitigation methods (the second is the favorable) :
1)adding a function to set (change) the implementation inside the proxy and allow only the factory to call this function ,
and then add this function to the factory contract and allow only the owner to set the new implementation , and prevent all the other function in the factory from calling this setImplemntation() function , by reverting in case of the selector is the selector of this function
add this function in the proxy

function setImplementation(address newImpl) external {
if (msg.sender != FACTORY_ADDRESS) {
revert Distributor__OnlyFactoryAddressIsAllowed();
}
implementation = newImpl ;
}

and this function in the factory

function setImplementation(address newImpl) external onlyOwner {
(bool success,) = proxy.call(abi.encodeWithSignture("setImplementation(address)" , newImpl));
}

2)add the setImplementation function in the proxy and allow only the owner of the factory contract to call it

function setImplementation(address newImpl) external {
if (msg.sender != factoryOwner) {
revert Distributor__OnlyFactoryOwnerIsAllowed();
}
implementation = newImpl ;
}

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!