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

Loss of rewards for a part of users since `PAXG` token is fee-on-transfer and unaccounted for in the system

amarfares

Vulnerability Details

When LiquidationPool.sol#distributeAssets() is invoked, the protocol calculates rewards for the supported asset tokens and maps a rewards _portion to each holder depending on their positions size.

https://github.com/Cyfrin/2023-12-the-standard/blob/91132936cb09ef9bf82f38ab1106346e2ad60f91/contracts/LiquidationPool.sol#L205-L241

function distributeAssets(...) external payable {
...
for (uint256 j = 0; j < holders.length; j++) {
Position memory _position = positions[holders[j]];
uint256 _positionStake = stake(_position);
if (_positionStake > 0) {
for (uint256 i = 0; i < _assets.length; i++) {
ILiquidationPoolManager.Asset memory asset = _assets[i];
if (asset.amount > 0) {
(,int256 assetPriceUsd,,,) = Chainlink.AggregatorV3Interface(asset.token.clAddr).latestRoundData();
uint256 _portion = asset.amount * _positionStake / stakeTotal;
uint256 costInEuros = _portion * 10 ** (18 - asset.token.dec) * uint256(assetPriceUsd) / uint256(priceEurUsd)
* _hundredPC / _collateralRate;
if (costInEuros > _position.EUROs) {
_portion = _portion * _position.EUROs / costInEuros;
costInEuros = _position.EUROs;
}
_position.EUROs -= costInEuros;
rewards[abi.encodePacked(_position.holder, asset.token.symbol)] += _portion;
...
}

The rewards amount _portion is first assigned to the _position.holder in the rewards mapping, and then transferred from the manager address to the LiquidationPool. The issue here is thatPAXG is a fee-on-transfer token so later when a user tries to call LiquidationPool.sol#claimRewards() the function loops through the rewards tokens and transfers the claimable rewards to the user, but whilst the _portion amount is assigned as reward, the actual amount the contract will have received will be < _portion due to the fee-on-transfer mechanism.

https://github.com/Cyfrin/2023-12-the-standard/blob/91132936cb09ef9bf82f38ab1106346e2ad60f91/contracts/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);
}
}
​
}
}

In other words, when the for-loop gets to the PAXG token, the latter part of users that try to claim rewards will not be able to as the balance of the contract will not be enough and the for-loop will revert the whole claim function (not just for PAXG). The earlier users will be able to claim rewards since the pool will have enough to accommodate by taking away from the latter users' rewards.

As time goes on some users with stuck funds might be able to withdraw when new rewards are accrued and transferred in for other users, but there will always be people who are unable to claim their rewards since PAXG.balanceOf(address(this)) < userRewards will always be true.

Impact

Loss of rewards for a part of users when trying to claim

Tools Used

Manual Review

Recommendations

Account for fee-on-transfer tokens:

uint256 balanceBefore = IERC20(asset.token.addr).balanceOf(address(this));
IERC20(asset.token.addr).safeTransferFrom(manager, address(this), _portion)
uint256 balanceAfter = IERC20(asset.token.addr).balanceOf(address(this)) - balanceBefore;
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!