`applyCover()` Does Not Enforce Index Market Lock

[code423n4]

Handle

leastwood

Vulnerability details

Impact

The applyCover() function is called by the insurance pool owner and intends to store data related to an insurance incident. Upon function execution, applyCover() iterates over all available index markets and calls lock(), denying all deposits and withdrawals from IndexTemplate.sol. However, due to the fact that IndexTemplate.resume() does not perform a proper check on PoolTemplate.sol, anyone can resume the market immediately and continue to deposit and withdraw while the insurance pool is paying out for an incident.

This may cause issues when the insurance pool resumes as any insolvency must be compensated from these other markets. By not locking pools during incident payouts, users can avoid having to compensate the insurance pool.

Proof of Concept

https://github.com/code-423n4/2022-01-insure/blob/main/contracts/PoolTemplate.sol#L655-L686

function applyCover(
    uint256 _pending,
    uint256 _payoutNumerator,
    uint256 _payoutDenominator,
    uint256 _incidentTimestamp,
    bytes32 _merkleRoot,
    string calldata _rawdata,
    string calldata _memo
) external override onlyOwner {
    require(paused == false, "ERROR: UNABLE_TO_APPLY");
    incident.payoutNumerator = _payoutNumerator;
    incident.payoutDenominator = _payoutDenominator;
    incident.incidentTimestamp = _incidentTimestamp;
    incident.merkleRoot = _merkleRoot;
    marketStatus = MarketStatus.Payingout;
    pendingEnd = block.timestamp + _pending;
    for (uint256 i = 0; i < indexList.length; i++) {
        if (indicies[indexList[i]].credit > 0) {
            IIndexTemplate(indexList[i]).lock();
        }
    }
    emit CoverApplied(
        _pending,
        _payoutNumerator,
        _payoutDenominator,
        _incidentTimestamp,
        _merkleRoot,
        _rawdata,
        _memo
    );
    emit MarketStatusChanged(marketStatus);
}

https://github.com/code-423n4/2022-01-insure/blob/main/contracts/IndexTemplate.sol#L459-L481

function resume() external override {
    uint256 _poolLength = poolList.length;

    for (uint256 i = 0; i < _poolLength; i++) {
        require(
            IPoolTemplate(poolList[i]).paused() == false,
            "ERROR: POOL_IS_PAUSED"
        );
    }

    locked = false;
    emit Resumed();
}

/**
    * @notice lock market withdrawal
    */
function lock() external override {
    require(allocPoints[msg.sender] > 0);

    locked = true;
    emit Locked();
}

Tools Used

Manual code review. Discussions with Oishun.

Recommended Mitigation Steps

Consider fixing the IndexTemplate.resume() function such that it also checks if IPoolTemplate(poolList[i]).marketStatus() == MarketStatus.Trading. This should properly enforce the lock while the insurance pool pays out funds to the policy holder(s).

[oishun1112]

We need to fix. We have request withdraw period (more than Payout period length), so the impact is not big as explanation.

[oishun1112]

https://github.com/code-423n4/2022-01-insure-findings/issues/278

[0xean]

dupe of #278