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
Rank: 2/5
Findings: 6
Award: $0.00
🌟 Selected for report: 4
🚀 Solo Findings: 4
🌟 Selected for report: sashik_eth
Also found by: said
Data not available
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
Position's owner can steal other users Wlp
collateral, as long as it doesn't completely withdraw all the balance of tokenId
LP.
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.
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; }
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
🌟 Selected for report: said
Data not available
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.
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.
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.
Manual review
Consider to also check the provided shares against the actual debt shares inside the InitCore
/PosManager
.
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
🌟 Selected for report: said
Data not available
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
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.
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.
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.
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.
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); }
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
🌟 Selected for report: said
Data not available
https://github.com/code-423n4/2023-12-initcapital/blob/main/contracts/core/InitCore.sol#L169-L213
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.
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); }
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); }
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
🌟 Selected for report: said
Data not available
https://github.com/code-423n4/2023-12-initcapital/blob/main/contracts/core/PosManager.sol#L125
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
.
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.
Manual review.
Implement internal accounting for wLP inside the PosManager
.
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
🌟 Selected for report: rvierdiiev
Data not available
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
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.
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.
/// @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.
Manual review
Inside accrueInterest
, check the canRepay
status inside InitCore
; if it's paused, return early and do not accrue the interest.
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