The Standard
The Standard
DeFiHardhat
20,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Users might end up being DOS'd while claiming rewards

bauchibred

Proof of Concept

Take a look at LiquidationPool.sol#L164-L180

function claimRewards() external {
ITokenManager.Token[] memory _tokens = ITokenManager(tokenManager).getAcceptedTokens();
for (uint256 i = 0; i < _tokens.length; i++) {
ITokenManager.Token memory _token = _tokens[i];
uint256 _rewardAmount = rewards[abi.encodePacked(msg.sender, _token.symbol)];
if (_rewardAmount > 0) {
delete rewards[abi.encodePacked(msg.sender, _token.symbol)];
if (_token.addr == address(0)) {
(bool _sent,) = payable(msg.sender).call{value: _rewardAmount}("");
require(_sent);
} else {
IERC20(_token.addr).transfer(msg.sender, _rewardAmount);
}
}
​
}
}

And see distributeAssets() too, the main idea of how this bug case works is that the transfer of tokens are being done in a push and not pull method, i.e the receivers have been hardcoded to be the msg.sender in these instances, now in the case where this receiver's address has been blacklisted, for example by Circle in regards to USDC then the user would not be able to claim rewards, and even worse no other user would be able to receive tokens from distributeAssets() if user is among them.

Protocol owner on discord said they'd like to know about issues that would occur with other EERC20s... assuming popular tokens, we can consider the most popular stablecoins out there to be a bet, i.e USDC & USDT which employ this blacklisting mechanism.

Impact

Borderline medium/low, medium cause:

  • This leads to a DOS from users claiming their rewards , i.e stuck funds note that this also affects other tokens in the pool even the ones that do not employ the blacklisting mechanism cause the withdrawal is looped for all teh different tokens in the "i < _tokens.length;" in claimReward(),

  • This also causes protocol to be unable to distribute fees to other holders if one user is blacklisted

  • Low cause this is a futuristic flaw in code, depending on which assets get added.

Recommended Mitigation Steps

Implement a pull and not push method, i.e in the case of claimRewards() allow msg.sender to provide a fresh address and in the case of distributeAssets() use a try-catch method to transfer these tokens and any user that couldn't receive their tokens should be stored on-chain and allowed to come pull their tokens later on.

Updates

Lead Judging Commences

hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
Assigned finding tags:

informational/invalid

Support

FAQs

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