code-423n4 / 2023-10-wildcat-findings

14 stars 10 forks source link

calling `closeMarket` is not possible because of the `onlyController` modifier #620

Closed c4-submissions closed 1 year ago

c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarket.sol#L142 https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketBase.sol#L136-L139

Vulnerability details

Impact

The closeMarket function has the onlyController modifier, which restricts calls to the closeMarket function to the controller only. However, the controller contract does not contain a function that invokes the closeMarket function. As a result, it is currently impossible to close the market by either the borrower or the controller.

Proof of Concept

the function closeMarket have the onlyController modifier which allows calls from the controller only:

    function closeMarket() external onlyController nonReentrant {
        MarketState memory state = _getUpdatedState();
        state.annualInterestBips = 0;
        state.isClosed = true;
        state.reserveRatioBips = 0;
        if (_withdrawalData.unpaidBatches.length() > 0) {
            revert CloseMarketWithUnpaidWithdrawals();
        }
        uint256 currentlyHeld = totalAssets();
        uint256 totalDebts = state.totalDebts();
        if (currentlyHeld < totalDebts) {
            // Transfer remaining debts from borrower
            asset.safeTransferFrom(
                borrower,
                address(this),
                totalDebts - currentlyHeld
            );
        } else if (currentlyHeld > totalDebts) {
            // Transfer excess assets to borrower
            asset.safeTransfer(borrower, currentlyHeld - totalDebts);
        }
        _writeState(state);
        emit MarketClosed(block.timestamp);
    }

the onlyController modifier:

modifier onlyController() {
    if (msg.sender != controller) revert NotController();
    _;
  }

Since the controller contract does not include a function to invoke the closeMarket function, market closure is currently impossible.

Tools Used

manual review

Recommended Mitigation Steps

To address this issue, it is recommended to modify the modifier to use the onlyBorrower modifier instead, or create a function inside the controller contract that invokes the closeMarket function. This adjustment will enable market closure by the borrower or the controller as intended.

Assessed type

Other

c4-pre-sort commented 1 year ago

minhquanym marked the issue as duplicate of #147

c4-judge commented 1 year ago

MarioPoneder changed the severity to 2 (Med Risk)

c4-judge commented 1 year ago

MarioPoneder marked the issue as partial-50

c4-judge commented 1 year ago

MarioPoneder changed the severity to 3 (High Risk)