hansfriese - A malicious user can drain the protocol funds using `reclaimContract()`.

[sherlock-admin]

hansfriese

high

A malicious user can drain the protocol funds using reclaimContract().

Summary

A malicious user can drain the protocol funds using reclaimContract().

Vulnerability Detail

reclaimContract() is used to reclaim the contract after it's expired without settlement.

    function reclaimContract(Order calldata order) public nonReentrant { //@audit custom order
        bytes32 orderHash = hashOrder(order);

        // ContractId
        uint contractId = uint(orderHash);

        address bull = bulls[contractId];

        // Check that the contract is expired
        require(block.timestamp > order.expiry, "NOT_EXPIRED_CONTRACT");

        // Check that the contract is not settled
        require(!settledContracts[contractId], "SETTLED_CONTRACT");

        // Check that the contract is not reclaimed
        require(!reclaimedContracts[contractId], "RECLAIMED_CONTRACT");

        uint bullAssetAmount = order.premium + order.collateral;
        if (bullAssetAmount > 0) {
            // Transfer payment tokens to the Bull
            IERC20(order.asset).safeTransfer(bull, bullAssetAmount);
        }

        reclaimedContracts[contractId] = true;

        emit ReclaimedContract(orderHash, order);
    }

This function is callable by anyone and it transfers the funds to the Bull.

But it doesn't verify if the order was matched really so the below scenario would be possible.

• The protocol contains 1000 of the USDC now.
• A malicious user calls reclaimContract() with a custom order of order.expiry = 0, order.premium = 1000, order.collateral = 0 and other relevant settings.
• The contractId of this order is not matched yet and it will pass the below requirements.

    // ContractId
    uint contractId = uint(orderHash);

    address bull = bulls[contractId];

    // Check that the contract is expired
    require(block.timestamp > order.expiry, "NOT_EXPIRED_CONTRACT");

    // Check that the contract is not settled
    require(!settledContracts[contractId], "SETTLED_CONTRACT");

    // Check that the contract is not reclaimed
    require(!reclaimedContracts[contractId], "RECLAIMED_CONTRACT");

• Then bull will be address(0) and 1000 USDC will to transferred to address(0).

    uint bullAssetAmount = order.premium + order.collateral;
    if (bullAssetAmount > 0) {
        // Transfer payment tokens to the Bull
        IERC20(order.asset).safeTransfer(bull, bullAssetAmount);
    }

    reclaimedContracts[contractId] = true;

• So the protocol has lost the funds permanently.

Impact

The protocol funds might be lost by malicious users permanently.

Code Snippet

https://github.com/sherlock-audit/2022-11-bullvbear/blob/main/bvb-protocol/src/BvbProtocol.sol#L417

Tool used

Manual Review

Recommendation

We should check if bull != address(0) in reclaimContract().

address bull = bulls[contractId];

require(bull != address(0), "Invalid order");

Duplicate of #127