hats-finance / Most--Aleph-Zero-Bridge-0xab7c1d45ae21e7133574746b2985c58e0ae2e61d

Aleph Zero bridge to Ethereum
Apache License 2.0
0 stars 1 forks source link

Calling _pause() in initialize() causes a denial of service by preventing all whenNotPaused functions from being called #58

Open hats-bug-reporter[bot] opened 5 months ago

hats-bug-reporter[bot] commented 5 months ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0xb853533339febe1c794a5c6d77fe809352d8f1dae1bf89f7c29bb1cf94ae7405 Severity: medium

Description: Description\ In the Most.sol's initialize() function, _pause() is called after initializing the contract.

_pause() sets the internal _paused state to true, which prevents any functions with the whenNotPaused modifier from executing.

This includes critical functions like sendRequest(), sendRequestNative, and receiveRequest.

The purpose of the pausable contract is to serve as an emergency stop mechanism.

The vulnerability causes a temporary denial of service until the unpause() function is called by the 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 sendRequest(
        bytes32 srcTokenAddress,
        uint256 amount,
        bytes32 destReceiverAddress
    ) external whenNotPaused {
        //do something
    }

      function unpause() external /*onlyOwner*/ {
        _unpause();
    }
}

contract MostTest is Test {
    Most most;

    address user = vm.addr(1);

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

    function testPause() public {
        vm.prank(user);
        most.initialize();
        vm.expectRevert();
     // most.unpause();
        most.sendRequest(0x7465737400000000000000000000000000000000000000000000000000000000, 3, 0x7465737400000000000000000000000000000000000000000000000000000000);
    }

}

Here's the result:

forge 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] testPause() (gas: 35167) Traces: [35167] MostTest::testPause() ├─ [0] VM::prank(0x7E5F4552091A69125d5DfCb7b8C2659029395Bdf) │ └─ ← () ├─ [23600] Most::initialize() │ ├─ emit Paused(account: 0x7E5F4552091A69125d5DfCb7b8C2659029395Bdf) │ └─ ← () ├─ [0] VM::expectRevert(custom error f4844814:) │ └─ ← () ├─ [526] Most::sendRequest(0x7465737400000000000000000000000000000000000000000000000000000000, 3, 0x7465737400000000000000000000000000000000000000000000000000000000) │ └─ ← EnforcedPause() └─ ← ()

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

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

  1. Revised Code File (Optional)
fbielejec commented 4 months ago

Precisely as intended. Not an issue