Skip to content
This repository has been archived by the owner on Mar 3, 2024. It is now read-only.

p0wd3r - Destination Vault rewards are not added to idleIncrease when info.totalAssetsPulled > info.totalAssetsToPull #5

Open
sherlock-admin2 opened this issue Aug 29, 2023 · 0 comments
Labels
Has Duplicates A valid issue with 1+ other issues describing the same vulnerability High A valid High severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid

Comments

@sherlock-admin2
Copy link
Contributor

sherlock-admin2 commented Aug 29, 2023

p0wd3r

high

Destination Vault rewards are not added to idleIncrease when info.totalAssetsPulled > info.totalAssetsToPull

Summary

Destination Vault rewards are not added to idleIncrease when info.totalAssetsPulled > info.totalAssetsToPull in _withdraw of LMPVault.

This result in rewards not being recorded by LMPVault and ultimately frozen in the contract.

Vulnerability Detail

In the _withdraw function, Destination Vault rewards will be first recorded in info.IdleIncrease by info.idleIncrease += _baseAsset.balanceOf(address(this)) - assetPreBal - assetPulled;.

But when info.totalAssetsPulled > info.totalAssetsToPull, info.idleIncrease is directly assigned as info.totalAssetsPulled - info.totalAssetsToPull, and info.totalAssetsPulled is assetPulled without considering Destination Vault rewards.

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

                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.totalAssetsPulled = info.totalAssetsToPull;
                    break;
                }

For example,

                    // preBal == 100 pulled == 10 reward == 5 toPull == 6
                    // idleIncrease = 115 - 100 - 10 == 5
                    // totalPulled(0) += assetPulled == 10 > toPull
                    // idleIncrease = totalPulled - toPull == 4 < reward

The final info.idleIncrease does not record the reward, and these assets are not ultimately recorded by the Vault.

Impact

The final info.idleIncrease does not record the reward, and these assets are not ultimately recorded by the Vault.

Meanwhile, due to the recover function's inability to extract the baseAsset, this will result in no operations being able to handle these Destination Vault rewards, ultimately causing these assets to be frozen within the contract.

Code Snippet

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

Tool used

Manual Review

Recommendation

info.idleIncrease = info.totalAssetsPulled - info.totalAssetsToPull; -> info.idleIncrease += info.totalAssetsPulled - info.totalAssetsToPull;

@github-actions github-actions bot added High A valid High severity issue Has Duplicates A valid issue with 1+ other issues describing the same vulnerability labels Sep 11, 2023
This was referenced Sep 11, 2023
@codenutt codenutt added the Sponsor Confirmed The sponsor acknowledged this issue is valid label Sep 20, 2023
@sherlock-admin sherlock-admin changed the title Macho Shamrock Huskie - Destination Vault rewards are not added to idleIncrease when info.totalAssetsPulled > info.totalAssetsToPull p0wd3r - Destination Vault rewards are not added to idleIncrease when info.totalAssetsPulled > info.totalAssetsToPull Oct 3, 2023
@sherlock-admin sherlock-admin added the Reward A payout will be made for this issue label Oct 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Has Duplicates A valid issue with 1+ other issues describing the same vulnerability High A valid High severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid
Projects
None yet
Development

No branches or pull requests

3 participants