WETH compatibility issue on Blast chain

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-07-karak/blob/ab18e1f6c03e118158369527baa2487b2b4616b1/src/SlashingHandler.sol#L52-L59

Vulnerability details

Impact

The slashing mechanism in the Vault contract may fail or behave inconsistently when handling certain ERC20 tokens on chains with non-standard token implementations, such as Blast.

Proof of Concept

According to ReadMe,

ERC20 token behaviors in scope

Question	Answer
ERC20 used by the protocol	Any (all possible ERC20s

WETH is a type of ERC-20.

Also, it's explicitly stated that the protocol will be deployed on the Blast chain too. However, if we look at the handleSlashing function of SlashingHandler.sol contract,

function handleSlashing(IERC20 token, uint256 amount) external {
        if (amount == 0) revert ZeroAmount();
        if (!_config().supportedAssets[token]) revert UnsupportedAsset();

 @>     SafeTransferLib.safeTransferFrom(address(token), msg.sender, address(this), amount);
        // Below is where custom logic for each asset lives
        SafeTransferLib.safeTransfer(address(token), address(0), amount);
    }

The token transfer is done using the safeTransferFrom method. This works fine on most chains (Ethereum, Optimism, Polygon, BSC) which use the standard WETH9 contract that handles the case 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 is that the WETH implementation on Blast uses a different contract, and does not have this src == msg.sender handling.

Also, the issue is presented in Wrapped Arbitrum and Wrapped Fantom.

Setup a foundry environment, get an Arbitrum RPC endpoint url and run the following command: forge test --match-test testTransferFromIssue --fork-url {YOUR_RPC}. This will revert with a ERC20: transfer amount exceeds allowance error, because of the issue described above.

pragma solidity ^0.8.0;

import "forge-std/Test.sol";
import "forge-std/console.sol";
import {IERC20} from "forge-std/interfaces/IERC20.sol";

contract PairTest is Test {
    using stdStorage for StdStorage;
    address alice = address(0x1);
    address bob = address(0x2);
    IERC20 public constant WETH = IERC20(0x82aF49447D8a07e3bd95BD0d56f35241523fBab1);

    function setUp() public {
    }

    function testTransferFromIssue() public {
      // given WETH to Alice
      stdstore
            .target(address(WETH))
            .sig(WETH.balanceOf.selector)
            .with_key(alice)
            .checked_write(10 ether);

      vm.startPrank(alice);

      //IERC20(address(WETH)).transfer(bob, 1 ether); //@audit toggle this line on and the one below off to see the allowance problem
      IERC20(address(WETH)).transferFrom(alice, bob, 1 ether);
      vm.stopPrank();
    }
}

Tools used

manual review

Recommended Mitigation Steps

Modify handleSlashing in slashinghandler.sol as follows:

-       SafeTransferLib.safeTransferFrom(address(token), msg.sender, address(this), amount);
+       SafeTransferLib.safeTransfer(address(token), address(this), amount);

Assessed type

ERC20

[c4-judge]

MiloTruck marked the issue as unsatisfactory: Invalid

[c4-judge]

MiloTruck marked the issue as not a duplicate

[MiloTruck]

Invalid.

The transfer here is from the caller of handleSlashing() to the SlashingHandler contract. The case you pointed out is when the transfer is from the SlashingHandler contract, if the call was:

SafeTransferLib.safeTransferFrom(address(token), address(this), ..., ...)