Sparkn

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

Taking fee from rescued user when rescuing the tokens stuck in the proxy after the proxy's expiration

klaus

Summary

After the contest is over and all prizes have been distributed, the fee is also charged when rescuing token by distributeOwner to take out the token that was accidentally sent.

Vulnerability Details

Tokens stuck in the Distributor contract(proxy) should be taken out through the distributeByOwner function. Since distribute is the only function that can take out tokens from the Distributor contract, it should use this function to take out tokens.

function distributeByOwner(
address proxy,
address organizer,
bytes32 contestId,
address implementation,
bytes calldata data
) public onlyOwner {
if (proxy == address(0)) revert ProxyFactory__ProxyAddressCannotBeZero();
bytes32 salt = _calculateSalt(organizer, contestId, implementation);
if (saltToCloseTime[salt] == 0) revert ProxyFactory__ContestIsNotRegistered();
// distribute only when it exists and expired
if (saltToCloseTime[salt] + EXPIRATION_TIME > block.timestamp) revert ProxyFactory__ContestIsNotExpired();
_distribute(proxy, data);
}
​

https://github.com/Cyfrin/2023-08-sparkn/blob/47c22b818818af4ea7388118dd83fa308ad67b83/src/ProxyFactory.sol#L205-L218

Distributor.distribute is a function for distributing prizes to winners and internally calls _distribute. _distribute transfer 5% protocol fee to STADIUM_ADDRESS

function _distribute(address token, address[] memory winners, uint256[] memory percentages, bytes memory data)
internal
{
...
for (uint256 i; i < percentagesLength;) {
totalPercentage += percentages[i];
unchecked {
++i;
}
}
// check if totalPercentage is correct
if (totalPercentage != (10000 - COMMISSION_FEE)) {
revert Distributor__MismatchedPercentages();
}
...
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);
}

https://github.com/Cyfrin/2023-08-sparkn/blob/47c22b818818af4ea7388118dd83fa308ad67b83/src/Distributor.sol#L154-L155

This fee is for using the protocol. Therefore, it seems unreasonable to charge a fee of 5% to users who did not use the protocol and only sent tokens by mistake. There is no specified anywhere in the documentation or explanation to take fee for accidental token sent.

Also, the user will be logged by emit Distributed(token, winners, percentages, data);. It is not right that the user will be marked as a winner in the log who is not winner.

If you use The Graph service or collect events to display information on web2, incorrect information will be displayed due to incorrect events.

So, it is better to create and use a function to rescue tokens at Distributor contract.

Impact

A protocol fee is also charged from those who mistakenly insert tokens. Unnecessary logs are maked, causing confusion.

Tools Used

vscode

Recommendations

Create and use a function to rescue tokens at Distributor contract.

Support

FAQs

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