PeripheryPayments Manual Review Findings

PPS-01M: Improper Payable Attribute of Functions

Type	Severity	Location
Language Specific		PeripheryPayments.sol:L21, L36, L51, L72, L97, L105, L109

Description:

The referenced functions are set as payable yet do not handle native funds sent to them.

Impact:

Funds transmitted as part of the referenced payable calls can be compromised by any party via methods like refundETH.

Example:

Copy
19// public methods
20/// @inheritdoc IPeripheryPayments
21function unwrapWETH9(uint256 amountMinimum, address recipient) public payable override {
22    uint256 balanceWETH9 = IWETH9(WETH9).balanceOf(address(this));
23    require(balanceWETH9 >= amountMinimum, "Insufficient WETH9");
24
25    if (balanceWETH9 > 0) {
26        IWETH9(WETH9).withdraw(balanceWETH9);
27        TransferHelper.safeTransferETH(recipient, balanceWETH9);
28    }
29}

Recommendation:

We advise them to omit the payable keyword, preventing funds from being locked or mis-sent to the contract.

Alleviation:

After extensive discussions with the Native team, we concluded that this is the same paradigm applied by Uniswap V3 and is done for the sake of optimality. As such, the code behaves as expected and is meant to tap into its own funds prior to consuming the user's funds for fulfilling WETH9 payments.

PPS-02M: Inexistent Evaluation of Payer

Type	Severity	Location
Logical Fault		PeripheryPayments.sol:L120

Description:

The pay function will incorrectly assume that the payer is the contract itself when the token is set as WETH9 and the contract has sufficient balance.

Impact:

Unexpected behaviour can currently arise in the Router::swapCallback implementation when dealing with multiple orders in sequence that contain the WETH9 token.

Example:

Copy
114// internal methods
115/// @param token The token to pay
116/// @param payer The entity that must pay
117/// @param recipient The entity that will receive payment
118/// @param value The amount to pay
119function pay(address token, address payer, address recipient, uint256 value) internal {
120     if (token == WETH9 && address(this).balance >= value) {
121        //require(address(this).balance >= value, "Insufficient native token value");
122        // pay with WETH9
123        IWETH9(WETH9).deposit{value: value}(); // wrap only what is needed to pay
124        IWETH9(WETH9).transfer(recipient, value);
125    } else if (payer == address(this)) {
126        // pay with tokens already in the contract (for the exact input multihop case)
127        TransferHelper.safeTransfer(token, recipient, value);
128    } else {
129        // pull payment
130        TransferHelper.safeTransferFrom(token, payer, recipient, value);
131    }
132}

Recommendation:

Given that the contract can be used in multiple orders, a user may want to use their own WETH to satisfy a particular payment and use the contract's address(this).balance for another purpose. We advise the code to properly validate that the payer is the contract itself in the first conditional. As an added note, the conditionals can be optimized by including an outer if-else branch that evaluates payer == address(this) and an inner if-else clause that evaluates token == WETH9 && address(this).balance >= value, optimizing the code's gas cost.

Alleviation:

After extensive discussions with the Native team, we concluded that this is the same paradigm applied by Uniswap V3 and is done for the sake of optimality. As such, the code behaves as expected and is meant to tap into its own funds prior to consuming the user's funds for fulfilling WETH9 payments.