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

4 stars 2 forks source link

The `_depositIBT` mints more share than it should, due to rounding up `ptRate` #36

Closed c4-bot-4 closed 8 months ago

c4-bot-4 commented 8 months ago

Lines of code

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

Vulnerability details

Impact

_depositIBT uses the _convertIBTsToShares to calculate the amount of shares to be minted for input _ibts. The shares here is correctly rounded down which can be seen below;

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

function _convertIBTsToShares(
    uint256 _ibts,
    bool _roundUp
) internal view returns (uint256 shares) {
    (uint256 _ptRate, uint256 _ibtRate) = _getPTandIBTRates(false);
    if (_ptRate == 0) {
        revert RateError();
    }
    shares = _ibts.mulDiv(
        _ibtRate,
        _ptRate,
        _roundUp ? Math.Rounding.Ceil : Math.Rounding.Floor // @audit-ok rounds down
    );
}

However, the _ptRate which is calculated from _getPTandIBTRates passes the false parameter, rounding down the new _ptRate. Since the _ptRate is involved in share calculations and is inversely proportional to the shares we mint, the final shares its mints will actually be higher.

Check out the _convertIBTsToSharesPreview implementation which rightly rounds up the ptRate to evaluate shares amount.

Proof of Concept

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

Tools Used

Manual

Recommended Mitigation Steps

If this is not intentional, round up the ptRate the same way it has been done for _convertIBTsToSharesPreview function.

Assessed type

Other

c4-pre-sort commented 8 months ago

gzeon-c4 marked the issue as duplicate of #201

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