code-423n4 / 2023-12-initcapital-findings

3 stars 3 forks source link

repay(), liquidate() and liquidateWLp() receive shares as argument, which may revert if from approval to tx settled blocks have passed #38

Open c4-bot-1 opened 8 months ago

c4-bot-1 commented 8 months ago

Lines of code

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

Vulnerability details

Impact

repay(), liquidate() and liquidateWLp() transactions revert if users approve the exact repay amount they need in the frontend and only after some blocks have passed is the transaction settled. This happens because the interest accrual is by timestamp, so the debt would have increased since the approval, when the transaction settles.

Proof of Concept

A test when repaying debt was carried out in TestInitCore.sol. The timestamp increased just 1 second, but it was enough to make the transaction revert. It may be possible to request a bigger alowance than expected, but this has other implications.

function test_POC_TransferFromFails_DueToDebtAccrual() public {
    uint256 _wbtcAmount = 3e8;
    uint256 _borrowAmount = 1e8;
    address _user = makeAddr("user");
    deal(WBTC, _user, _wbtcAmount);

    uint256 _posId = _createPos(_user, _user, 2);
    uint256 shares_ = _mintPool(_user, address(lendingPools[WBTC]), _wbtcAmount, "");
    vm.startPrank(_user);
    lendingPools[WBTC].transfer(address(positionManager), shares_);
    initCore.collateralize(_posId, address(lendingPools[WBTC]));
    vm.stopPrank();

    uint256 _debtShares = _borrow(_user, _posId, address(lendingPools[WBTC]), _borrowAmount, "");

    uint256 _userDebtBalance = lendingPools[WBTC].debtShareToAmtCurrent(_debtShares);

    vm.prank(_user);
    IERC20(WBTC).approve(address(initCore), _userDebtBalance);

    skip(1); 

    vm.prank(_user);
    vm.expectRevert("ERC20: transfer amount exceeds balance");
    initCore.repay(address(lendingPools[WBTC]), _debtShares, _posId);
}

Tools Used

Vscode, Foundry

Recommended Mitigation Steps

Receive the amount in InitCore as argument instead of the shares on the repay(), liquidate() and liquidateWLp() functions.

Assessed type

Under/Overflow

c4-judge commented 8 months ago

hansfriese marked the issue as primary issue

c4-sponsor commented 8 months ago

fez-init (sponsor) acknowledged

fez-init commented 8 months ago

The issue should be mitigated with the introduction of hooks, where such additional logic of amount to share conversion can be implemented.

c4-judge commented 8 months ago

hansfriese marked the issue as satisfactory

c4-judge commented 8 months ago

hansfriese marked the issue as selected for report