position ownership transfer breaks order cancellation functionality

Summary

If user Alice transfers ownership position to another Bob user. Bob cannot cancel the order because he is not the original maker of the order.

Vulnerability Detail

To cancel the order you must be the original maker:

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

Impact

Transferring of position ownership should allow the new user (the one that get the position ownership transferred to) to have the same functionality as the original owner.

Code Snippet

The following test shows that after transferring a position canceling an order doesn't work anymore. It gets reverted with NOT_SIGNER reason.

    function testCannotCancelTransferedOrder() public {
        BvbProtocol.Order memory order = defaultOrder();

        bytes32 orderHash = bvb.hashOrder(order);
        bytes memory signature = signOrder(bullPrivateKey, order);
        bvb.matchOrder(order, signature);

        uint bobPK = 567;
        address bob = vm.addr(bobPK);

        vm.prank(bull);
        bvb.transferPosition(orderHash, true,bob);

        vm.prank(bob);
        vm.expectRevert("NOT_SIGNER");
        bvb.cancelOrder(order);
    }

Tool used

foundry

Recommendation

Instead of just checking the immutable order.maker field. Implement something like an approve mapping on transferring positions. Do not forget to cancel previous approved owner on every ownership transfer.

sherlock-audit / 2022-11-bullvbear-judging

imare - position ownership transfer breaks order cancellation functionality #103