Incorrect Handling of safeTransferFrom for WETH on Non-WETH9 Chains Causes Settlement Failures

[c4-bot-7]

Lines of code

https://github.com/code-423n4/2024-05-predy/blob/a9246db5f874a91fb71c296aac6a66902289306a/src/base/SettlementCallbackLib.sol#L48 https://github.com/code-423n4/2024-05-predy/blob/a9246db5f874a91fb71c296aac6a66902289306a/src/base/SettlementCallbackLib.sol#L105 https://github.com/code-423n4/2024-05-predy/blob/a9246db5f874a91fb71c296aac6a66902289306a/src/base/SettlementCallbackLib.sol#L128 https://github.com/code-423n4/2024-05-predy/blob/a9246db5f874a91fb71c296aac6a66902289306a/src/base/SettlementCallbackLib.sol#L152 https://github.com/code-423n4/2024-05-predy/blob/a9246db5f874a91fb71c296aac6a66902289306a/src/base/SettlementCallbackLib.sol#L177

Vulnerability details

Impact

The failure to handle the safeTransferFrom function correctly can prevent the protocol from executing certain functions on chains that do not use the WETH9 contract.

This can lead to:

Failed settlements in trading operations.

Overall disruption of protocol functionality on affected chains.

Proof of Concept

The scenario here is that if the token being transferred is WETH, then it has a problem.

In the SettlementCallbackLib, safeTransferFrom is used to transfer tokens. If the WETH implementation does not handle the case where src == msg.sender, the transfer will fail.

function execSettlement(
    IPredyPool predyPool,
    address quoteToken,
    address baseToken,
    SettlementParams memory settlementParams,
    int256 baseAmountDelta
) internal {
    if (settlementParams.fee < 0) {
        //@c4-audit Weth9 failure on ARB
->        ERC20(quoteToken).safeTransferFrom(
            settlementParams.sender, address(predyPool), uint256(-settlementParams.fee)
        );
    }
    ..////.....
}

The transfer using ERC20(quoteToken).safeTransferFrom (see line 92) works well on most chains (Ethereum, Optimism, Polygon, BSC) because they use the standard WETH9 contract that handles the case when src == msg.sender

WETH9.sol;
if (src != msg.sender && allowance[src][msg.sender] != uint(-1)) {
  require(allowance[src][msg.sender] >= wad);
  allowance[src][msg.sender] -= wad;
}

The problem arises on Arbitrum.

Note:- Note: The Sender here is the filler, and the filler is a contract because the filler will execute the parameters inside _executeOrderV3. Creating parameters is too difficult for an EOA (Externally Owned Account). The filler is a contract.

The following functions inside the SettlementCallbackLib contract are also affected:

• sell
• buy

Tools Used

Manual Review

Recommended Mitigation

Before calling safeTransferFrom, check if the allowance is sufficient. If not, call approve to set the necessary allowance. This ensures that the transfer will succeed even if the WETH implementation does not handlesrc == msg.sender.

Use safetransfer Instead of safeTransferFrom When Possible.

Assessed type

Error