WETH tokens sent to the Most contract will be temporarily locked

[hats-bug-reporter[bot]]

Github username: -- Twitter username: 97Sabit Submission hash (on-chain): 0xceff9669a8dd879f6eb3f7a6a1c4830c9ff4237c5ba3b75a50ceabdd00664768 Severity: medium

Description: Description\ The initialize function calls the _pause() function which pauses the contract when initialized.

   function initialize(
        address[] calldata _committee,
        uint256 _signatureThreshold,
        address owner,
        address payable _wethAddress
    ) public initializer {
        committeeId = 0;
        wethAddress = _wethAddress;

        _setCommittee(_committee, _signatureThreshold);

        __Ownable_init(owner);
        __Pausable_init();
        _pause();
    }

Then sendRequestNative function is marked whenNotPaused. It means the function won't work when called the function is invoked because the contract is paused. Test in the PoC section.

However, there is a receive function in the contract that can receive weth tokens and not affected by the _pause function:

   receive() external payable {
        require(msg.sender == wethAddress);
    }

The impact of this is that weth tokens sent to the Most contract will be received by the contract but temporarily locked in the contract. The tokens will be withdrawable only when recoverNative is called by owner.

1. Proof of Concept (PoC) File

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import {Test, console2} from "forge-std/Test.sol";

contract PausableUpgradeable {

    struct PausableStorage {
        bool _paused;
    }

    bytes32 private constant PausableStorageLocation = 0xcd5ed15c6e187e77e9aee88184c21f4f2182ab5827cb3b7e07fbedcd63f03300;

    function _getPausableStorage() private pure returns (PausableStorage storage $) {
        assembly {
            $.slot := PausableStorageLocation
        }
    }

    event Paused(address account);
    event Unpaused(address account);
    error ExpectedPause();
    error EnforcedPause();

        modifier whenNotPaused() {
        _requireNotPaused();
        _;
    }

    modifier whenPaused() {
        _requirePaused();
        _;
    }

    function paused() public view virtual returns (bool) {
        PausableStorage storage $ = _getPausableStorage();
        return $._paused;
    }

    function _requireNotPaused() internal view virtual {
        if (paused()) {
            revert EnforcedPause();
        }
    }

    function _requirePaused() internal view virtual {
        if (!paused()) {
            revert ExpectedPause();
        }
    }

    function _pause() internal virtual whenNotPaused {
        PausableStorage storage $ = _getPausableStorage();
        $._paused = true;
        emit Paused(msg.sender);
    }

    function _unpause() internal virtual whenPaused {
        PausableStorage storage $ = _getPausableStorage();
        $._paused = false;
        emit Unpaused(msg.sender);
    }
}

contract Most is PausableUpgradeable {

      function initialize() public {
        _pause();
    }
 function sendRequestNative(
        bytes32 destReceiverAddress
    ) external payable whenNotPaused {
        // do something
    }

contract MostTest is Test {
    Most most;

    address user = vm.addr(1);

    function setUp() public {
        most = new Most();
    }

    function testSendRequestNative() public {
        vm.prank(user); 
        most.initialize();
        vm.expectRevert();
        most.sendRequestNative(0x7465737400000000000000000000000000000000000000000000000000000000);
        vm.stopPrank();
    }
}

Output:

orge test -vvvv [⠔] Compiling... [⠘] Compiling 1 files with 0.8.22 [⠃] Solc 0.8.22 finished in 1.21s Compiler run successful!

Running 1 test for test/A.t.sol:MostTest [PASS] testSendRequestNative() (gas: 35172) Traces: [35172] MostTest::testSendRequestNative() ├─ [0] VM::prank(0x7E5F4552091A69125d5DfCb7b8C2659029395Bdf) │ └─ ← () ├─ [23528] Most::initialize() │ ├─ emit Paused(account: 0x7E5F4552091A69125d5DfCb7b8C2659029395Bdf) │ └─ ← () ├─ [0] VM::expectRevert(custom error f4844814:) │ └─ ← () ├─ [357] Most::sendRequestNative(0x7465737400000000000000000000000000000000000000000000000000000000) │ └─ ← EnforcedPause() ├─ [0] VM::stopPrank() │ └─ ← () └─ ← ()

Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.12ms

Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)

[krzysztofziobro]

The PoC does not show that the funds are indeed locked in the contract

[ololade97]

The PoC does not show that the funds are indeed locked in the contract

So true the PoC supplied didn't show this. But tokens will be locked because the PoC showed that some part of the contract is paused and the receive function is not.

It would be unfair to the auditor here if the protocol goes on to rectify the issue.

[krzysztofziobro]

The author can correct the PoC here - it's not obvious to me why the funds would be locked