Concentrating vault's liquidity might enable flashloan-backed vault reset attack leading to partial vault drain

[c4-bot-8]

Lines of code

https://github.com/code-423n4/2024-02-spectra/blob/main/src/tokens/PrincipalToken.sol#L609
https://github.com/code-423n4/2024-02-spectra/blob/main/src/tokens/PrincipalToken.sol#L588

Vulnerability details

Impact

The PrincipalToken allows for easier concentration of a underlying vault's shares in form of the ibt. In the same time, it is possible to take a flashloan for all of the ibt tokens. If enough vault's shares are concentrated in the PT, then it may enable vault reset attack, explained below, partially draining the underlying vault.

Proof of Concept

During the lifecycle of an ERC4626 vault, it's shares price related to underlying asset slowly increases. The initial ratio is usually 1 share per 1 unit of asset, but over time, when vault's supplies grow, the shares become slightly more expensive and for example they cost 1 share for 1.3 or 1.5 unit of an asset.

Now, considering references: Twitter thread by Kankodu, OZ issue 3800

The risk of "resetting" the vault exist, if vault shares liquidity is concentrated and someone could redeem most of the shares at once, e.g. by a flashloan, resetting the vault, and depositing assets at discounted price. Then, user can return the shares which are "bought" for less assets, repaying the flashloan and keeping the profit.

Since the protocol aims to gain popularity and each PT is dedicated to one specific vault, it should be considered a real risk, that if a vault becomes popular at a PT, which means, it's shares' liquidity is intensely concentrated in form of ibts, which is incentivized, then the vault may be attacked this way.

The flow of the attack is following:

• an ERC4626 vault, which gains popularity with Spectra, grows large. At some point the share price is 1.5 per 1 asset unit
• the vault has a corresponding PT where large amount of shares is stored as IBT
• now, anyone can take a flashLoan for max IBT balance.
• during the flashloan, attacker redeems IBT in the underlying vault for assets, almost emptying the vault.
• As the share price drops close to 1:1, user deposits assets again, gaining more than initially shares (IBTs)
• Attacker repays the flashloans, keeping the profit
• As per the reference, the profit is:
  total flashloan value in asset minus fees * (initial share price - reseted share price)

Tools Used

Manual approach.

Recommended Mitigation Steps

If flashloans are to be used, consider limiting their amount to only a percentage of IBT balance, instead of full contract balance.

Assessed type

ERC4626

[c4-pre-sort]

gzeon-c4 marked the issue as primary issue

[c4-pre-sort]

gzeon-c4 marked the issue as high quality report

[gzeon-c4]

attack likelihood is pretty low (require all ibt in this contract or the attacker have access to the rest)

[c4-sponsor]

yanisepfl (sponsor) disputed

[yanisepfl]

As the share price drops close to 1:1, user deposits assets again, gaining more than initially shares (IBTs)

Even if the liquidity was entirely concentrated in our protocol, and the IBT rate does indeed drop to 1, the user would get more IBTs than initially indeed, but those IBTs worth in assets will remain the same as the initial lower amount of IBTs with a higher rate.

Note: we have also considered many other attack vectors with the same hypothesis (where the IBTs are mostly concentrated in our PT vault) but different PoCs, and none of them lead to harming our protocol.

Therefore, the issue reported is wrong and disputed.

[c4-judge]

JustDravee marked the issue as unsatisfactory:
Invalid

[Arabadzhiew]

Hey @JustDravee, thank you for your work!

I believe that this issue was misjudged. The following statement provided by the sponsor seems to be incorrect:

Even if the liquidity was entirely concentrated in our protocol, and the IBT rate does indeed drop to 1, the user would get more IBTs than initially indeed, but those IBTs worth in assets will remain the same as the initial lower amount of IBTs with a higher rate.

The core idea of this issue is that when a malicious user redeems all of the shares they borrowed and resets the IBT vault rate to 1, they are simply going to mint back an amount of IBT shares that is equal to the borrowed amount + flash loan fees, repay those to protocol and keep the difference in assets for themselves. They are not going to mint more shares at the lower rate and pay those back, since that is not required from them at all.

Issue #111 has a coded PoC that demonstrates exactly how this vulnerability can be exploited. I am also not sure why this issue (#240) was selected as the primary one instead of the one I just mentioned, but perhaps you can elaborate more on that.

[blutorque]

Drop in IBTRate does profit the attacker and also do affect the protocol users.

Say you have 1000 IBTs backed by $2000 worth of USDT in an yield generating ERC4626 vault, the ratio of share to underlying is 1:2(which initially was 1:1 like any other vault).

As the sponsor said,

Note: we have also considered many other attack vectors with the same hypothesis (where the IBTs are mostly concentrated in our PT vault) but different PoCs, and none of them lead to harming our protocol.

Since, those IBTs are completely hold by PrincipalToken. Any user can claim the ownership of 1000 IBTs via flashloan, and redeem them all for the underlying. Now the attacker only required 1000 USDT to mint 1000 IBTs(since totalSupply==0), with which attacker can repay his debt.

So in total from IBTs vault, attacker manage to steal $1000 worth USDT.

My report #62 explains further ahead, How this actually affect the spectra user who have not claimed their yield yet.

Since, IBTRate drop back to 1. Attacker further calls updateYield function for any user,

    function updateYield(address _user) public override returns (uint256 updatedUserYieldInIBT) {
        (uint256 _ptRate, uint256 _ibtRate) = _updatePTandIBTRates();
    ...snip...

The purpose of doing that is to update the global ptRate and ibtRate via _updatePTandIBTRates(),

Since, ibtRate was dropped by 50% already, the ptRate will also drop to 50%,

        uint256 currentPTRate = currentIBTRate < ibtRate
            ? ptRate.mulDiv(
                currentIBTRate,
                ibtRate,
                roundUpPTRate ? Math.Rounding.Ceil : Math.Rounding.Floor
            )

	t0	t1	t2
ptRate	1	1	0.5
ibtRate	1	2	1

This results in negative yield from ibtRate,
https://github.com/code-423n4/2024-02-spectra/blob/383202d0b84985122fe1ba53cfbbb68f18ba3986/src/libraries/PrincipalTokenUtil.sol#L104-L107

as well as the less yield from the ptRate for user deposited at t1,
https://github.com/code-423n4/2024-02-spectra/blob/383202d0b84985122fe1ba53cfbbb68f18ba3986/src/libraries/PrincipalTokenUtil.sol#L98-L103

[kazantseff]

It looks like that this issue of ERC4626 vault price reset is the vulnerability in the implementation of the underlying ERC4626 vault itself, which should be out of scope, or am I missing something? It was also addressed and fixed here, it seems OpenZeppelin/openzeppelin-contracts#3800
OpenZeppelin/openzeppelin-contracts#3979

[Arabadzhiew]

@kazantseff Quoting what is stated in the contest README:

The protocol is expected to interact with any ERC4626 compliant vault.

So essentially, what you are saying is the same as saying that vulnerabilities that are arising from ERC20 token integrations are out of scope for a given contest, since those are "vulnerabilities in the implementations of the underlying ERC20 tokens themself", even if it is said that the protocol is intended to interact with any ERC20 tokens.

Additionally, from what I can see this flaw is still present in the most recent ERC4626 OZ implementation (not that this matters).

[kazantseff]

@Arabadzhiew
It was stated multiple times by a team that such issues are considered out of scope, since integrated IBTs will be monitored by the team, and it's up to the users to check if the IBT they put their money in is safe.

I feel like many issue could have been submitted based on vulnerabilities arising because of a 'wrong' or 'faulty' ERC4626 implementation, but at the end of the day we were auditing an ERC5095 PT contract, not an underlying vault it's being built on top, but that's just my opinion, the judge of course will have a final say.

[Arabadzhiew]

You are referring to something completely different. The issue at question is not reliant on a malicious vault implementation at all. In fact, the issue will be present when integrating with completely normal and fair-functioning vaults. That is why I have used the OpenZeppelin ERC4626 implementation as an example in my report.

[blutorque]

I feel like many issue could have been submitted based on vulnerabilities arising because of a 'wrong' or 'faulty' ERC4626 implementation,

But we never considered ERC4626 vault as faulty, as said before, either the supply is all in one place or the attacker own the whole,

Also check this comment, from sponser

Note: we have also considered many other attack vectors with the same hypothesis (where the IBTs are mostly concentrated in our PT vault) but different PoCs, and none of them lead to harming our protocol.

they tested the same hypothesis but unable to harm the protocol. We showed them how exactly under certain condition, this bug is exploitable.

Even with the legit vault, this could be a problem, also SS from contest channel

[yanisepfl]

Hello all,

Thanks for the interesting conversation.

After discussing this issue and #111 with the team, we came to the conclusion that:

• Having all the IBTs concentrated in our PTs is a very uncommon scenario. In particular, the usefulness of Spectra also relies on markets and if most of the IBTs are in our vaults then that would imply none or few are used as liquidity in the markets.
• As it was rightly mentioned by @kazantseff, it is mostly an issue on the IBT 4626 not to be protected against vault price resets. Our protocol is neutral, notices the rate change and act accordingly (as per our design).

We therefore consider it a low or medium severity issue.

Concerning the mitigation, we believe there is no better solution than to have dead shares. The mitigation proposed in #111 is too restrictive (e.g. imprecisions) and the one proposed here wouldn’t work depending on the attacker’s PTs/YTs/IBTs ownership.

I hope this helps!

Edit: We acknowledge this issue. It will be clearly specified in our UI and documentations that users should be careful where they invest their funds. In particular, they are expected to make sure the IBTs follow the ERC 4626 and that their rate cannot be easily controlled by someone (e.g. price reset attack, see OpenZeppelin/openzeppelin-contracts#3800 & OpenZeppelin/openzeppelin-contracts#3979). In particular, the PoC provided here does not work on Open Zeppelin's 4626.

[c4-judge]

JustDravee removed the grade

[c4-judge]

JustDravee marked issue #111 as primary and marked this issue as a duplicate of 111

[c4-judge]

JustDravee marked the issue as satisfactory