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

Unclaimed Liquidation Rewards Remain Unavailable After Token Removal

ua1552

Summary

The LiquidationPool::claimRewards(...) function prevents users from claiming rewards for removed tokens if there was an active unclaimed reward during the token removal.

Vulnerability Details

The vulnerability is centered around the LiquidationPool::claimRewards(...) function, which relies on the TokenManager::getAcceptedTokens(...) contract to obtain accepted tokens and claim rewards for them. If a token is removed before the staker claims the reward, the staker will be unable to claim rewards for that token because TokenManager::getAcceptedTokens(...) will not return the data for the removed token.

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);
}
}
}
}

Github: [64-80]

Impact

Stakers will lose unclaimed rewards for removed tokens.

Proof of Concept

The test verifies that rewards cannot be claimed for removed tokens in the LiquidationPool. The test includes the removal of the ARB token and subsequent attempts by a staker to claim rewards for the removed token.

function test_rewardsCannotBeClaimedFroRemovedTokensInLiquidationPool() public {
///////////////////////////////
/// Setup ///
///////////////////////////////
​
uint amount = 1000 ether;
​
///////////////////////////////////////////////////
/// minting some tokens to alice ///
///////////////////////////////////////////////////
​
tokens.eurosToken.mint(alice, amount);
tokens.tstToken.mint(alice, amount);
​
///////////////////////////////////////////////
/// alice increases her stake ///
///////////////////////////////////////////////
​
vm.startPrank(alice);
tokens.eurosToken.approve(address(contracts.liquidationPool), amount);
tokens.tstToken.approve(address(contracts.liquidationPool), amount);
contracts.liquidationPool.increasePosition(amount, amount);
vm.stopPrank();
​
//////////////////////////////////////////////////////
// alice's position should have been increased ///
//////////////////////////////////////////////////////
​
(LiquidationPool.Position memory alicePosition, LiquidationPool.Reward[] memory rewards) =
contracts.liquidationPool.position(alice);
assertEq(alicePosition.EUROs, amount, "Alice's euros amount are not eqaul");
assertEq(alicePosition.TST, amount, "Alice's tst amount are not equal");
​
//////////////////////////////////////////////////////////
/// skipping some time to consolidate stakes ///
//////////////////////////////////////////////////////////
​
skip(block.timestamp + 1 weeks);
​
////////////////////////////////////////////////////////////////////////
// minting some ARBs to the LiquidationPoolManger ///
/// to represent the Liquidated assets ///
////////////////////////////////////////////////////////////////////////
​
tokens.arbToken.mint(address(contracts.liquidationPoolManager), amount);
tokens.paxgToken.mint(address(contracts.liquidationPoolManager), amount);
​
/////////////////////////////////////////////////////////////////////////////
/// Mocking LiquidationPoolManager approving the LiquidationPool ///
/// to spend the assets ///
/////////////////////////////////////////////////////////////////////////////
​
vm.startPrank(address(contracts.liquidationPoolManager));
tokens.arbToken.approve(address(contracts.liquidationPool), amount);
tokens.paxgToken.approve(address(contracts.liquidationPool), amount);
vm.stopPrank();
​
////////////////////////////////////////////////////////
// setting up data for mock call ///
////////////////////////////////////////////////////////
​
ILiquidationPoolManager.Asset[1] memory _assets;
​
// setting arb token data
_assets[0] = ILiquidationPoolManager.Asset(
ITokenManager.Token({
symbol: 'ARB',
addr: address(tokens.arbToken),
dec: 18,
clAddr: address(priceFeeds.arbUsdPriceFeed),
clDec: 8
}),
amount
);
​
assets.push(_assets[0]);
​
/////////////////////////////////////////////////////////////////////////////
// mocking distribute asset call from the liquidationPoolManager ///
/////////////////////////////////////////////////////////////////////////////
​
console2.log("> Alice's ARB tokens Balance before Distributing Rewards: %s\n", tokens.arbToken.balanceOf(alice));
​
vm.startPrank(address(contracts.liquidationPoolManager));
contracts.liquidationPool.distributeAssets(assets, uint(constants.DEFAULT_COLLATERAL_RATE), 100_000);
vm.stopPrank();
​
////////////////////////////////////////////////////////////////////
// getting the position of alice after the liquidation ///
// There should be rewards in ARBs token. Also the EURO ///
// balance of the Alice should be 0 as the amount should ///
// have been spent to buy the liquidated assets. ///
////////////////////////////////////////////////////////////////////
​
(alicePosition, rewards) = contracts.liquidationPool.position(alice);
​
bool hasRewards;
​
console2.log("> Alice's Rewards After liquidation: ");
​
for (uint i; i < rewards.length; i++) {
if (rewards[i].amount > 0) {
console2.log('\tToken: %s', string(abi.encode(rewards[i].symbol)));
console2.log('\tReward Earned: %s\n', rewards[i].amount);
hasRewards = true;
}
}
​
require(hasRewards, 'No rewards are earned by the staker');
​
assertEq(alicePosition.EUROs, 0, 'Alice still has EUROs left');
​
/////////////////////////////////////////////////////////////////////////////////////////
// Owner decides to remove arb token from the TokenManager ///
// and alice didn't claim her tokens rewards ///
/////////////////////////////////////////////////////////////////////////////////////////
​
contracts.tokenManager.removeAcceptedToken(bytes32('ARB'));
​
///////////////////////////////////////////////////////////////////////////////////////
/// Getting alice's position to check if she still has any rewards. ///
/// Should return no rewards as the previous rewards token has been removed ///
///////////////////////////////////////////////////////////////////////////////////////
​
(alicePosition, rewards) = contracts.liquidationPool.position(alice);
​
console2.log("> Alice's Rewards After ARB Token Removal:");
​
hasRewards = false;
for (uint i; i < rewards.length; i++) {
if (rewards[i].amount > 0) {
console2.log("\tToken: %s", string(abi.encode(rewards[i].symbol)));
console2.log('\tReward Earned: %s', rewards[i].amount);
hasRewards = true;
}
}
​
require(!hasRewards, 'No rewards are earned by the staker');
​
if (!hasRewards) {
console2.log('\tThere are No rewards for the staker\n');
}
​
///////////////////////////////////////////////////////////////////////////
/// Alice decides to claim her rewards but will not get anything ///
/// Her Arbs token balance will still be zero ///
///////////////////////////////////////////////////////////////////////////
​
vm.startPrank(alice);
contracts.liquidationPool.claimRewards();
vm.stopPrank();
​
console2.log("> Alice's ARB tokens Balance After Distributing Rewards And Claiming: %s", tokens.arbToken.balanceOf(alice));
​
assertEq(tokens.arbToken.balanceOf(alice), 0, "There are ARBs token in Alice's account");
}

GitHub: [548]

Output:

$ forge test --mt test_rewardsCannotBeClaimedFroRemovedTokensInLiquidationPool -vv
[⠢] Compiling...
No files changed, compilation skipped
​
Running 1 test for test/unit/Unit.t.sol:Unit
[PASS] test_rewardsCannotBeClaimedFroRemovedTokensInLiquidationPool() (gas: 813307)
Logs:
> Alice's ARB tokens Balance before Distributing Rewards: 0
​
> Alice's Rewards After liquidation:
Token: ARB
Reward Earned: 779505882352941176471
​
> Alice's Rewards After ARB Token Removal:
There are No rewards for the staker
​
> Alice's ARB tokens Balance After Distributing Rewards And Claiming: 0
​
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 4.16ms
​
Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)

Tools Used

  • Manual Review

  • Foundry

Recommendations

It is recommended to consider one of the following solutions:

  1. Create a separate function allowing users to claim unclaimed rewards for any token.

+ function claimAsset(address token, bytes32 symbol) external {
+ uint256 _rewardAmount = rewards[abi.encodePacked(msg.sender, _token.symbol)];
+ if (_rewardAmount > 0) {
+ delete rewards[abi.encodePacked(msg.sender, symbol)];
+ IERC20(_token.addr).transfer(msg.sender, _rewardAmount);
+ }
+ }
​
+ function claimNative() external {
+ uint256 _rewardAmount = rewards[abi.encodePacked(msg.sender, NATIVE)];
+ if (_rewardAmount > 0) {
+ delete rewards[abi.encodePacked(msg.sender, symbol)];
+ (bool _sent,) = payable(msg.sender).call{value: _rewardAmount}("");
+ require(_sent);
+ }
+ }

  1. Modify the LiquidationPool::claimRewards() function to accept the token address or symbol, enabling stakers to claim rewards for specific tokens.

Updates

Lead Judging Commences

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

remove-token

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

removetoken-low

Support

FAQs

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

Give us feedback!