Wrong implmentation of function BorrowingVault.convertDebtToShares

Title

Affected smart contract

https://github.com/Fujicracy/fuji-v2/blob/1b939ec84af137db430fc2aa1b4c6f15e5254003/packages/protocol/src/vaults/borrowing/BorrowingVault.sol#L175

Description

Function BorrowingVault.borrow() is implemented as follows:

/// url = https://github.com/Fujicracy/fuji-v2/blob/1b939ec84af137db430fc2aa1b4c6f15e5254003/packages/protocol/src/vaults/borrowing/BorrowingVault.sol#L216-L234
    function borrow(uint256 debt, address receiver, address owner) public override returns (uint256) {
    address caller = _msgSender();

    if (debt == 0 || receiver == address(0) || owner == address(0) || debt < minAmount) {
      revert BorrowingVault__borrow_invalidInput();
    }
    if (debt > maxBorrow(owner)) {
      revert BorrowingVault__borrow_moreThanAllowed();
    }

    if (caller != owner) {
      _spendBorrowAllowance(owner, caller, receiver, debt);
    }

    uint256 shares = convertDebtToShares(debt);
    _borrow(caller, receiver, owner, debt, shares);

    return shares;
  }

/// url = https://github.com/Fujicracy/fuji-v2/blob/1b939ec84af137db430fc2aa1b4c6f15e5254003/packages/protocol/src/vaults/borrowing/BorrowingVault.sol#L174-L176
  function convertDebtToShares(uint256 debt) public view override returns (uint256 shares) {
    return _convertDebtToShares(debt, Math.Rounding.Down);
  }

/// url = https://github.com/Fujicracy/fuji-v2/blob/1b939ec84af137db430fc2aa1b4c6f15e5254003/packages/protocol/src/vaults/borrowing/BorrowingVault.sol#L385-L395
  function _convertDebtToShares(
    uint256 debt,
    Math.Rounding rounding
  )
    internal
    view
    returns (uint256 shares)
  {
    uint256 supply = debtSharesSupply;
    return (debt == 0 || supply == 0) ? debt : debt.mulDiv(supply, totalDebt(), rounding);
  }

The function calculates the debtShares corresponding to the amount which the user want to borrow by a round-down calculation. This could lead to the vulnerability that when user borrow many times and try to payback in 1 turn, their transaction can be revert because of the following condition:

function payback(uint256 debt, address owner) public override returns (uint256) {
    ...
    if (debt > convertToDebt(_debtShares[owner])) {
          revert BorrowingVault__payback_moreThanMax();
        }

Attack scenario

We shall assume the state of vault as follows:

debtShareSupply = 100
totalDebt = 200
minAmount = 1

The whole strategy as follows:

Bob call borrow(3, bob_address, bob_address) He will get3 * 100 / 200 = 1 debtShare
Bob call borrow(3, bob_address, bob_address) He will get3 * (100+1) / (200+3) = 1 debtShare
Bob call payback(6, bob_address), function payBack get convertToDebt(2) = 2 * (200 + 6) / (100 + 2) = 4 Then it will be revert in this function since (debt > convertToDebt(_debtShares[owner])) (6 > 4)

Recommendation

Modify function convertDebtToShares() as follows:

  function convertDebtToShares(uint256 debt) public view override returns (uint256 shares) {
    return _convertDebtToShares(debt, Math.Rounding.Up);
  }

Fujicracy / fuji-v2