Rain Disclosed Report

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

Vulnerability details

• Title: Head-of-book pointer not updated on cancel, enabling gas-griefing

• The Problem: The _cancelSellOrder and _cancelBuyOrder functions correctly remove orders from the linked list at a given price tick but do not update the firstSellOrderPrice/firstBuyOrderPrice head pointers if the cancelled bucket becomes empty. This leaves the head pointer stale, pointing to an empty bucket, until the next matching pass traverses past it.

• Code that creates the vulnerability:

function _cancelSellOrder(
    uint256 option,
    uint256 price, // 1e18
    uint256 orderID,
    address caller
) private {
    if (orderBook[option][price][orderID].exists == false) {
        _revert(OrderDoesNotExist.selector);
    }
...
    linkedList.remove(nodeIndex);
    orderBook[option][price][orderID].exists = false;
...
    // NO head pointer update here
}

• Why it matters:
  • An attacker can repeatedly place and cancel orders at the cheapest ticks to force the next taker in enterOption or placeBuyOrder to iterate through a series of empty buckets, burning extra gas.
  • The attack is bounded by the number of ticks (~99) but still creates a griefing vector.

Impact & Attack Scenario

• Impact: Minor griefing and bounded gas overhead for takers. The issue does not risk funds or liveness but degrades UX and allows an attacker to increase others’ transaction costs at will. Per the bug bounty scope, this fits under “Griefing” and possibly “Block stuffing for profit.”

• Attack scenario:

  1. Setup: Order book has sell orders at ticks 0.02, 0.03, etc. firstSellOrderPrice = 0.02 ether.
  2. Attack: Attacker places a tiny sell order at 0.01 ether; firstSellOrderPrice becomes 0.01 ether. They then immediately cancel it.
  3. Stale state: firstSellOrderPrice remains 0.01 ether, but that bucket is empty.
  4. Griefing: The next taker calls enterOption. The matching loop starts at the stale head (0.01), finds it empty, advances to 0.02, finds the real order, and executes. The taker paid for an extra, useless loop iteration. An attacker can repeat this across multiple ticks to magnify the gas wasted.

Recommendation

Update the head-of-book pointers immediately on cancel, with safe bounds and a sentinel fallback, and optionally harden the linked-list library.

1. Sell side: update firstSellOrderPrice in _cancelSellOrder

• After removing the node and if the cancelled bucket equals the current head and is now empty, scan forward to the next non-empty sell bucket. If none is found up to 0.99 ether, set the head to 0 (no sells).

// After linkedList.remove(nodeIndex);
if (price == firstSellOrderPrice[option] && linkedList.isEmpty()) {
    uint256 head = price;
    // Advance to the next non-empty bucket; bounded by max sell price
    while (head <= 0.99 ether && sellOrders[option][head].isEmpty()) {
        head += TICK_SPACING;
    }
    // Sentinel fallback if no sells remain
    firstSellOrderPrice[option] = (head <= 0.99 ether) ? head : 0;
}

2. Buy side: update firstBuyOrderPrice in _cancelBuyOrder

• After removing the node and if the cancelled bucket equals the current head and is now empty, scan downward to the next non-empty buy bucket. If none is found down to TICK_SPACING, set the head to 0 (no buys).

// After linkedList.remove(nodeIndex);
if (price == firstBuyOrderPrice[option] && linkedList.isEmpty()) {
    uint256 head = price;
    // Move down to the next non-empty bucket; bounded by min buy price
    while (head >= TICK_SPACING && buyOrders[option][head].isEmpty()) {
        head -= TICK_SPACING;
    }
    // Sentinel fallback if no buys remain
    firstBuyOrderPrice[option] = (head >= TICK_SPACING && !buyOrders[option][head].isEmpty()) ? head : 0;
}

3. Optional hardening (safe, no behavior change)

• In LinkedList.initialize:

require(!list.isInitialized, "Already initialized");

• In LinkedList.remove:

require(node.nextIndex != 0 && node.prevIndex != 0, "Invalid node");

These changes eliminate the stale-head griefing vector by keeping head pointers current after cancellations and ensure callers cannot corrupt the list with invalid indices.

I create a small PoC to prove the issue

function test_PoC_StaleHeadPointer_OnCancel_SellHeadNotAdvanced() public {
        // Use private pool instance; poolOwner holds initial votes on each option
        uint256 option = 1;
        uint256 tick = rainPool.TICK_SPACING(); // 0.01 ether
        uint256 p1 = tick;            // cheapest tick
        uint256 p2 = tick * 2;        // next tick

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

        // Place two sell orders at adjacent ticks
        vm.startPrank(poolOwner);
        uint256 id1 = rainPool.placeSellOrder(option, p1, 1_000); // ensure (votes*price)/1e18 >= 1
        uint256 id2 = rainPool.placeSellOrder(option, p2, 1_000);
        vm.stopPrank();

        // Head should point to the cheapest active price p1
        assertEq(rainPool.firstSellOrderPrice(option), p1, "head should point to p1");

        // Cancel the only order at p1; bucket becomes empty
        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] = id1;

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

        // PoC: head remains stale (still p1) even though p1 bucket is now empty;
        // it will be advanced only on the next matching pass.
        assertEq(rainPool.firstSellOrderPrice(option), p1, "stale head not updated after cancel");
    }