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

Fee on transfer and rebasing tokens can lead to race conditions

cosine

Summary

The protocol stated out in discord that they want to know about any potential issue with ERC-20 tokens that could be added to the system in the future and already uses PAXG which is a fee on transfer token.

Fee on transfer and rebasing tokens can lead to race conditions when claiming rewards, where the first users calling claimRewards get their full reward, while the last get nothing.

Vulnerability Details

Here we can see the claimRewards function of the LiquidationPool contract and how rewards are stored inside the distributeAssets function:

​
function distributeAssets(ILiquidationPoolManager.Asset[] memory _assets, uint256 _collateralRate, uint256 _hundredPC) external payable {
...
// portion = amount of tokens
rewards[abi.encodePacked(_position.holder, asset.token.symbol)] += _portion;
...
}
​
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);
}
}
​
}
}

As we can see, the distributeAssets function will store the amount of tokens to every user's rewards balance and the claimRewards function will transfer the tokens to the user.

If the given tokens are fee on transfer tokens every time a user calls claimRewards the balance of the contract will decrease a little bit more than the amount saved in the rewards mapping. This creates a race condition as the last users who want to claim their rewards will not get any tokens as the transfer call will revert, because there are not enough tokens left in the contract.

The same applies to rebasing tokens when the balance of the contract is reduced and otherwise leads to frozen funds in the contract.

Impact

A race condition is created which distributes rewards unfairly.

Recommendations

Implement a share based approach to the rewards instead, or never allow rebasing tokens and fee on transfer tokens.

Updates

Lead Judging Commences

hrishibhat Lead Judge almost 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

fee-on-transfer

hrishibhat Lead Judge almost 2 years ago
Submission Judgement Published
Invalidated
Reason: Out of scope
Assigned finding tags:

fee-on-transfer

Support

FAQs

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

Give us feedback!