simon135 - An attacker can borrow with out collateral and gain free tokens through reentrancy

[sherlock-admin]

simon135

high

An attacker can borrow with out collateral and gain free tokens through reentrancy

Summary

An attacker can borrow without collateral and gain free tokens through reentrancy

Vulnerability Detail

An attacker can call deferLiquidityCheck() and it changes their status to LIQUIDITY_CHECK_DEFERRED so when the callback happens call the same function but with a different user causing that other user to be checked and not the other user(attacker). Steps:

1. Attacker calls deferLiquidityCheck(Alice)
2. Attacker reenters with the callback deferLiquidityCheck(bob) and we do nothing so the function would pass
3. Alice is status=LIQUIDITY_CHECK_DEFERRED

4. Alice can call borrow readily and borrow without worrying about collateral checks and cause bad debt in the protocol

   Impact

   Bad debt, borrowing more funds than should be allowed

   Code Snippet

   
       uint8 status = liquidityCheckStatus[user];
   
       if (status == LIQUIDITY_CHECK_NORMAL) {
           (uint256 collateralValue, uint256 debtValue) = _getAccountLiquidity(user);
           require(collateralValue >= debtValue, "insufficient collateral");
       } else if (status == LIQUIDITY_CHECK_DEFERRED) {
           liquidityCheckStatus[user] = LIQUIDITY_CHECK_DIRTY;
       }

```solidity
  require(liquidityCheckStatus[user] == LIQUIDITY_CHECK_NORMAL, "reentry defer liquidity check");
        liquidityCheckStatus[user] = LIQUIDITY_CHECK_DEFERRED;
// @audit-issue here we would reenter with another user(user that is normal and has no collateral should pass)
        DeferLiquidityCheckInterface(msg.sender).onDeferredLiquidityCheck(data);

        uint8 status = liquidityCheckStatus[user];
        liquidityCheckStatus[user] = LIQUIDITY_CHECK_NORMAL;

        if (status == LIQUIDITY_CHECK_DIRTY) {
            _checkAccountLiquidity(user);
        }

Tool used

Manual Review

Recommendation

Make this function based on msg.sender instead

require(user==msg.sender);
// so msg.sender can only borrow for them selfs and flashloan with their own loss and not cause them to borrow more then they should

or non reentrant