sherlock-audit / 2024-01-napier-judging

8 stars 5 forks source link

MatricksDeCoder - Protocol may not work well with pausable tokens #113

Closed sherlock-admin closed 5 months ago

sherlock-admin commented 5 months ago

MatricksDeCoder

medium

Protocol may not work well with pausable tokens

Summary

Protocol will not work well with yield earning tokens that may have pause functionality like yTokens. Common stablecoin tokens with Pause functionality like USDC or USDT

Vulnerability Detail

yield earning tokens like yTokens from Yearn Finance e.g yETH, yDAI, yUSDC can be paused in certain cases e. g by governance. This implies all parts of protocol may not be able to send tokens in and out of protocol, transfer these tokens as expected

Impact

It will impact adding removing liquidity, swapping, skimming, returning funds to users and more aspect. e, g

The following parts transferrring underlying token will fail https://github.com/sherlock-audit/2024-01-napier/blob/main/v1-pool/src/NapierPool.sol#L284

/// INTERACTION ///
        underlying.safeTransfer(recipient, underlyingOut);

Swapping bucket tokens for underlying will faill https://github.com/sherlock-audit/2024-01-napier/blob/main/v1-pool/src/NapierPool.sol#L362

/// INTERACTION ///
            // dev: Optimistically transfer underlying to recipient
            underlying.safeTransfer(recipient, underlyingOut);

skimming of Napier Pool will fail https://github.com/sherlock-audit/2024-01-napier/blob/main/v1-pool/src/NapierPool.sol#L533

 if (feesAndExcess != 0) underlying.safeTransfer(feeRecipient, feesAndExcess);

Swapping with underlying token will fail https://github.com/sherlock-audit/2024-01-napier/blob/main/v1-pool/src/NapierRouter.sol#L275

uint256 prevBalance = pt.balanceOf(address(this));
        uint256 underlyingUsed = INapierPool(pool).swapUnderlyingForPt(
            index,
            ptOutDesired,
            address(this), // this contract will receive principal token from pool
            data
        );

        pt.safeTransfer(recipient, pt.balanceOf(address(this)) - prevBalance);
        return underlyingUsed;

https://github.com/sherlock-audit/2024-01-napier/blob/main/v1-pool/src/TrancheRouter.sol#L53

} else {
            underlying.safeTransferFrom(msg.sender, address(this), underlyingAmount);
        }

Code Snippet

underlying.safeTransferFrom(msg.sender, address(this), underlyingAmount);

Tool used

Manual Review

Recommendation

It is recommended to maybe not allow these tokens as they are the only potential pausable yield bearing tokens or to add mechanisms that can allow handling of this special case or additionally keep track of Yearn Governance of issues to be ahead of any potential pause of tokens

sherlock-admin commented 5 months ago

1 comment(s) were left on this issue during the judging contest.

tsvetanovv commented:

Invalid. The protocol has specified in the Readme that it will not use tokens with non-standard behaviour