xiaoming90 - `swapUnderlyingForYt` function did not sweep unused ETH back to users

[sherlock-admin]

xiaoming90

high

swapUnderlyingForYt function did not sweep unused ETH back to users

Summary

swapUnderlyingForYt does not sweep the remaining unused ETH back to users, resulting in a loss of assets for the victim.

Vulnerability Detail

The Napier Router is designed to support both WETH and native ETH for its operation.

The swapUnderlyingForYt function allows the users to pay in Native ETH with the support of the payable modifier.

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

File: NapierRouter.sol
297:     function swapUnderlyingForYt(
298:         address pool,
299:         uint256 index,
300:         uint256 ytOutDesired,
301:         uint256 underlyingInMax,
302:         address recipient,
303:         uint256 deadline
304:     ) external payable override nonReentrant checkDeadline(deadline) returns (uint256) {

During the swap, it will make a callback to the Router, and the Router will need to ensure that uPull amount of ETH has been collected.

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

File: NapierRouter.sol
85:     function swapCallback(int256 underlyingDelta, int256 ptDelta, bytes calldata data) external override {
..SNIP..
152:             // Pull underlying from payer.
153:             // Economically, it's almost unlikely that the payer doesn't need to pay underlying asset.
154:             // But if the above case happens, it would be reverted.
155:             if (params.underlyingDeposit <= uReceived) revert Errors.RouterNonSituationSwapUnderlyingForYt();
156:             uint256 uPull = params.underlyingDeposit - uReceived;
157:             if (uPull > params.maxUnderlyingPull) revert Errors.RouterExceededLimitUnderlyingIn();
158:             _pay(underlying, params.payer, address(this), uPull);

Before execution, the user does not know the exact amount of Native ETH to be sent to the contract as the actual on-chain condition might be different from the result during off-chain simulation. Thus, users will often send more ETH as a buffer to avoid unnecessary revert.

Bob transferred 100 ETH to the contract. However, in the end, only needs 99 ETH and the uPull is set to 99 ETH. Thus, the remaining 1 ETH is stuck in the contract at the end of the TX and will be stolen by MEV or front-runner after the TX.

https://github.com/sherlock-audit/2024-01-napier/blob/main/v1-pool/src/base/PeripheryPayments.sol#L82

File: PeripheryPayments.sol
82:     function _pay(address token, address payer, address recipient, uint256 value) internal {
83:         if (token == address(WETH9) && address(this).balance >= value) {
84:             // pay with WETH9
85:             WETH9.deposit{value: value}(); // wrap only what is needed to pay
86:             WETH9.transfer(recipient, value);

Impact

Loss of assets for the victim.

Code Snippet

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

Tool used

Manual Review

Recommendation

The remaining unused ETH should be returned to the caller after the TX.

Duplicate of #81

[sherlock-admin]

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

takarez commented:

valid: medium(2)