xiaoming90 - Claimed tokens obtained during the burning of DV shares are overwritten

[sherlock-admin]

xiaoming90

high

Claimed tokens obtained during the burning of DV shares are overwritten

Summary

The reward tokens claimed while burning DV shares are overwritten, resulting in the loss of reward tokens for the users.

Vulnerability Detail

When withdrawing the base assets via the withdrawBaseAsset function in Line 487 below, it will burn the DV shares internally. When the DV shares are burned, the DV rewards will be automatically redeemed and transferred to this contract. This was also highlighted in the source code's comment on Line 485 below.

Assume the LMPVault only has one DV, and the states are as follows at this point.

• info.idleIncrease = 0
• info.totalAssetsPulled = 0
• info.totalAssetsToPull = 100 WETH
• assetPulled = 0
• assetPreBal = 0

The DV.withdrawBaseAsset function is executed at this point, and the function returns 103 WETH.

The _baseAsset.balanceOf(address(this)) - assetPreBal - assetPulled will compute the reward tokens received and add the amount to the info.idleIncrease.

Assume that _baseAsset.balanceOf(address(this)) - assetPreBal is 110 WETH. The reward tokens received will be 7 WETH (110 - 103) as shown below.

info.idleIncrease += _baseAsset.balanceOf(address(this)) - assetPreBal - assetPulled
info.idleIncrease += (110 WETH - 0 WETH) - 103 WETH
info.idleIncrease += 7 WETH

The info.idleIncrease will be incremented to 7 WETH to account for the reward tokens received.

At Line 488, the info.totalAssetsPulled will be incremented by 103 WETH.

The states at this point will be as follows:

• info.idleIncrease = 7 WETH
• info.totalAssetsPulled = 103 WETH
• info.totalAssetsToPull = 100 WETH
• assetPulled = 103 WETH
• assetPreBal = 0

At Line 493 below, since info.totalAssetsPulled > info.totalAssetsToPull (103 > 100) is true, the info.idleIncrease will be set to 3 WETH.

info.idleIncrease = info.totalAssetsPulled - info.totalAssetsToPull
info.idleIncrease = 103 - 100
info.idleIncrease = 3

As shown above, the 7 WETH reward tokens were initially collected and recorded in the info.idleIncrease variable has been overwritten and cleared. Thus, the reward tokens are lost.

https://github.com/sherlock-audit/2023-06-tokemak/blob/main/v2-core-audit-2023-07-14/src/vault/LMPVault.sol#L482

File: LMPVault.sol
482:                 uint256 assetPreBal = _baseAsset.balanceOf(address(this));
483:                 uint256 assetPulled = destVault.withdrawBaseAsset(sharesToBurn, address(this));
484: 
485:                 // Destination Vault rewards will be transferred to us as part of burning out shares
486:                 // Back into what that amount is and make sure it gets into idle
487:                 info.idleIncrease += _baseAsset.balanceOf(address(this)) - assetPreBal - assetPulled;
488:                 info.totalAssetsPulled += assetPulled;
489:                 info.debtDecrease += totalDebtBurn;
490: 
491:                 // It's possible we'll get back more assets than we anticipate from a swap
492:                 // so if we do, throw it in idle and stop processing. You don't get more than we've calculated
493:                 if (info.totalAssetsPulled > info.totalAssetsToPull) {
494:                     info.idleIncrease = info.totalAssetsPulled - info.totalAssetsToPull;
495:                     info.totalAssetsPulled = info.totalAssetsToPull;
496:                     break;
497:                 }

Impact

Loss of reward tokens.

Code Snippet

https://github.com/sherlock-audit/2023-06-tokemak/blob/main/v2-core-audit-2023-07-14/src/vault/LMPVault.sol#L482

Tool used

Manual Review

Recommendation

The extra tokens collected when burning the DV shares and swaps should be incremented to info.idleIncrease instead of overwriting it.

uint256 assetPreBal = _baseAsset.balanceOf(address(this));
uint256 assetPulled = destVault.withdrawBaseAsset(sharesToBurn, address(this));

// Destination Vault rewards will be transferred to us as part of burning out shares
// Back into what that amount is and make sure it gets into idle
info.idleIncrease += _baseAsset.balanceOf(address(this)) - assetPreBal - assetPulled;
info.totalAssetsPulled += assetPulled;
info.debtDecrease += totalDebtBurn;

// It's possible we'll get back more assets than we anticipate from a swap
// so if we do, throw it in idle and stop processing. You don't get more than we've calculated
if (info.totalAssetsPulled > info.totalAssetsToPull) {
-   info.idleIncrease = info.totalAssetsPulled - info.totalAssetsToPull;
+	info.idleIncrease += info.totalAssetsPulled - info.totalAssetsToPull;
    info.totalAssetsPulled = info.totalAssetsToPull;
    break;
}

Using the same example, if the above changes are implemented, 3 WETH will be added to the info.idleIncrease that already contains 7 WETH. In the end, it will correctly account for all the extra 10 WETH.

Duplicate of #5