hats-finance / Tapioca--Lending-Engine--0x5bee198f5b060eecd86b299fdbea6b0c07c728dd

Other
0 stars 0 forks source link

An incorrect fee calculation in the _computeVariableOpeningFee() function in BBBorrow.sol could result in a loss of liquidity for the protocol. #13

Open hats-bug-reporter[bot] opened 4 months ago

hats-bug-reporter[bot] commented 4 months ago

Github username: @MohiniVasisth Twitter username: -- Submission hash (on-chain): 0x58fb80edcb42a90e0a0403f98edf3cd38ad99421b0702bf139f7287b92814e9d Severity: high

Description: Description The current implementation of the borrow function in BBBorrow.sol allows users to borrow any amount without incurring borrowing fees due to an incorrect fee calculation. The fee calculation function _computeVariableOpeningFee() always returns a fee amount of zero.

Attack Scenario\ When a user attempts to borrow funds using the borrow function in BBBorrow.sol, the function calculates the borrowing fee amount. Due to the erroneous fee calculation in _computeVariableOpeningFee(), the fee amount is always zero. This allows users to borrow any amount without incurring any fees, leading to a potential loss of liquidity for the protocol. This flaw essentially enables users to borrow without any cost, compromising the financial stability of the protocol.

Attachments https://github.com/Tapioca-DAO/Tapioca-bar/blob/8920782db6044643fd0c682f58ef37f7e59f99b1/contracts/markets/bigBang/BBBorrow.sol#L37-L52

https://github.com/Tapioca-DAO/Tapioca-bar/blob/8920782db6044643fd0c682f58ef37f7e59f99b1/contracts/markets/bigBang/BBLendingCommon.sol#L81-L105

https://github.com/Tapioca-DAO/Tapioca-bar/blob/8920782db6044643fd0c682f58ef37f7e59f99b1/contracts/markets/bigBang/BBStorage.sol#L48-L52

  1. Proof of Concept (PoC) File

    function borrow(address from, address to, uint256 amount)
    external optionNotPaused(PauseType.Borrow) notSelf(to)
     solvent(from, false) returns (uint256 part, uint256 share)
      {
    if (amount == 0) return (0, 0);
    penrose.reAccrueBigBangMarkets();
    // @audit: The feeAmount returned by             _computeVariableOpeningFee is always zero.
    uint256 feeAmount = _computeVariableOpeningFee(amount);
    uint256 allowanceShare =         _computeAllowanceAmountInAsset(from, exchangeRate, amount         + feeAmount, asset.safeDecimals());
    _allowedBorrow(from, allowanceShare);
    (part, share) = _borrow(from, to, amount, feeAmount);
     }    
    
    function _computeVariableOpeningFee(uint256 amount) internal returns (uint256) {
    if (amount == 0) return 0;
    
    // Get asset <> USDC price (USDO <> USDC)
    (bool updated, uint256 _exchangeRate) =         assetOracle.get(oracleData);
    if (!updated) revert OracleCallFailed();
    // @audit: minMintFee is zero.
    // @audit: minMintFeeStart and maxMintFeeStart both hold     the same value, which is zero.
    // @audit: _exchangeRate is an unsigned int, it will always         hold a value greater than zero, and also the value of         minMintFeeStart and maxMintFeeStart have the same value, zero.     This means the calculation depends on which comes first. The     minMintFee always holds a zero value in the first condition. For     example: if the amount is 6e18, then (6e18 * 0) / 1e5 = 0
    
    if (_exchangeRate >= minMintFeeStart) {
        return (amount * minMintFee) / FEE_PRECISION;
    }    
    if (_exchangeRate <= maxMintFeeStart) {
        return (amount * maxMintFee) / FEE_PRECISION;
    }
    
    uint256 fee = maxMintFee
        - (((_exchangeRate - maxMintFeeStart) * (maxMintFee -     minMintFee)) / (minMintFeeStart - maxMintFeeStart));
    
    if (fee > maxMintFee) return (amount * maxMintFee) /     FEE_PRECISION;
    if (fee < minMintFee) return (amount * minMintFee) /         FEE_PRECISION;  // @audit-issue: minMintFee is zero.
    
    if (fee > 0) {
    return (amount * fee) / FEE_PRECISION;
    }
    return 0;
     }
  2. Revised Code File (Optional)

The calculation of _computeVariableOpeningFee() must be modified as follows:

  function _computeVariableOpeningFee(uint256 amount) internal returns (uint256) {
if (amount == 0) return 0;

// Get asset <> USDC price (USDO <> USDC)
(bool updated, uint256 _exchangeRate) = assetOracle.get(oracleData);
if (!updated) revert OracleCallFailed();
// @audit: Assign appropriate values to minMintFeeStart and maxMintFeeStart and write the condition like this.
if (_exchangeRate >= minMintFeeStart || _exchangeRate <= maxMintFeeStart) {
    return (amount * maxMintFee) / FEE_PRECISION;
}

uint256 fee = maxMintFee
    - (((_exchangeRate - maxMintFeeStart) * (maxMintFee - minMintFee)) / (minMintFeeStart - maxMintFeeStart));

if (fee > maxMintFee) return (amount * maxMintFee) / FEE_PRECISION; // FEE_PRECISION is 1e5.
if (fee < minMintFee) return (amount * minMintFee) / FEE_PRECISION;  // @audit-issue: minMintFee is zero.

if (fee > 0) {
    return (amount * fee) / FEE_PRECISION;
}
return 0;
}

This ensures that a proper fee amount is calculated, preventing users from bypassing borrowing fees and safeguarding the protocol's liquidity.

cryptotechmaker commented 4 months ago

Values are set inside _initCoreStorage

maxMintFeeStart = 975000000000000000; // 0.975 *1e18
minMintFeeStart = 1000000000000000000; // 1*1e18