INIT Capital Invitational - said's results

The Liquidity Hook Money Market -- Lend, Borrow, and Access Yield Strategies.

General Information

Platform: Code4rena

Start Date: 15/12/2023

Pot Size: $38,500 USDC

Total HM: 15

Participants: 5

Period: 6 days

Judge: hansfriese

Total Solo HM: 8

Id: 313

League: ETH

INIT Capital

Findings Distribution

Researcher Performance

Rank: 2/5

Findings: 6

Award: $0.00

🌟 Selected for report: 4

🚀 Solo Findings: 4

Findings Information

🌟 Selected for report: sashik_eth

Also found by: said

Labels

bug
3 (High Risk)
satisfactory
duplicate-31

Awards

Data not available

External Links

Lines of code

https://github.com/code-423n4/2023-12-initcapital/blob/main/contracts/core/InitCore.sol#L277 https://github.com/code-423n4/2023-12-initcapital/blob/main/contracts/core/PosManager.sol#L249-L268

Vulnerability details

Impact

Position's owner can steal other users Wlp collateral, as long as it doesn't completely withdraw all the balance of tokenId LP.

Proof of Concept

When users call decollateralizeWLp function from InitCore, as long as Wlp is whitelisted and the mode's decollateralize is not paused, it will trigger POS_MANAGERS.removeCollateralWLpTo, providing all the information provided by users.

https://github.com/code-423n4/2023-12-initcapital/blob/main/contracts/core/InitCore.sol#L264-L279

    function decollateralizeWLp(uint _posId, address _wLp, uint _tokenId, uint _amt, address _to)
        public
        virtual
        onlyAuthorized(_posId)
        ensurePositionHealth(_posId)
        nonReentrant
    {
        IConfig _config = IConfig(config);
        // check mode status
        _require(_config.getModeStatus(_getPosMode(_posId)).canDecollateralize, Errors.DECOLLATERALIZE_PAUSED);
        // check wLp is whitelisted
        _require(_config.whitelistedWLps(_wLp), Errors.TOKEN_NOT_WHITELISTED);
        // update and take _wLp from position to _to
>>>     uint amtDecoll = IPosManager(POS_MANAGER).removeCollateralWLpTo(_posId, _wLp, _tokenId, _amt, _to);
        emit Decollateralize(_wLp, _posId, _to, amtDecoll);
    }

Inside POS_MANAGER.removeCollateralWLpTo, it will trigger _harvest to claim the reward for the posId and unwrap the token from the _wLp contract, sending it to the provided receiver address. However, this function doesn't check if this _posId is the provider of this collateral. As long as it doesn't completely drain the balance of LP, arbitrary posId owners can steal other users' wlp's tokenId balance and rewards provided to the POS_MANAGER.

    function removeCollateralWLpTo(uint _posId, address _wLp, uint _tokenId, uint _amt, address _receiver)
        external
        onlyCore
        returns (uint)
    {
        PosCollInfo storage posCollInfo = __posCollInfos[_posId];
        // NOTE: balanceOfLp should be 1:1 with amt
        uint newWLpAmt = IBaseWrapLp(_wLp).balanceOfLp(_tokenId) - _amt;
>>>     if (newWLpAmt == 0) {
            _require(posCollInfo.ids[_wLp].remove(_tokenId), Errors.NOT_CONTAIN);
            posCollInfo.collCount -= 1;
            if (posCollInfo.ids[_wLp].length() == 0) {
                posCollInfo.wLps.remove(_wLp);
            }
            isCollateralized[_wLp][_tokenId] = false;
        }
        _harvest(_posId, _wLp, _tokenId);
        IBaseWrapLp(_wLp).unwrap(_tokenId, _amt, _receiver);
        return _amt;
    }

Coded PoC :

Add the following test to /2023-12-initcapital/tests/wrapper/TestWLp.sol :

    function testStealWlpOther() public {
        uint amt = 100000000;
        uint withdrawAmt = amt - 1;
        uint alicePosId = _openPositionWithLp(ALICE, amt);
        uint bobPosId = _openPositionWithLp(BOB, 1);
        vm.startPrank(BOB, BOB);
        initCore.decollateralizeWLp(bobPosId, address(mockWLpUniV2), 1, withdrawAmt, BOB);
        vm.stopPrank();
        assertEq(positionManager.getCollWLpAmt(alicePosId, address(mockWLpUniV2), 1), amt - withdrawAmt);
        assertEq(IERC20(lp).balanceOf(BOB), withdrawAmt);
    }

Run the test :

anvil -f https://rpc.mantle.xyz --chain-id 5000
forge test --match-contract TestWLp --match-test testStealWlpOther -vvv

It can be observed that as long as bob own posId, it can steal alice wlp collateral.

Tools Used

Manual review

Add check inside removeCollateralWLpTo to make sure the posId is the holder of wlps's tokenId.

    function removeCollateralWLpTo(uint _posId, address _wLp, uint _tokenId, uint _amt, address _receiver)
        external
        onlyCore
        returns (uint)
    {
        PosCollInfo storage posCollInfo = __posCollInfos[_posId];
+      _require(posCollInfo.ids[_wlp].contains(_tokenId), Errors.NOT_CONTAIN);
        // NOTE: balanceOfLp should be 1:1 with amt
        uint newWLpAmt = IBaseWrapLp(_wLp).balanceOfLp(_tokenId) - _amt;
        if (newWLpAmt == 0) {
            _require(posCollInfo.ids[_wLp].remove(_tokenId), Errors.NOT_CONTAIN);
            posCollInfo.collCount -= 1;
            if (posCollInfo.ids[_wLp].length() == 0) {
                posCollInfo.wLps.remove(_wLp);
            }
            isCollateralized[_wLp][_tokenId] = false;
        }
        _harvest(_posId, _wLp, _tokenId);
        IBaseWrapLp(_wLp).unwrap(_tokenId, _amt, _receiver);
        return _amt;
    }

Assessed type

Access Control

#0 - sashik-eth

2023-12-21T23:40:20Z

Dup od #31

#1 - c4-judge

2023-12-22T02:18:03Z

hansfriese marked the issue as satisfactory

#2 - c4-judge

2023-12-22T02:18:21Z

hansfriese marked the issue as duplicate of #31

Findings Information

🌟 Selected for report: said

Labels

bug
3 (High Risk)
disagree with severity
primary issue
satisfactory
selected for report
sponsor confirmed
H-03

Awards

Data not available

External Links

Lines of code

https://github.com/code-423n4/2023-12-initcapital/blob/main/contracts/hook/MoneyMarketHook.sol#L145-L159

Vulnerability details

Impact

When users construct repay operations via MoneyMarketHook, it doesn't consider the actual debt shares of the position inside the InitCore and PosManager. This could lead to users' tokens getting stuck inside the MoneyMarketHook contract.

Proof of Concept

When users want to repay his positions in MoneyMarketHook, they can provide the parameters inside repayParams, and MoneyMarketHook will construct the operation via _handleRepay function.

https://github.com/code-423n4/2023-12-initcapital/blob/main/contracts/hook/MoneyMarketHook.sol#L145-L159

    function _handleRepay(uint _offset, bytes[] memory _data, uint _initPosId, RepayParams[] memory _params)
        internal
        returns (uint, bytes[] memory)
    {
        for (uint i; i < _params.length; i = i.uinc()) {
            address uToken = ILendingPool(_params[i].pool).underlyingToken();
>>>         uint repayAmt = ILendingPool(_params[i].pool).debtShareToAmtCurrent(_params[i].shares);
            _ensureApprove(uToken, repayAmt);
>>>         IERC20(uToken).safeTransferFrom(msg.sender, address(this), repayAmt);
            _data[_offset] =
                abi.encodeWithSelector(IInitCore.repay.selector, _params[i].pool, _params[i].shares, _initPosId);
            _offset = _offset.uinc();
        }
        return (_offset, _data);
    }

It can be observed that it calculates the repayAmt based on the shares provided by the users and transfers the corresponding amount of tokens from the sender to the hook. However, the actual debt shares of the position can be less than the _params[i].shares provided by users. This means that the actual repay amount of tokens needed could be less than the calculated repayAmt.

https://github.com/code-423n4/2023-12-initcapital/blob/main/contracts/core/InitCore.sol#L530-L551

    function _repay(IConfig _config, uint16 _mode, uint _posId, address _pool, uint _shares)
        internal
        returns (address tokenToRepay, uint amt)
    {
        // check status
        _require(_config.getPoolConfig(_pool).canRepay && _config.getModeStatus(_mode).canRepay, Errors.REPAY_PAUSED);
        // get position debt share
>>>     uint positionDebtShares = IPosManager(POS_MANAGER).getPosDebtShares(_posId, _pool);
>>>     uint sharesToRepay = _shares < positionDebtShares ? _shares : positionDebtShares;
        // get amtToRepay (accrue interest)
>>>     uint amtToRepay = ILendingPool(_pool).debtShareToAmtCurrent(sharesToRepay);
        // take token from msg.sender to pool
        tokenToRepay = ILendingPool(_pool).underlyingToken();
>>>     IERC20(tokenToRepay).safeTransferFrom(msg.sender, _pool, amtToRepay);
        // update debt on the position
        IPosManager(POS_MANAGER).updatePosDebtShares(_posId, _pool, -sharesToRepay.toInt256());
        // call repay on the pool
        amt = ILendingPool(_pool).repay(sharesToRepay);
        // update debt on mode
        IRiskManager(riskManager).updateModeDebtShares(_mode, _pool, -sharesToRepay.toInt256());
        emit Repay(_pool, _posId, msg.sender, _shares, amt);
    }

Consider a scenario where the user's positions are currently liquidatable, and the user wishes to repay all of the position's debt inside the MoneyMarketHook. However, a liquidator front-runs the operation by liquidating the user's position. Now, when the repayment operation executes from MoneyMarketHook, it transfers the repayAmt to the MoneyMarketHook but the amount is not used/fully utilized and becomes stuck inside the contract.

Tools Used

Manual review

Consider to also check the provided shares against the actual debt shares inside the InitCore/PosManager.

Assessed type

Invalid Validation

#0 - c4-judge

2023-12-22T02:22:53Z

hansfriese marked the issue as primary issue

#1 - c4-judge

2023-12-22T02:22:59Z

hansfriese marked the issue as satisfactory

#2 - c4-sponsor

2023-12-27T13:46:12Z

fez-init (sponsor) confirmed

#3 - c4-sponsor

2023-12-27T13:46:15Z

fez-init marked the issue as disagree with severity

#4 - fez-init

2023-12-27T13:47:58Z

The issue should be medium, since the funds cannot be retrieved by someone else. The hook will be upgradeable, so if funds actually get stuck, it is still retrievable.

#5 - hansfriese

2023-12-29T05:43:50Z

I agree that this issue is in the middle of Medium and High. Users might face a temporary lock on their funds, and the hook should be upgraded every time to unlock them. Given the high probability of this scenario occurring, I will keep this issue as a valid High.

#6 - c4-judge

2023-12-29T05:44:04Z

hansfriese marked the issue as selected for report

Findings Information

🌟 Selected for report: said

Labels

bug
2 (Med Risk)
primary issue
satisfactory
selected for report
sponsor confirmed
M-03

Awards

Data not available

External Links

Lines of code

https://github.com/code-423n4/2023-12-initcapital/blob/main/contracts/hook/MoneyMarketHook.sol#L168-L196 https://github.com/code-423n4/2023-12-initcapital/blob/main/contracts/hook/MoneyMarketHook.sol#L76-L80

Vulnerability details

Impact

When param.returnNative is set to true while calling MoneyMarketHook.execute, users expect the returned token from the withdraw operation to be in native form and sent to the caller. However, in the current implementation, this is not considered and could disrupt user expectations.

Proof of Concept

The withdraw functionality inside MoneyMarketHook will process the WithdrawParams provided by users and construct the operations using _handleWithdraw, which consist of calling decollateralize and burnTo in InitCore, providing the parameters accordingly.

https://github.com/code-423n4/2023-12-initcapital/blob/main/contracts/hook/MoneyMarketHook.sol#L168-L196

    function _handleWithdraw(uint _offset, bytes[] memory _data, uint _initPosId, WithdrawParams[] calldata _params)
        internal
        view
        returns (uint, bytes[] memory)
    {
        for (uint i; i < _params.length; i = i.uinc()) {
            // decollateralize to pool
            _data[_offset] = abi.encodeWithSelector(
                IInitCore.decollateralize.selector, _initPosId, _params[i].pool, _params[i].shares, _params[i].pool
            );
            _offset = _offset.uinc();
            // burn collateral to underlying token
            address helper = _params[i].rebaseHelperParams.helper;
            address uTokenReceiver = _params[i].to;
            // if need to unwrap to rebase token
            if (helper != address(0)) {
                address uToken = ILendingPool(_params[i].pool).underlyingToken();
                _require(
                    _params[i].rebaseHelperParams.tokenIn == uToken
                        && IRebaseHelper(_params[i].rebaseHelperParams.helper).YIELD_BEARING_TOKEN() == uToken,
                    Errors.INVALID_TOKEN_IN
                );
                uTokenReceiver = helper;
            }
            _data[_offset] = abi.encodeWithSelector(IInitCore.burnTo.selector, _params[i].pool, uTokenReceiver);
            _offset = _offset.uinc();
        }
        return (_offset, _data);
    }

As it can be observed, _handleWithdraw doesn't check param.returnNative and not adjust the uTokenReceiver token receiver to address(this) when param.returnNative is set to true.

Now, when execute finish perform the multicall and check that _params.returnNative is set to true, it will not work properly as the token is not send to the Hook.

https://github.com/code-423n4/2023-12-initcapital/blob/main/contracts/hook/MoneyMarketHook.sol#L76-L80

    function execute(OperationParams calldata _params)
        external
        payable
        returns (uint posId, uint initPosId, bytes[] memory results)
    {
        // create position if not exist
        if (_params.posId == 0) {
            (posId, initPosId) = createPos(_params.mode, _params.viewer);
        } else {
            // for existing position, only owner can execute
            posId = _params.posId;
            initPosId = initPosIds[msg.sender][posId];
            _require(IERC721(POS_MANAGER).ownerOf(initPosId) == address(this), Errors.NOT_OWNER);
        }
        results = _handleMulticall(initPosId, _params);
        // check slippage
        _require(_params.minHealth_e18 <= IInitCore(CORE).getPosHealthCurrent_e18(initPosId), Errors.SLIPPAGE_CONTROL);
        // unwrap token if needed
        for (uint i; i < _params.withdrawParams.length; i = i.uinc()) {
            address helper = _params.withdrawParams[i].rebaseHelperParams.helper;
            if (helper != address(0)) IRebaseHelper(helper).unwrap(_params.withdrawParams[i].to);
        }
        // return native token
        if (_params.returnNative) {
>>>         IWNative(WNATIVE).withdraw(IERC20(WNATIVE).balanceOf(address(this)));
>>>         (bool success,) = payable(msg.sender).call{value: address(this).balance}('');
            _require(success, Errors.CALL_FAILED);
        }
    }

This could disrupt user expectations. Consider a third-party contract integrated with this hook that can only operate using native balance, doesn't expect, and cannot handle tokens/ERC20. This can cause issues for the integrator.

Tools Used

Manual review.

When constructing withdraw operation, check if _params.returnNative is set to true, and change uTokenReceiver to address(this.

-    function _handleWithdraw(uint _offset, bytes[] memory _data, uint _initPosId, WithdrawParams[] calldata _params)
+    function _handleWithdraw(uint _offset, bytes[] memory _data, uint _initPosId, WithdrawParams[] calldata _params, bool _returnNative)
        internal
        view
        returns (uint, bytes[] memory)
    {
        for (uint i; i < _params.length; i = i.uinc()) {
            // decollateralize to pool
            _data[_offset] = abi.encodeWithSelector(
                IInitCore.decollateralize.selector, _initPosId, _params[i].pool, _params[i].shares, _params[i].pool
            );
            _offset = _offset.uinc();
            // burn collateral to underlying token
            address helper = _params[i].rebaseHelperParams.helper;
            address uTokenReceiver = _params[i].to;
            // if need to unwrap to rebase token
            if (helper != address(0)) {
                address uToken = ILendingPool(_params[i].pool).underlyingToken();
                _require(
                    _params[i].rebaseHelperParams.tokenIn == uToken
                        && IRebaseHelper(_params[i].rebaseHelperParams.helper).YIELD_BEARING_TOKEN() == uToken,
                    Errors.INVALID_TOKEN_IN
                );
                uTokenReceiver = helper;
            }
+            if (_returnNative && uToken == WNATIVE) {
+                uTokenReceiver = address(this);
+            }
            _data[_offset] = abi.encodeWithSelector(IInitCore.burnTo.selector, _params[i].pool, uTokenReceiver);
            _offset = _offset.uinc();
        }
        return (_offset, _data);
    }
    function _handleMulticall(uint _initPosId, OperationParams calldata _params)
        internal
        returns (bytes[] memory results)
    {
        // prepare data for multicall
        // 1. repay (if needed)
        // 2. withdraw (if needed)
        // 3. change position mode (if needed)
        // 4. borrow (if needed)
        // 5. deposit (if needed)
        bool changeMode = _params.mode != 0 && _params.mode != IPosManager(POS_MANAGER).getPosMode(_initPosId);
        bytes[] memory data;
        {
            uint dataLength = _params.repayParams.length + (2 * _params.withdrawParams.length) + (changeMode ? 1 : 0)
                + _params.borrowParams.length + (2 * _params.depositParams.length);
            data = new bytes[](dataLength);
        }
        uint offset;
        // 1. repay
        (offset, data) = _handleRepay(offset, data, _initPosId, _params.repayParams);
        // 2. withdraw
-        (offset, data) = _handleWithdraw(offset, data, _initPosId, _params.withdrawParams);
+        (offset, data) = _handleWithdraw(offset, data, _initPosId, _params.withdrawParams, _params.returnNative);
        // 3. change position mode
        if (changeMode) {
            data[offset] = abi.encodeWithSelector(IInitCore.setPosMode.selector, _initPosId, _params.mode);
            offset = offset.uinc();
        }
        // 4. borrow
        (offset, data) = _handleBorrow(offset, data, _initPosId, _params.borrowParams);
        // 5. deposit
        (offset, data) = _handleDeposit(offset, data, _initPosId, _params.depositParams);
        // execute multicall
        results = IMulticall(CORE).multicall(data);
    }

Assessed type

Context

#0 - JeffCX

2023-12-21T22:27:32Z

duplicate of #2

#1 - c4-judge

2023-12-22T05:35:02Z

hansfriese marked the issue as primary issue

#2 - c4-sponsor

2023-12-27T13:50:26Z

fez-init (sponsor) confirmed

#3 - c4-judge

2023-12-28T16:45:32Z

hansfriese marked the issue as satisfactory

#4 - c4-judge

2023-12-28T16:45:39Z

hansfriese marked the issue as selected for report

Findings Information

🌟 Selected for report: said

Labels

bug
2 (Med Risk)
primary issue
satisfactory
selected for report
sponsor confirmed
M-04

Awards

Data not available

External Links

Lines of code

https://github.com/code-423n4/2023-12-initcapital/blob/main/contracts/core/InitCore.sol#L169-L213

Vulnerability details

Impact

In the scenario where the mode's canRepay status is set to false, positions using that mode cannot be repaid and liquidated. However, users are allowed to change their position's mode to one where the canRepay status is currently set to false. This could be exploited when a position owner observes that their position's health is approaching the liquidation threshold, allowing them to prevent liquidation.

Proof of Concept

It can be observed that when setPosMode is called, it check that newModeStatus.canBorrow and currentModeStatus.canRepay is set to true. However, it doesn't the status of newModeStatus.canRepay flag.

https://github.com/code-423n4/2023-12-initcapital/blob/main/contracts/core/InitCore.sol#L203-L204

    function setPosMode(uint _posId, uint16 _mode)
        public
        virtual
        onlyAuthorized(_posId)
        ensurePositionHealth(_posId)
        nonReentrant
    {
        IConfig _config = IConfig(config);
        // get current collaterals in the position
        (address[] memory pools,, address[] memory wLps, uint[][] memory ids,) =
            IPosManager(POS_MANAGER).getPosCollInfo(_posId);
        uint16 currentMode = _getPosMode(_posId);
        ModeStatus memory currentModeStatus = _config.getModeStatus(currentMode);
        ModeStatus memory newModeStatus = _config.getModeStatus(_mode);
        if (pools.length != 0 || wLps.length != 0) {
            _require(newModeStatus.canCollateralize, Errors.COLLATERALIZE_PAUSED);
            _require(currentModeStatus.canDecollateralize, Errors.DECOLLATERALIZE_PAUSED);
        }
        // check that each position collateral belongs to the _mode
        for (uint i; i < pools.length; i = i.uinc()) {
            _require(_config.isAllowedForCollateral(_mode, pools[i]), Errors.INVALID_MODE);
        }
        for (uint i; i < wLps.length; i = i.uinc()) {
            for (uint j; j < ids[i].length; j = j.uinc()) {
                _require(_config.isAllowedForCollateral(_mode, IBaseWrapLp(wLps[i]).lp(ids[i][j])), Errors.INVALID_MODE);
            }
        }
        // get current debts in the position
        uint[] memory shares;
        (pools, shares) = IPosManager(POS_MANAGER).getPosBorrInfo(_posId);
        IRiskManager _riskManager = IRiskManager(riskManager);
        // check that each position debt belongs to the _mode
        for (uint i; i < pools.length; i = i.uinc()) {
            _require(_config.isAllowedForBorrow(_mode, pools[i]), Errors.INVALID_MODE);
            _require(newModeStatus.canBorrow, Errors.BORROW_PAUSED);
            _require(currentModeStatus.canRepay, Errors.REPAY_PAUSED);
            // update debt on current mode
            _riskManager.updateModeDebtShares(currentMode, pools[i], -shares[i].toInt256());
            // update debt on new mode
            _riskManager.updateModeDebtShares(_mode, pools[i], shares[i].toInt256());
        }
        // update position mode
        IPosManager(POS_MANAGER).updatePosMode(_posId, _mode);
        emit SetPositionMode(_posId, _mode);
    }

As mentioned before, if users see his position's health status is about to reach liquidation threshold and change the mode, this will allow users to prevent their positions from getting liquidated, as both liquidate and liquidateWLp will check the canRepay flag and revert if it's not allowed.

https://github.com/code-423n4/2023-12-initcapital/blob/main/contracts/core/InitCore.sol#L282-L314 https://github.com/code-423n4/2023-12-initcapital/blob/main/contracts/core/InitCore.sol#L317-L353 https://github.com/code-423n4/2023-12-initcapital/blob/main/contracts/core/InitCore.sol#L587-L599

    /// @dev liquidation internal logic
    function _liquidateInternal(uint _posId, address _poolToRepay, uint _repayShares)
        internal
        returns (LiquidateLocalVars memory vars)
    {
        vars.config = IConfig(config);
        vars.mode = _getPosMode(_posId);

        // check position must be unhealthy
        vars.health_e18 = getPosHealthCurrent_e18(_posId);
        _require(vars.health_e18 < ONE_E18, Errors.POSITION_HEALTHY);

>>>     (vars.repayToken, vars.repayAmt) = _repay(vars.config, vars.mode, _posId, _poolToRepay, _repayShares);
    }

https://github.com/code-423n4/2023-12-initcapital/blob/main/contracts/core/InitCore.sol#L530-L551

    function _repay(IConfig _config, uint16 _mode, uint _posId, address _pool, uint _shares)
        internal
        returns (address tokenToRepay, uint amt)
    {
        // check status
>>>     _require(_config.getPoolConfig(_pool).canRepay && _config.getModeStatus(_mode).canRepay, Errors.REPAY_PAUSED);
        // get position debt share
        uint positionDebtShares = IPosManager(POS_MANAGER).getPosDebtShares(_posId, _pool);
        uint sharesToRepay = _shares < positionDebtShares ? _shares : positionDebtShares;
        // get amtToRepay (accrue interest)
        uint amtToRepay = ILendingPool(_pool).debtShareToAmtCurrent(sharesToRepay);
        // take token from msg.sender to pool
        tokenToRepay = ILendingPool(_pool).underlyingToken();
        IERC20(tokenToRepay).safeTransferFrom(msg.sender, _pool, amtToRepay);
        // update debt on the position
        IPosManager(POS_MANAGER).updatePosDebtShares(_posId, _pool, -sharesToRepay.toInt256());
        // call repay on the pool
        amt = ILendingPool(_pool).repay(sharesToRepay);
        // update debt on mode
        IRiskManager(riskManager).updateModeDebtShares(_mode, _pool, -sharesToRepay.toInt256());
        emit Repay(_pool, _posId, msg.sender, _shares, amt);
    }

Tools Used

Manual review

Add a canRepay check status inside setPosMode; if it is paused, revert the change. Besides that, the canRepay and canBorrow checks don't need to be inside the pools check loop.

    function setPosMode(uint _posId, uint16 _mode)
        public
        virtual
        onlyAuthorized(_posId)
        ensurePositionHealth(_posId)
        nonReentrant
    {
        IConfig _config = IConfig(config);
        // get current collaterals in the position
        (address[] memory pools,, address[] memory wLps, uint[][] memory ids,) =
            IPosManager(POS_MANAGER).getPosCollInfo(_posId);
        uint16 currentMode = _getPosMode(_posId);
        ModeStatus memory currentModeStatus = _config.getModeStatus(currentMode);
        ModeStatus memory newModeStatus = _config.getModeStatus(_mode);
        if (pools.length != 0 || wLps.length != 0) {
            _require(newModeStatus.canCollateralize, Errors.COLLATERALIZE_PAUSED);
            _require(currentModeStatus.canDecollateralize, Errors.DECOLLATERALIZE_PAUSED);
        }
        // check that each position collateral belongs to the _mode
        for (uint i; i < pools.length; i = i.uinc()) {
            _require(_config.isAllowedForCollateral(_mode, pools[i]), Errors.INVALID_MODE);
        }
        for (uint i; i < wLps.length; i = i.uinc()) {
            for (uint j; j < ids[i].length; j = j.uinc()) {
                _require(_config.isAllowedForCollateral(_mode, IBaseWrapLp(wLps[i]).lp(ids[i][j])), Errors.INVALID_MODE);
            }
        }
        // get current debts in the position
        uint[] memory shares;
        (pools, shares) = IPosManager(POS_MANAGER).getPosBorrInfo(_posId);
        IRiskManager _riskManager = IRiskManager(riskManager);
        // check that each position debt belongs to the _mode
+      _require(newModeStatus.canBorrow, Errors.BORROW_PAUSED);
+      _require(currentModeStatus.canRepay, Errors.REPAY_PAUSED);
+      _require(newModeStatus.canRepay, Errors.REPAY_PAUSED);
        for (uint i; i < pools.length; i = i.uinc()) {
            _require(_config.isAllowedForBorrow(_mode, pools[i]), Errors.INVALID_MODE);
-            _require(newModeStatus.canBorrow, Errors.BORROW_PAUSED);
-            _require(currentModeStatus.canRepay, Errors.REPAY_PAUSED);
            // update debt on current mode
            _riskManager.updateModeDebtShares(currentMode, pools[i], -shares[i].toInt256());
            // update debt on new mode
            _riskManager.updateModeDebtShares(_mode, pools[i], shares[i].toInt256());
        }
        // update position mode
        IPosManager(POS_MANAGER).updatePosMode(_posId, _mode);
        emit SetPositionMode(_posId, _mode);
    }

Assessed type

Invalid Validation

#0 - c4-judge

2023-12-22T04:43:01Z

hansfriese marked the issue as primary issue

#1 - c4-sponsor

2023-12-27T13:42:41Z

fez-init (sponsor) confirmed

#2 - c4-judge

2023-12-28T16:46:41Z

hansfriese marked the issue as satisfactory

#3 - c4-judge

2023-12-28T16:46:46Z

hansfriese marked the issue as selected for report

Findings Information

🌟 Selected for report: said

Labels

bug
2 (Med Risk)
primary issue
satisfactory
selected for report
sponsor acknowledged
M-05

Awards

Data not available

External Links

Lines of code

https://github.com/code-423n4/2023-12-initcapital/blob/main/contracts/core/PosManager.sol#L125

Vulnerability details

Impact

Admin can pause collateralization for a specific mode to prevent users from providing more collateral either via collateralize or collateralizeWLp. However, due to not properly using internal accounting when tracking wLP collateral, users can still provide more collateral by directly donating tokens to a specific LP tokenId.

Proof of Concept

It can be seen that when canCollateralize of certain mode is paused, collateralizeWLp should be paused.

https://github.com/code-423n4/2023-12-initcapital/blob/main/contracts/core/InitCore.sol#L243-L261

    /// @inheritdoc IInitCore
    function collateralizeWLp(uint _posId, address _wLp, uint _tokenId)
        public
        virtual
        onlyAuthorized(_posId)
        nonReentrant
    {
        IConfig _config = IConfig(config);
        uint16 mode = _getPosMode(_posId);
        // check mode status
>>>     _require(_config.getModeStatus(mode).canCollateralize, Errors.COLLATERALIZE_PAUSED);
        // check if the wLp is whitelisted
        _require(_config.whitelistedWLps(_wLp), Errors.TOKEN_NOT_WHITELISTED);
        // check if the position mode supports _wLp
        _require(_config.isAllowedForCollateral(mode, IBaseWrapLp(_wLp).lp(_tokenId)), Errors.INVALID_MODE);
        // update collateral on the position
        uint amtColl = IPosManager(POS_MANAGER).addCollateralWLp(_posId, _wLp, _tokenId);
        emit CollateralizeWLp(_wLp, _tokenId, _posId, amtColl);
    }

However, when calculating collateral credit, it will calculate based on balance of LP of specific token inside wLP contract.

https://github.com/code-423n4/2023-12-initcapital/blob/main/contracts/core/InitCore.sol#L456

    function getCollateralCreditCurrent_e36(uint _posId) public virtual returns (uint collCredit_e36) {
        address _oracle = oracle;
        IConfig _config = IConfig(config);
        uint16 mode = _getPosMode(_posId);
        // get position collateral
>>>     (address[] memory pools, uint[] memory shares, address[] memory wLps, uint[][] memory ids, uint[][] memory amts)
        = IPosManager(POS_MANAGER).getPosCollInfo(_posId);
        // calculate collateralCredit
        uint collCredit_e54;
        for (uint i; i < pools.length; i = i.uinc()) {
            address token = ILendingPool(pools[i]).underlyingToken();
            uint tokenPrice_e36 = IInitOracle(_oracle).getPrice_e36(token);
            uint tokenValue_e36 = ILendingPool(pools[i]).toAmtCurrent(shares[i]) * tokenPrice_e36;
            TokenFactors memory factors = _config.getTokenFactors(mode, pools[i]);
            collCredit_e54 += tokenValue_e36 * factors.collFactor_e18;
        }
        for (uint i; i < wLps.length; i = i.uinc()) {
            for (uint j; j < ids[i].length; j = j.uinc()) {
                uint wLpPrice_e36 = IBaseWrapLp(wLps[i]).calculatePrice_e36(ids[i][j], _oracle);
                uint wLpValue_e36 = amts[i][j] * wLpPrice_e36;
                TokenFactors memory factors = _config.getTokenFactors(mode, IBaseWrapLp(wLps[i]).lp(ids[i][j]));
                collCredit_e54 += wLpValue_e36 * factors.collFactor_e18;
            }
        }
        collCredit_e36 = collCredit_e54 / ONE_E18;
    }

https://github.com/code-423n4/2023-12-initcapital/blob/main/contracts/core/PosManager.sol#L125

    function getPosCollInfo(uint _posId)
        external
        view
        returns (
            address[] memory pools,
            uint[] memory amts,
            address[] memory wLps,
            uint[][] memory ids,
            uint[][] memory wLpAmts
        )
    {
        PosCollInfo storage posCollInfo = __posCollInfos[_posId];
        pools = posCollInfo.collTokens.values();
        amts = new uint[](pools.length);
        for (uint i; i < pools.length; i = i.uinc()) {
            amts[i] = posCollInfo.collAmts[pools[i]];
        }
        wLps = posCollInfo.wLps.values();
        ids = new uint[][](wLps.length);
        wLpAmts = new uint[][](wLps.length);
        for (uint i; i < wLps.length; i = i.uinc()) {
            ids[i] = posCollInfo.ids[wLps[i]].values();
            wLpAmts[i] = new uint[](ids[i].length);
            for (uint j; j < ids[i].length; j = j.uinc()) {
>>>             wLpAmts[i][j] = IBaseWrapLp(wLps[i]).balanceOfLp(ids[i][j]);
            }
        }
    }

It should be noted that most DEXs (e.g., Uniswap) allow any user to provide liquidity to any other users position. In practice, this bypasses the collateralization paused functionality.

Tools Used

Manual review.

Implement internal accounting for wLP inside the PosManager.

Assessed type

Other

#0 - c4-judge

2023-12-22T04:39:25Z

hansfriese marked the issue as duplicate of #3

#1 - c4-sponsor

2023-12-27T13:37:18Z

fez-init (sponsor) acknowledged

#2 - fez-init

2023-12-27T13:38:17Z

Internal accounting shall be ensured in wLp.

#3 - c4-judge

2023-12-28T01:22:21Z

hansfriese marked the issue as not a duplicate

#4 - c4-judge

2023-12-28T01:22:27Z

hansfriese marked the issue as primary issue

#5 - c4-judge

2023-12-28T01:22:32Z

hansfriese marked the issue as satisfactory

#6 - hansfriese

2023-12-28T16:30:23Z

Medium is appropriate as the admin's action can be bypassed.

#7 - c4-judge

2023-12-28T16:30:44Z

hansfriese marked the issue as selected for report

Findings Information

🌟 Selected for report: rvierdiiev

Also found by: ladboy233, said

Labels

bug
2 (Med Risk)
satisfactory
duplicate-17

Awards

Data not available

External Links

Lines of code

https://github.com/code-423n4/2023-12-initcapital/blob/main/contracts/core/InitCore.sol#L535 https://github.com/code-423n4/2023-12-initcapital/blob/main/contracts/lending_pool/LendingPool.sol#L155-L169

Vulnerability details

Impact

When the canRepay status of pools inside InitCore is paused, users are not allowed to repay their positions when borrowing from the paused pool. However, interests continue to accrue during this pause period, exposing users to potential liquidation risk once the canRepay flag is allowed again.

Proof of Concept

It can be observed that when canRepay flag is set to false inside the config, users are not allowed to repaid his position.

https://github.com/code-423n4/2023-12-initcapital/blob/main/contracts/core/InitCore.sol#L530-L551

    function _repay(IConfig _config, uint16 _mode, uint _posId, address _pool, uint _shares)
        internal
        returns (address tokenToRepay, uint amt)
    {
        // check status
>>>     _require(_config.getPoolConfig(_pool).canRepay && _config.getModeStatus(_mode).canRepay, Errors.REPAY_PAUSED);
        // get position debt share
        uint positionDebtShares = IPosManager(POS_MANAGER).getPosDebtShares(_posId, _pool);
        uint sharesToRepay = _shares < positionDebtShares ? _shares : positionDebtShares;
        // get amtToRepay (accrue interest)
        uint amtToRepay = ILendingPool(_pool).debtShareToAmtCurrent(sharesToRepay);
        // take token from msg.sender to pool
        tokenToRepay = ILendingPool(_pool).underlyingToken();
        IERC20(tokenToRepay).safeTransferFrom(msg.sender, _pool, amtToRepay);
        // update debt on the position
        IPosManager(POS_MANAGER).updatePosDebtShares(_posId, _pool, -sharesToRepay.toInt256());
        // call repay on the pool
        amt = ILendingPool(_pool).repay(sharesToRepay);
        // update debt on mode
        IRiskManager(riskManager).updateModeDebtShares(_mode, _pool, -sharesToRepay.toInt256());
        emit Repay(_pool, _posId, msg.sender, _shares, amt);
    }

However, accrueInterest still can be called and not considering the status of repay inside the InitCore contract.

https://github.com/code-423n4/2023-12-initcapital/blob/main/contracts/lending_pool/LendingPool.sol#L155-L169

    /// @inheritdoc ILendingPool
    function accrueInterest() public {
        uint _lastAccruedTime = lastAccruedTime;
        if (block.timestamp != _lastAccruedTime) {
            uint _totalDebt = totalDebt;
            uint _cash = cash;
            uint borrowRate_e18 = IIRM(irm).getBorrowRate_e18(_cash, _totalDebt);
            uint accruedInterest = (borrowRate_e18 * (block.timestamp - _lastAccruedTime) * _totalDebt) / ONE_E18;
            uint reserve = (accruedInterest * reserveFactor_e18) / ONE_E18;
            if (reserve > 0) {
                _mint(treasury, _toShares(reserve, _cash + _totalDebt + accruedInterest - reserve, totalSupply()));
            }
            totalDebt = _totalDebt + accruedInterest;
            lastAccruedTime = block.timestamp;
        }
    }

This will allow the debt to continue growing while users cannot repay their positions, exposing the risk of getting liquidated once the canRepay status is allowed again by the admin.

Tools Used

Manual review

Inside accrueInterest, check the canRepay status inside InitCore; if it's paused, return early and do not accrue the interest.

Assessed type

Invalid Validation

#0 - c4-judge

2023-12-22T03:09:42Z

hansfriese marked the issue as duplicate of #17

#1 - c4-judge

2023-12-29T06:59:37Z

hansfriese marked the issue as satisfactory

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter