Sparkn

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

ERC20 balance not checked before "_distribute" called

falconhoof

Summary

There is no preliminary balanceOf check in any of the ProxyFactory.sol functions which call the _distribute function from the implementation contract via the proxy.

Given that the proxy address is known, it can be used as a parameter to call the ERC20 contract's balanceOf function in order to check that the contest's proxy contract has indeed been sent token's of the expected type (e.g. USDC, JPYCv1, JPYCv2, USDT, DAI, etc) before the proxy contract is deployed.

As is currently designed, it could be the case that the proxy contract is deployed and then a lot of avoidable computation is carried out in _distribute before the balanceOf check is reached at the bottom of the function in (line 142 in Distribution.sol).

Vulnerability Details

Consider a scenario where a contest's supporter claims to have funded the as yet undeployed proxy contract for the contest with DAI token but in reality, by accident or malice, they have not.

Thus when any of the four functions listed below call the _distribute via the proxy contract, the ERC20 balance check will be hit in the function at line 142 if (totalAmount == 0) revert Distributor__NoTokenToDistribute()

While this check makes the _distribute function secure, it is carried out quite far down in the equation. This means that in three of the four functions (1, 2 & 3 below), we will have deployed the contest's proxy contract but distribution of funds will revert so the protocol will not work as intended.

Four affected functions:
1 deployProxyAndDistribute
2 deployProxyAndDistributeBySignature
3 deployProxyAndDistributeByOwner
4 distributeByOwner

Impact

This will have a number of consequences if the ERC20 balance in the proxy contract is 0. In 3 functions, the contest's proxy will be deployed but _distribute will fail. This means:

  1. A lot of gas and computation is wasted reaching the check in _distribute. Inefficient executions may deter users from interacting with the protocol

  2. The proxy contract is incorrectly deployed as it is intended to be deployed only when the proxy is funded. This breaks the basic functionality of the protocol.

  3. Manual intervention would be required to rectify; i.e. triggering of distributeByOwner in proxyFactory.sol to pay the winners which could only be done after the EXPIRATION_TIME (7 days) which would lead a bad user experience and unnecessary admin overhead for the protocol runners

Tools Used

Manual Review
Foundry

Recommendations

Add a check in the form of a modifier to each of the four functions listed above.

modifier hasFunds(address tokenAddress, bytes32 salt, address implementation) {
IERC20 token = IERC20(tokenAddress);
uint256 balance = token.balanceOf(getProxyAddress(salt, implementation));
require(balance > 0, "Insufficient funds in proxy");
_;
}

To be used as:

function deployProxyAndDistribute(address tokenAddress, bytes32 salt, address implementation)
external
hasFunds(tokenAddress, salt, implementation)
{
// no change to the rest of existing logic
}

Support

FAQs

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