sherlock-audit / 2024-01-napier-judging

9 stars 6 forks source link

FastTiger - As frontrunning occurs, the protocol suffers a loss of funds. #64

Closed sherlock-admin2 closed 7 months ago

sherlock-admin2 commented 7 months ago

FastTiger

high

As frontrunning occurs, the protocol suffers a loss of funds.

Summary

When the Manager calls Tranche.sol#claimIssuanceFees function, the issuanceFees is calculated as the underlyingasset and transferred to the feeRecipient. At this time, users will see Tranche.sol#claimIssuanceFees function has been called and redeem the token first. Then the protocol will suffer a loss.

Vulnerability Detail

The Tranche.sol#issue function and Tranche.sol#claimIssuanceFees function are as follows.

    function issue(
        address to,
        uint256 underlyingAmount
    ) external nonReentrant whenNotPaused notExpired returns (uint256 issued) {
        uint256 _lscale = lscales[to];
        uint256 accruedInTarget = unclaimedYields[to];
        uint256 _maxscale = gscales.maxscale;

        // NOTE: Updating mscale/maxscale in the cache before the issue to determine the accrued yield.
        uint256 cscale = adapter.scale();

        if (cscale > _maxscale) {
            // If the current scale is greater than the maxscale, update scales
            _maxscale = cscale;
            gscales.maxscale = cscale.toUint128();
        }
        // Updating user's last scale to the latest maxscale
        lscales[to] = _maxscale;
        delete unclaimedYields[to];

        uint256 yBal = _yt.balanceOf(to);
        // If recipient has unclaimed interest, claim it and then reinvest it to issue more PT and YT.
        // Reminder: lscale is the last scale when the YT balance of the user was updated.
        if (_lscale != 0) {
            accruedInTarget += _computeAccruedInterestInTarget(_maxscale, _lscale, yBal);
        }

        // Transfer underlying from user to adapter and deposit it into adapter to get target token
        _underlying.safeTransferFrom(msg.sender, address(adapter), underlyingAmount);
        (, uint256 sharesMinted) = adapter.prefundedDeposit();

        // Deduct the issuance fee from the amount of target token minted + reinvested yield
        // Fee should be rounded up towards the protocol (against the user) so that issued principal is rounded down
        // Hackmd: F0
        // ptIssued
        // = (u/s + y - fee) * S
        // = (sharesUsed - fee) * S
        // where u = underlyingAmount, s = current scale, y = reinvested yield, S = maxscale
        uint256 sharesUsed = sharesMinted + accruedInTarget;
        uint256 fee = sharesUsed.mulDivUp(issuanceFeeBps, MAX_BPS);
        issued = (sharesUsed - fee).mulWadDown(_maxscale);

        // Accumulate issueance fee in units of target token
222:    issuanceFees += fee;
        // Mint PT and YT to user
        _mint(to, issued);
        _yt.mint(to, issued);

        emit Issue(msg.sender, to, issued, sharesUsed);
    }
    function claimIssuanceFees() external onlyManagement returns (uint256) {
        uint256 fees = issuanceFees - 1; // Ensure that the slot is not cleared, for gas savings
        issuanceFees = 1;
584:    _target.safeTransfer(address(adapter), fees);
585:    (uint256 feesInUnderlying, ) = adapter.prefundedRedeem(feeRecipient);
        return feesInUnderlying;
    }

As you can see,the fee is accumulated each time Tranche.sol#issue function is called. Therefore this value is not small, It will grow in an instant. If the Manager calls Tranche.sol#claimIssuanceFees function, the price of the token will go down.

So users will redeem their tokens with each other. Therefore this protocol will suffer a loss of funds.

Impact

This protocol will suffer a loss of funds.

Code Snippet

https://github.com/sherlock-audit/2024-01-napier/blob/main/napier-v1/src/Tranche.sol#L179C5-L228C6 https://github.com/sherlock-audit/2024-01-napier/blob/main/napier-v1/src/Tranche.sol#L581C5-L587C6

Tool used

Manual Review

Recommendation

    function claimIssuanceFees() external onlyManagement returns (uint256) {
        uint256 fees = issuanceFees - 1; // Ensure that the slot is not cleared, for gas savings
        issuanceFees = 1;
        _target.safeTransfer(address(adapter), fees);
        (uint256 feesInUnderlying, ) = adapter.prefundedRedeem(feeRecipient);
        return feesInUnderlying;
    }

you must add expired modifier to this code, Or when call this function, can also specify gasLimit.