Jigsaw - LMPVault exchange rate can potentially be reset by flashloan causing loss of user funds

[sherlock-admin2]

Jigsaw

medium

LMPVault exchange rate can potentially be reset by flashloan causing loss of user funds

Summary

It is possible for the exchange rate of shares/tokens to be reset to 1:1 via flashloan, causing loss of funds for protocol/users. Further detail can be found in this twitter thread: https://twitter.com/kankodu/status/1685320718870032384.

Vulnerability Detail

Consider the following thought experiment:

Lets start with an LMPVault that accepts WETH as the underlying and has share token tokWETH. Lets assume the strategy for the vault involves staking all share tokens in a protocol, which we'll call protocolF, that allows flashloans. Lets further assume that the vault is in a state such that the exchange rate for 1 tokWETH is 2 WETH ie, to mint 1 tokWETH token at S0 (current state) a user must supply 2 WETH.

A malicious user would be able to flashloan the entire supply of the LMPVault from protocolF. They would then redeem all underlying assets in the LMPVault and receive 2 WETH for each tokWETH redeemed. They then remint the same number of tokWETH by supplying an equal number of WETH at a 1:1 ratio plus any additional WETH needed to repay a fee denominated in tokWETH, and repay the flashloan, thereby stealing almost half of the underlying WETH in the vault. This would give us S1, a state where the totalSupply of tokWETH is unchanged, however the exchange rate and underlying assets are cut in half to 1:1.

Submitting this a medium level vulnerability because while user funds are potentially directly at risk, this exploit requires many different factors aligning to be exploited, (singular destination vault etc) and thus is unlikely to happen in (most) cases.

Impact

This vulnerability does require the vault to be in a specific state, however as the twitter thread above provided shows, this state is not outside the realms of possibility. The impact of such a transaction would be that all users in the vault have realized ~50% loss of funds due to our malicious actor.

Code Snippet

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

Tool used

Manual Review

Recommendation

Per this issue thread on OpenZeppelins 4626 implementation: OpenZeppelin/openzeppelin-contracts#3800, I recommend implementing a requirement that once a vault has been initialized and funded, totalSupply must remain greater than some threshold. A common threshold used for this is 1 gwei. This ensures that the current exchange rate will always be kept track of and prevent this attack vector.

[sherlock-admin2]

1 comment(s) were left on this issue during the judging contest.

Trumpero commented:

valid, but require a lot of factors which can reduce to low:

• all partcipants must provide liquidity to that lending
• the integrated lending should a mechanism handle the maximum shares holding by each wallet defined by the tokemak ? which is hard and no lending have this mechanism yet ?

[carrotsmuggler]

Escalate

This issue makes a lot of assumptions.

The LMPVault share token has a perWalletLimit. So it is unlikely to be ever used in lending protocols or in exchanges in bulk, since the contract holding the tokens will hit that limit in any practical scenario.

Keeping the wallet limits in mind, the attack involves the following assumptions:

1. All tokens, 100 % of the float should be in lending protocols or dexs.
2. Total supply of the share tokens must below the perWalletLimit. Otherwise the contract holding the tokens will hit the walletlimit.
3. The platforms holding the tokens allow flash swaps.

While the attack vector is valid, the auditor himself in the tweet explains that this is highly situational. Due to the way tokemak shares are designed, it is extremely unlikely this will ever happen and should thus be classified a low.

According to the sherlock docs, The attack path is possible with assumptions that either mimic on-chain conditions or reflect conditions that have a reasonable chance of becoming true in the future. is the requirement for a Medium. I don't think the situation is likely enough to justify the severity, thus should be a low.

[sherlock-admin2]

Escalate

This issue makes a lot of assumptions.

The LMPVault share token has a perWalletLimit. So it is unlikely to be ever used in lending protocols or in exchanges in bulk, since the contract holding the tokens will hit that limit in any practical scenario.

Keeping the wallet limits in mind, the attack involves the following assumptions:

1. All tokens, 100 % of the float should be in lending protocols or dexs.
2. Total supply of the share tokens must below the perWalletLimit. Otherwise the contract holding the tokens will hit the walletlimit.
3. The platforms holding the tokens allow flash swaps.

While the attack vector is valid, the auditor himself in the tweet explains that this is highly situational. Due to the way tokemak shares are designed, it is extremely unlikely this will ever happen and should thus be classified a low.

According to the sherlock docs, The attack path is possible with assumptions that either mimic on-chain conditions or reflect conditions that have a reasonable chance of becoming true in the future. is the requirement for a Medium. I don't think the situation is likely enough to justify the severity, thus should be a low.

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[xiaoming9090]

Escalate

This issue makes a lot of assumptions.

The LMPVault share token has a perWalletLimit. So it is unlikely to be ever used in lending protocols or in exchanges in bulk, since the contract holding the tokens will hit that limit in any practical scenario.

Keeping the wallet limits in mind, the attack involves the following assumptions:

1. All tokens, 100 % of the float should be in lending protocols or dexs.
2. Total supply of the share tokens must below the perWalletLimit. Otherwise the contract holding the tokens will himit the walletlimit.
3. The platforms holding the tokens allow flash swaps.

While the attack vector is valid, the auditor himself in the tweet explains that this is highly situational. Due to the way tokemak shares are designed, it is extremely unlikely this will ever happen and should thus be classified a low.

According to the sherlock docs, The attack path is possible with assumptions that either mimic on-chain conditions or reflect conditions that have a reasonable chance of becoming true in the future. is the requirement for a Medium. I don't think the situation is likely enough to justify the severity, thus should be a low.

I agree with the escalator's comment. It is unlikely that all the LMPVault tokens would be deposited into an external protocol that provided flash-loan in the real-world. Thus, this should be a low at most.

[Trumpero]

I concur that this problem lacks practicality in real-world situations, hence it should be rated as having a low severity

[hrishibhat]

Agree with escalation comments. This can be considered a low issue.

[Evert0x]

Planning to accept escalation and close issue.

[Evert0x]

Result:
Invalid
Unique

[sherlock-admin2]

Escalations have been resolved successfully!

Escalation status:

• carrotsmuggler: accepted