xiaoming90 - `TrancheRouter.issue` function does not sweep unused ETH back to the caller

[sherlock-admin]

xiaoming90

high

TrancheRouter.issue function does not sweep unused ETH back to the caller

Summary

The TrancheRouter.issue function does not sweep unused ETH back to the caller, resulting in a loss of assets for the affected users.

Vulnerability Detail

Assume that when executing the issue function, Bob sends 101 ETH but sets the underlyingAmount to 100 ETH. Line 51 below will wrap 100 ETH to 100 WETH. Line 58 below will pull 100 WETH from the Router and 100 WETH worth of PT+YT will be minted to Bob.

However, there is still 1 unused ETH left on the Router, which is not swept back to Bob's account at the end of the TX. As such, the remaining ETH will be stolen by MEV.

Most router functions within Napier protocol will sweep any unused assets back to the caller at the end of the TX to handle such a case. However, this feature was missed in the TrancheRouter.issue function.

https://github.com/sherlock-audit/2024-01-napier/blob/main/napier-v1/src/Tranche.sol#L179

File: TrancheRouter.sol
35:     /// @notice deposit an `underlyingAmount` of underlying token into the yield source, receiving PT and YT.
36:     /// @dev Accept native ETH.
37:     /// @inheritdoc ITrancheRouter
38:     function issue(address adapter, uint256 maturity, uint256 underlyingAmount, address to)
39:         external
40:         payable
41:         nonReentrant
42:         returns (uint256)
43:     {
44:         ITranche tranche =
45:             TrancheAddress.computeAddress(adapter, maturity, TRANCHE_CREATION_HASH, address(trancheFactory));
46:         IERC20 underlying = IERC20(tranche.underlying());
47: 
48:         // Transfer underlying tokens to this contract
49:         // If this contract holds enough ETH, wrap it. Otherwise, transfer from the caller.
50:         if (address(underlying) == address(WETH9) && address(this).balance >= underlyingAmount) {
51:             WETH9.deposit{value: underlyingAmount}();
52:         } else {
53:             underlying.safeTransferFrom(msg.sender, address(this), underlyingAmount);
54:         }
55:         // Force approve
56:         underlying.forceApprove(address(tranche), underlyingAmount);
57: 
58:         return tranche.issue(to, underlyingAmount);
59:     }

Impact

Loss of assets for the affected users.

Code Snippet

https://github.com/sherlock-audit/2024-01-napier/blob/main/napier-v1/src/Tranche.sol#L179

Tool used

Manual Review

Recommendation

Consider implementing a feature to sweep any unused assets back to the callers at the end of the TX.

[sherlock-admin]

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

tsvetanovv commented:

This is user input mistake

takarez commented:

valid: high(3)