Bahurum - `transferPosition()` to `address(0)` allows matching the same order multiple times

[sherlock-admin]

Bahurum

medium

transferPosition() to address(0) allows matching the same order multiple times

Summary

transferPosition() accepts address(0) as recipient. This will reset bulls[contractId] or bears[contractId] to the initial state, which allows to pass the check on the already matched order in checkIsValidOrder()

Vulnerability Detail

1. bear creates an order
2. bull matches the order
3. bull calls transferPosition(orderHash, true, address(0)). bulls[contractId] is reset to address(0)
4. bull matches the order again. Note that check at line 760 passes. If both bear and bull have enough allowance to the contract, then premium and collateral + fees are transfered to the contract.
5. points 3 and 4 can be repeated many times

Note that the bull will also loose fund, except in the case when the bear asks a zero collateral for filling the order. In such case the bear would expect that the order can only be filled once but it can be filled multiple times until his/her allowance to the contract is too low. In this case the bear looses all the allowance to the contract but the bull doesn't have to transfer any token. Note also that the owner of the contract cannot recover the bear's tokens stuck in the contract.

Impact

Griefing attack making a bear (maker) allowance stuck into the contract when it creates an order with very small or zero collateral required.

Code Snippet

    function transferPosition(bytes32 orderHash, bool isBull, address recipient) public {
        // ContractId
        uint contractId = uint(orderHash);

        if (isBull) {
            // Check that the msg.sender is the Bull
            require(msg.sender == bulls[contractId], "SENDER_NOT_BULL");

            bulls[contractId] = recipient;
        } else {
            // Check that the msg.sender is the Bear
            require(msg.sender == bears[contractId], "SENDER_NOT_BEAR");

            bears[contractId] = recipient;
        }

        emit TransferedPosition(orderHash, isBull, recipient);

Tool used

Manual Review

Recommendation

Deny transfering the position to the zero address

    function transferPosition(bytes32 orderHash, bool isBull, address recipient) public {
        require(recipient != address(0), "Zero address recipient");
    ...

Duplicate of #114