code-423n4 / 2024-02-spectra-findings

4 stars 2 forks source link

If the underlying asset amount in the IBT changes outside of Spectra's control, it could lead to inaccuracies in the ibtRate tracking. #166

Closed c4-bot-7 closed 8 months ago

c4-bot-7 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-02-spectra/blob/383202d0b84985122fe1ba53cfbbb68f18ba3986/src/tokens/PrincipalToken.sol#L901-L914 https://github.com/code-423n4/2024-02-spectra/blob/383202d0b84985122fe1ba53cfbbb68f18ba3986/src/tokens/PrincipalToken.sol#L902 https://github.com/code-423n4/2024-02-spectra/blob/383202d0b84985122fe1ba53cfbbb68f18ba3986/src/tokens/PrincipalToken.sol#L901-L914 https://github.com/code-423n4/2024-02-spectra/blob/383202d0b84985122fe1ba53cfbbb68f18ba3986/src/tokens/PrincipalToken.sol#L902

Vulnerability details

Impact

If the underlying asset amount in the IBT changes without Spectra's awareness, it could affect ibtRate without triggering Spectra's rate update logic.m The assumption that is violated is that the only way ibtRate will change is based on user interactions with the Spectra protocol. The ibtRate relies on previewing redemptions from the IBT using IERC4626(ibt).previewRedeem(ibtUnit), as shown here:

function _getCurrentPTandIBTRates(bool roundUpPTRate) internal view returns (uint256, uint256) {

 uint256 currentIBTRate = IERC4626(ibt).previewRedeem(ibtUnit).toRay(_assetDecimals);

    // ...
}

However, if assets are deposited/withdrawn directly into the IBT without Spectra's awareness, the total assets backing the IBT can change without impacting the previewRedeem view.

Proof of Concept

This allows attackers to artificially inflate yields for Spectra users by injecting assets into the IBT vault without Spectra's awareness. Users could coordinate deposits to quickly compound yields, draining value from the protocol at the expense of other users.

The _getCurrentPTandIBTRates() function relies on previewRedeem() to get the IBT rate. However, previewRedeem() only provides a view into the contract balance - it does not track deposits/withdrawals.

An attacker could:

  1. Deposit assets into a Spectra vault, minting 1,000 IBT at a 1:1 rate with 1,000 DAI (for example).

  2. Later, directly deposit 100 DAI into the IBT vault from an external account.

  3. Spectra would still expect 1 IBT = 1 DAI based on previewRedeem(). But with 1,100 DAI in the vault, the true rate is 1 IBT = 1.1 DAI.

This means users can artificially inflate yields by injecting assets into the IBT vault, as Spectra cannot accurately detect the rate change.

The key issue is the assumption previewRedeem() provides an accurate view into the IBT rate at Line 902

uint256 currentIBTRate = IERC4626(ibt).previewRedeem(ibtUnit).toRay(_assetDecimals);

It does not account for changes in the total assets that could happen without user interactions through Spectra. This assumption needs to be fixed.

To exploit this:

  1. Alice deposits 100 DAI into Spectra, minting 100 IBT
  2. Spectra IBT vault has 100 DAI, ibtRate is 1 IBT = 1 DAI
  3. Eve deposits 10 DAI directly into the IBT vault via IERC4626.deposit()
  4. The IBT vault now has 110 DAI, so real ibtRate is 1 IBT = 1.1 DAI
  5. But Spectra still expects 1 IBT = 1 DAI based on previewRedeem
  6. Alice claims yield which is now inflated by 10% due to Eve's actions

This works because _getCurrentPTandIBTRates() relies solely on previewRedeem():

function _getCurrentPTandIBTRates(bool roundUpPTRate) internal view returns (uint256, uint256) {

    uint256 currentIBTRate = IERC4626(ibt).previewRedeem(ibtUnit).toRay(_assetDecimals);

    // ...
}

It has no way to account for changes in assets that happen outside Spectra deploys/redeems.

Tools Used

Manual review

Recommended Mitigation Steps

Directly integrate with the IBT vault to track total asset balances rather than rely on previews. This will give it an authoritative measure of the true backing assets and rates.

Assessed type

Oracle

c4-pre-sort commented 8 months ago

gzeon-c4 marked the issue as duplicate of #158

c4-pre-sort commented 8 months ago

gzeon-c4 marked the issue as sufficient quality report

c4-judge commented 8 months ago

JustDravee marked the issue as unsatisfactory: Invalid