InsureDAO contest - leastwood's results

Handle

leastwood

Vulnerability details

Impact

The current method of market creation involves calling Factory.createMarket() with a list of approved _conditions and _references accounts. If a registered template address has templates[address(_template)].isOpen == true, then any user is able to call createMarket() using this template. If the template points to PoolTemplate.sol, then a malicious market creator can abuse PoolTemplate.initialize() as it makes a vault deposit from an account that they control. The vulnerable internal function, _depositFrom(), makes a vault deposit from the _references[4] address (arbitrarily set to an approved reference address upon market creation).

Hence, if approved _references accounts have set an unlimited approval amount for Vault.sol before deploying their market, a malicious user can frontrun market creation and cause these tokens to be transferred to the incorrect market.

This issue can cause honest market creators to have their tokens transferred to an incorrectly configured market, leading to unrecoverable funds. If their approval to Vault.sol was set to the unlimited amount, malicious users will also be able to force honest market creators to transfer more tokens than they would normally want to allow.

Proof of Concept

https://github.com/code-423n4/2022-01-insure/blob/main/contracts/Factory.sol#L158-L231

function createMarket(
    IUniversalMarket _template,
    string memory _metaData,
    uint256[] memory _conditions,
    address[] memory _references
) public override returns (address) {
    //check eligibility
    require(
        templates[address(_template)].approval == true,
        "ERROR: UNAUTHORIZED_TEMPLATE"
    );
    if (templates[address(_template)].isOpen == false) {
        require(
            ownership.owner() == msg.sender,
            "ERROR: UNAUTHORIZED_SENDER"
        );
    }
    if (_references.length > 0) {
        for (uint256 i = 0; i < _references.length; i++) {
            require(
                reflist[address(_template)][i][_references[i]] == true ||
                    reflist[address(_template)][i][address(0)] == true,
                "ERROR: UNAUTHORIZED_REFERENCE"
            );
        }
    }

    if (_conditions.length > 0) {
        for (uint256 i = 0; i < _conditions.length; i++) {
            if (conditionlist[address(_template)][i] > 0) {
                _conditions[i] = conditionlist[address(_template)][i];
            }
        }
    }

    if (
        IRegistry(registry).confirmExistence(
            address(_template),
            _references[0]
        ) == false
    ) {
        IRegistry(registry).setExistence(
            address(_template),
            _references[0]
        );
    } else {
        if (templates[address(_template)].allowDuplicate == false) {
            revert("ERROR: DUPLICATE_MARKET");
        }
    }

    //create market
    IUniversalMarket market = IUniversalMarket(
        _createClone(address(_template))
    );

    IRegistry(registry).supportMarket(address(market));
    
    markets.push(address(market));


    //initialize
    market.initialize(_metaData, _conditions, _references);

    emit MarketCreated(
        address(market),
        address(_template),
        _metaData,
        _conditions,
        _references
    );

    return address(market);
}

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

function initialize(
    string calldata _metaData,
    uint256[] calldata _conditions,
    address[] calldata _references
) external override {
    require(
        initialized == false &&
            bytes(_metaData).length > 0 &&
            _references[0] != address(0) &&
            _references[1] != address(0) &&
            _references[2] != address(0) &&
            _references[3] != address(0) &&
            _references[4] != address(0) &&
            _conditions[0] <= _conditions[1],
        "ERROR: INITIALIZATION_BAD_CONDITIONS"
    );
    initialized = true;

    string memory _name = string(
        abi.encodePacked(
            "InsureDAO-",
            IERC20Metadata(_references[1]).name(),
            "-PoolInsurance"
        )
    );
    string memory _symbol = string(
        abi.encodePacked("i-", IERC20Metadata(_references[1]).symbol())
    );
    uint8 _decimals = IERC20Metadata(_references[0]).decimals();

    initializeToken(_name, _symbol, _decimals);

    registry = IRegistry(_references[2]);
    parameters = IParameters(_references[3]);
    vault = IVault(parameters.getVault(_references[1]));

    metadata = _metaData;

    marketStatus = MarketStatus.Trading;

    if (_conditions[1] > 0) {
        _depositFrom(_conditions[1], _references[4]);
    }
}

Tools Used

Manual code review. Discussions with kohshiba.

Recommended Mitigation Steps

After discussions with the sponsor, they have opted to parse a _creator address to PoolTemplate.sol which will act as the depositor and be set to msg.sender in Factory.createMarket(). This will prevent malicious market creators from forcing vault deposits from unsuspecting users who are approved in Factory.sol and have also approved Vault.sol to make transfers on their behalf.

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).

Handle

leastwood

Vulnerability details

Impact

If an incident has occurred where an insurance policy is to be redeemed. The market is put into the MarketStatus.Payingout mode where the _insurance.insured account is allowed to redeem their cover and receive a payout amount. Upon paying out the insurance cover, any user is able to resume the market by calling PoolTemplate.resume(). This function will compensate the insurance pool if it is insolvent by querying IndexTemplate.compensate() which in turn queries CDSTemplate.compensate() to cover any shortage.

In the event none of these entities are able to cover the shortage in debt, the system accrues the debt. However, there is currently no mechanism to ensure when transferDebt() is called in PoolTemplate.resume(), the accrued system debt is paid off. Therefore, the system may incorrectly handle insolvency on an extreme edge case, generating system instability.

Proof of Concept

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

function resume() external {
    require(
        marketStatus == MarketStatus.Payingout &&
            pendingEnd < block.timestamp,
        "ERROR: UNABLE_TO_RESUME"
    );

    uint256 _debt = vault.debts(address(this));
    uint256 _totalCredit = totalCredit;
    uint256 _deductionFromIndex = (_debt * _totalCredit * MAGIC_SCALE_1E6) /
        totalLiquidity();
    uint256 _actualDeduction;
    for (uint256 i = 0; i < indexList.length; i++) {
        address _index = indexList[i];
        uint256 _credit = indicies[_index].credit;
        if (_credit > 0) {
            uint256 _shareOfIndex = (_credit * MAGIC_SCALE_1E6) /
                _totalCredit;
            uint256 _redeemAmount = _divCeil(
                _deductionFromIndex,
                _shareOfIndex
            );
            _actualDeduction += IIndexTemplate(_index).compensate(
                _redeemAmount
            );
        }
    }

    uint256 _deductionFromPool = _debt -
        _deductionFromIndex /
        MAGIC_SCALE_1E6;
    uint256 _shortage = _deductionFromIndex /
        MAGIC_SCALE_1E6 -
        _actualDeduction;

    if (_deductionFromPool > 0) {
        vault.offsetDebt(_deductionFromPool, address(this));
    }

    vault.transferDebt(_shortage);

    marketStatus = MarketStatus.Trading;
    emit MarketStatusChanged(MarketStatus.Trading);
}

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

Tools Used

Manual code review.

Recommended Mitigation Steps

Consider devising a mechanism to ensure system debt is properly handled. After discussions with the sponsor, it seems that they will be implementing a way to mint INSURE tokens which will be used to cover the shortfall.