sherlock-audit / 2024-01-napier-judging

9 stars 6 forks source link

jennifer37 - Missing refund left ether to users in swapUnderlyingForPt() #49

Closed sherlock-admin closed 7 months ago

sherlock-admin commented 7 months ago

jennifer37

medium

Missing refund left ether to users in swapUnderlyingForPt()

Summary

If users transfer more ether than needed in swapUnderlyingForPt(), contract should refund the left ether to user.

Vulnerability Detail

Function swapUnderlyingForPt() is payable. It means when underlying token is WETH9, users can send ether directly instead of WETH. If users send ethers directly, contract should refund left ether users.

    function swapUnderlyingForPt(
        address pool,
        uint256 index,
        uint256 ptOutDesired,
        uint256 underlyingInMax,
        address recipient,
        uint256 deadline
    ) external payable override nonReentrant checkDeadline(deadline) returns (uint256) {
        ...
        uint256 underlyingUsed = INapierPool(pool).swapUnderlyingForPt(
            index,
            ptOutDesired,
            address(this), // this contract will receive principal token from pool
            data
        );
        ...
}

For example, Alice estimates to use 1 ether to swap some pts, actual 0.9 ether is used. The left 0.1 ether is left in the contract. Although NapierRouter has one refundETH() inherited from PeripheryPayments to get left ether, anyone can refund ether in this contract.

    function refundETH() external payable {
        if (address(this).balance > 0) _safeTransferETH(msg.sender, address(this).balance);
    }

Impact

Users will lose some left ether in contract.

Code Snippet

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

Tool used

Manual Review

Recommendation

Refund left ether to user.

Duplicate of #81

sherlock-admin commented 7 months ago

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

takarez commented:

valid: medium(2)