burn() and borrow() in LendingPool are very likely to revert due to insufficient cash and could be triggered maliciously

[c4-bot-1]

Lines of code

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

Vulnerability details

Impact

Users might not always be able to burn() or borrow() due to being frontrunned by other borrow or burn calls, potentially in a malicious manner so funds don't leave the LendingPool.

Proof of Concept

This attack could happen frequently because the health of a position is given by all the collaterals in different lending pools, so it is likely that users borrow more on a certain pool, making this attack possible.

Borrowing and depositing in the same block doesn't trigger any feed, so attackers could move their funds around to block users' burn/borrow.

burn()

function burn(address _receiver) external onlyCore accrue returns (uint amt) {
    ...
    _require(amt <= _cash, Errors.NOT_ENOUGH_CASH);
    ...
}

borrow()

function borrow(address _receiver, uint _amt) external onlyCore accrue returns (uint shares) {
    _require(_amt <= cash, Errors.NOT_ENOUGH_CASH);
    ...
}

A simple POC was built in Foundry showing the issue:

function test_POC_FrontrunBurn() public {
    uint256 _wbtcAmount = 3e8; // 3 BTC
    uint256 _wethAmount = 3e18; // 3 WETH
    address _user = makeAddr("user");
    address _attacker = makeAddr("attacker");
    _mintPool(_user, WBTC, _wbtcAmount);
    _mintPool(_attacker, WBTC, _wbtcAmount);
    _mintPool(_attacker, WETH, _wethAmount);

    vm.prank(address(initCore));
    lendingPools[WBTC].borrow(_attacker, _wbtcAmount);

    vm.prank(address(initCore));
    vm.expectRevert();
    lendingPools[WBTC].borrow(_user, _wbtcAmount + 1);
}

Tools Used

Vscode, Foundry

Recommended Mitigation Steps

Instead of reverting, the amount could be cropped to the available. This alone would help significantly as the missing liquidity could be very small but still make the transaction revert. Additionally, a scheduling mechanism could be implemented to allow users to withdraw whenever there is liquidity, later. On top of this, a fixed borrow fee could be added to stop upsers from borrowing and depositing atomically, which would decrease the likelihood of attackers moving funds around maliciously.

Assessed type

Under/Overflow

[c4-judge]

hansfriese marked the issue as primary issue

[c4-sponsor]

fez-init marked the issue as disagree with severity

[c4-sponsor]

fez-init marked the issue as agree with severity

[c4-sponsor]

fez-init (sponsor) disputed

[fez-init]

This is the expected behavior.

[hansfriese]

Agree with the sponsor.

[c4-judge]

hansfriese marked the issue as unsatisfactory: Invalid