code-423n4 / 2022-01-insure-findings

2 stars 0 forks source link

Insurance Pool Locking Does Not Propagate To All Markets #347

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

leastwood

Vulnerability details

Impact

The applyCover() function locks all index markets in order to ensure compensation is properly accounted for when the insurance pool resumes trading. However, it seems that only IndexTemplate.sol is locked, even though it may potentially make a call to CDSTemplate.sol for additional compensation to cover the shortage in payout.

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#L476-L481

function lock() external override {
    require(allocPoints[msg.sender] > 0);

    locked = true;
    emit Locked();
}

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

function compensate(uint256 _amount)
    external
    override
    returns (uint256 _compensated)
{
    require(
        allocPoints[msg.sender] > 0,
        "ERROR_COMPENSATE_UNAUTHORIZED_CALLER"
    );
    uint256 _value = vault.underlyingValue(address(this));
    if (_value >= _amount) {
        //When the deposited value without earned premium is enough to cover
        vault.offsetDebt(_amount, msg.sender);
        //vault.transferValue(_amount, msg.sender);
        _compensated = _amount;
    } else {
        //Withdraw credit to cashout the earnings
        uint256 _shortage;
        if (totalLiquidity() < _amount) {
            //Insolvency case
            _shortage = _amount - _value;
            uint256 _cds = ICDSTemplate(registry.getCDS(address(this)))
                .compensate(_shortage);
            _compensated = _value + _cds;
        }
        vault.offsetDebt(_compensated, msg.sender);
    }
    adjustAlloc();
    emit Compensated(msg.sender, _compensated);
}

Tools Used

Manual code review.

Recommended Mitigation Steps

Consider propagating market locking to all index markets and all CDS markets.

oishun1112 commented 2 years ago

It is in consideration that CDS is not locked. CDS is going to be a backup of all indices(means all pools) It is useless if CDS is locked whenever one of the pool is in Payout status in InsureDAO (anyone can create insurance pool) CDS is backup and tends to be used hardly ever, so not necessary to be locked as paying out. Instead, withdraw wait-time may be set longer than default.