Spectra - ArmedGoose's results

gzeon-c4 marked the issue as primary issue

gzeon-c4 marked the issue as high quality report

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

yanisepfl (sponsor) disputed

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.

JustDravee marked the issue as unsatisfactory: Invalid

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.

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
            )

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

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 https://github.com/OpenZeppelin/openzeppelin-contracts/issues/3800 https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3979

@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).

@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.

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.

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

Hello all,

Thanks for the interesting conversation.

After discussing this issue and https://github.com/code-423n4/2024-02-spectra-findings/issues/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 https://github.com/code-423n4/2024-02-spectra-findings/issues/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 https://github.com/OpenZeppelin/openzeppelin-contracts/issues/3800 & https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3979). In particular, the PoC provided here does not work on Open Zeppelin's 4626.

JustDravee removed the grade

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

JustDravee marked the issue as satisfactory

gzeon-c4 marked the issue as sufficient quality report

JustDravee marked the issue as grade-b