Fix Review

Numa/contracts/NumaProtocol/NumaOracle.sol

@@ -23,14 +23,14 @@ contract NumaOracle is Ownable2Step, INumaOracle {
     //uint maxSpotOffsetBps = 145;//1.45%
     //uint maxSpotOffsetSqrtBps = 1204;//sqrt(1.45%)
 
-    uint160 maxSpotOffsetPlus1SqrtBps = 10072;
-    uint160 maxSpotOffsetMinus1SqrtBps = 9927;
+    uint160 public maxSpotOffsetPlus1SqrtBps = 10072;
+    uint160 public maxSpotOffsetMinus1SqrtBps = 9927;
 
     nuAssetManager public nuAManager;
 
     event IntervalShort(uint32 _intervalShort);
     event IntervalLong(uint32 _intervalLong);
-    event MaxSpotOffsetBps(uint _maxSpotOffsetBps);
+    event MaxSpotOffset(uint _maxSpotOffset);
     constructor(
         address _token,
         uint32 _intervalShort,
@@ -69,20 +69,18 @@ contract NumaOracle is Ownable2Step, INumaOracle {
 
     /**
      * 
-     * @param _maxSpotOffsetBps offset percentage variable (cf doc)
+     * @param _maxSpotOffset offset percentage variable (cf doc)
      */
-    function setMaxSpotOffsetBps(uint _maxSpotOffsetBps) external onlyOwner {
-        require(_maxSpotOffsetBps < 10000, "percentage must be less than 100");
+    function setMaxSpotOffset(uint _maxSpotOffset) external onlyOwner {
+        require(_maxSpotOffset < 1 ether, "percentage must be less than 100");
 
         maxSpotOffsetPlus1SqrtBps =
-            100 *
-            uint160(Math.sqrt(10000 + _maxSpotOffsetBps));
+            uint160(Math.sqrt(1 ether + _maxSpotOffset))/1e5;
 
         maxSpotOffsetMinus1SqrtBps =
-            100 *
-            uint160(Math.sqrt(10000 - _maxSpotOffsetBps));
+            uint160(Math.sqrt(1 ether - _maxSpotOffset))/1e5;
 
-        emit MaxSpotOffsetBps(_maxSpotOffsetBps);
+        emit MaxSpotOffset(_maxSpotOffset);
     }
 
     /**

Numa/contracts/NumaProtocol/NumaPrinter.sol

@@ -68,10 +68,12 @@ contract NumaPrinter is Pausable, Ownable2Step {
         uint256 _amountReceived
     );
 
+    // sherlock issue 41
+    // CF warning can be bypassed
     modifier notInWarningCF() {
+        _;
         uint currentCF = vaultManager.getGlobalCF();
         require(currentCF > vaultManager.getWarningCF(), "minting forbidden");
-        _;
     }
 
     constructor(
@@ -449,7 +451,7 @@ contract NumaPrinter is Pausable, Ownable2Step {
 
         (uint scaleSynthBurn, , , ) = vaultManager.getSynthScaling();
         // apply scale
-        costWithoutFee = (costWithoutFee * scaleSynthBurn) / BASE_1000;
+        costWithoutFee = (costWithoutFee * scaleSynthBurn) / BASE_SCALE;
         // burn fee
         uint256 amountToBurn = computeFeeAmountIn(
             costWithoutFee,
@@ -493,7 +495,7 @@ contract NumaPrinter is Pausable, Ownable2Step {
         uint256 nuAssetIn = oracle.ethToNuAssetRoundUp(_nuAsset, ethAmount);
         (uint scaleSynthBurn, , , ) = vaultManager.getSynthScaling();
         // apply scale
-        nuAssetIn = (nuAssetIn * BASE_1000) / scaleSynthBurn;
+        nuAssetIn = (nuAssetIn * BASE_SCALE) / scaleSynthBurn;
 
         return (nuAssetIn, amountWithFee - _numaAmount);
     }

Numa/contracts/NumaProtocol/NumaVault.sol

@@ -14,10 +14,11 @@ import "../interfaces/INumaVault.sol";
 
 import "./NumaMinter.sol";
 import "../lending/CNumaToken.sol";
-
+import "../lending/NumaComptroller.sol";
 import "@openzeppelin/contracts_5.0.2/utils/structs/EnumerableSet.sol";
 import "../utils/constants.sol";
 
+
 /// @title Numa vault to mint/burn Numa to lst token
 contract NumaVault is Ownable2Step, ReentrancyGuard, Pausable, INumaVault {
     using EnumerableSet for EnumerableSet.AddressSet;
@@ -172,14 +173,47 @@ contract NumaVault is Ownable2Step, ReentrancyGuard, Pausable, INumaVault {
     }
 
     /**
-     * @dev minimum reth borrow balance needed to allow partial liquidations
+     * @dev minimum lst borrow balance needed to allow partial liquidations
      */
     function setMinBorrowAmountAllowPartialLiquidation(
         uint _minBorrowAmountAllowPartialLiquidation
     ) external onlyOwner {
         minBorrowAmountAllowPartialLiquidation = _minBorrowAmountAllowPartialLiquidation;
     }
 
+    /**
+     * @dev minimum borrow balance needed to allow partial liquidations in numa or in lst
+     */
+    function getMinBorrowAmountAllowPartialLiquidation(address _cBorrowToken) external view returns (uint) {
+        if (_cBorrowToken == address(cNuma) )
+        {
+            // numa borrower
+            // min amount in numa
+            (
+            ,
+            ,
+            uint criticalScaleForNumaPriceAndSellFee,
+            ) = vaultManager.getSynthScaling();
+
+            uint minBorrowAmountAllowPartialLiquidationNuma = vaultManager
+                .tokenToNuma(
+                    minBorrowAmountAllowPartialLiquidation,
+                    last_lsttokenvalueWei,
+                    decimals,
+                    criticalScaleForNumaPriceAndSellFee
+                );
+            return minBorrowAmountAllowPartialLiquidationNuma;
+
+        }
+        else
+        {
+            return minBorrowAmountAllowPartialLiquidation;
+
+        }
+
+
+    }
+
     /**
      * @dev set the IVaultOracle address (used to compute token price in Eth)
      */
@@ -855,11 +889,12 @@ contract NumaVault is Ownable2Step, ReentrancyGuard, Pausable, INumaVault {
 
     function startLiquidation()
         internal
-        returns (uint criticalScaleForNumaPriceAndSellFee)
+        returns (uint criticalScaleForNumaPriceAndSellFee, uint sell_fee)
     {
         (
             ,
             criticalScaleForNumaPriceAndSellFee,
+            sell_fee
 
         ) = updateVaultAndUpdateDebasing();
         // lock numa supply
@@ -975,7 +1010,7 @@ contract NumaVault is Ownable2Step, ReentrancyGuard, Pausable, INumaVault {
             "invalid param"
         );
 
-        uint criticalScaleForNumaPriceAndSellFee = startLiquidation();
+        (uint criticalScaleForNumaPriceAndSellFee,) = startLiquidation();
 
         uint numaAmount = _numaAmount;
 
@@ -985,19 +1020,6 @@ contract NumaVault is Ownable2Step, ReentrancyGuard, Pausable, INumaVault {
         // AUDITV2FIX: handle max liquidations
         if (_numaAmount == type(uint256).max) {
             numaAmount = borrowAmount;
-        } else {
-            // min liquidation amount
-            // convert minimum amount for partial liquidations in numa
-            uint minBorrowAmountAllowPartialLiquidationNuma = vaultManager
-                .tokenToNuma(
-                    minBorrowAmountAllowPartialLiquidation,
-                    last_lsttokenvalueWei,
-                    decimals,
-                    criticalScaleForNumaPriceAndSellFee
-                );
-            uint minAmount = minBorrowAmountAllowPartialLiquidationNuma;
-            if (borrowAmount < minAmount) minAmount = borrowAmount;
-            require(numaAmount >= minAmount, "min liquidation");
         }
 
         if (_flashloan) {
@@ -1015,7 +1037,7 @@ contract NumaVault is Ownable2Step, ReentrancyGuard, Pausable, INumaVault {
 
         // liquidate
         numa.approve(address(cNuma), numaAmount);
-        cNuma.liquidateBorrow(
+        (,uint badDebt) = cNuma.liquidateBorrow(
             _borrower,
             numaAmount,
             CTokenInterface(address(cLstToken))
@@ -1051,9 +1073,15 @@ contract NumaVault is Ownable2Step, ReentrancyGuard, Pausable, INumaVault {
                 decimals,
                 criticalScaleForNumaPriceAndSellFee
             );
+            // sherlock 86 42
+            // for numa borrow, numa <-> lst conversions should use buyPrice
+            maxNumaProfitForLiquidations = (maxNumaProfitForLiquidations * vaultManager.getBuyFee()) / 1 ether;
 
             // cap profit
-            if (numaLiquidatorProfit > maxNumaProfitForLiquidations)
+            // sherlock 101 153
+            // cap only if there is no bad debt, because if we are in bad debt it means this is a partial liquidation which
+            // should not be capped
+            if ((badDebt ==  0) && (numaLiquidatorProfit > maxNumaProfitForLiquidations))
                 numaLiquidatorProfit = maxNumaProfitForLiquidations;
 
             uint numaToSend = numaLiquidatorProfit;
@@ -1078,6 +1106,9 @@ contract NumaVault is Ownable2Step, ReentrancyGuard, Pausable, INumaVault {
                 decimals,
                 criticalScaleForNumaPriceAndSellFee
             );
+            // sherlock 86 42
+            // for numa borrow, numa <-> lst conversions should use buyPrice
+            lstProvidedEstimate = (lstProvidedEstimate * 1 ether) / vaultManager.getBuyFee();
 
             uint lstLiquidatorProfit;
             // we don't revert if liquidation is not profitable because it might be profitable
@@ -1086,8 +1117,13 @@ contract NumaVault is Ownable2Step, ReentrancyGuard, Pausable, INumaVault {
                 lstLiquidatorProfit = receivedlst - lstProvidedEstimate;
             }
 
+
+            // cap profit
+            // sherlock 101 153
+            // cap only if there is no bad debt, because if we are in bad debt it means this is a partial liquidation which
+            // should not be capped
             uint vaultProfit;
-            if (lstLiquidatorProfit > maxLstProfitForLiquidations) {
+            if ((badDebt == 0) && (lstLiquidatorProfit > maxLstProfitForLiquidations)) {
                 vaultProfit = lstLiquidatorProfit - maxLstProfitForLiquidations;
             }
 
@@ -1132,12 +1168,9 @@ contract NumaVault is Ownable2Step, ReentrancyGuard, Pausable, INumaVault {
             lstAmount = borrowAmount;
         }
 
-        uint minAmount = minBorrowAmountAllowPartialLiquidation;
-        if (borrowAmount < minAmount) minAmount = borrowAmount;
 
-        require(lstAmount >= minAmount, "min liquidation");
 
-        uint criticalScaleForNumaPriceAndSellFee = startLiquidation();
+        (uint criticalScaleForNumaPriceAndSellFee,uint sellfee) = startLiquidation();
 
         if (!_flashloan) {
             // user supplied funds
@@ -1151,7 +1184,7 @@ contract NumaVault is Ownable2Step, ReentrancyGuard, Pausable, INumaVault {
 
         // liquidate
         IERC20(lstToken).approve(address(cLstToken), lstAmount);
-        cLstToken.liquidateBorrow(
+        (,uint badDebt) = cLstToken.liquidateBorrow(
             _borrower,
             lstAmount,
             CTokenInterface(address(cNuma))
@@ -1176,7 +1209,10 @@ contract NumaVault is Ownable2Step, ReentrancyGuard, Pausable, INumaVault {
             uint lstLiquidatorProfit = lstReceived - lstAmount;
 
             // cap profit
-            if (lstLiquidatorProfit > maxLstProfitForLiquidations)
+            // sherlock 101 153
+            // cap only if there is no bad debt, because if we are in bad debt it means this is a partial liquidation which
+            // should not be capped
+            if ((badDebt == 0) && (lstLiquidatorProfit > maxLstProfitForLiquidations))
                 lstLiquidatorProfit = maxLstProfitForLiquidations;
 
             uint lstToSend = lstLiquidatorProfit;
@@ -1193,12 +1229,20 @@ contract NumaVault is Ownable2Step, ReentrancyGuard, Pausable, INumaVault {
                 decimals,
                 criticalScaleForNumaPriceAndSellFee
             );
+            // sherlock 86 42
+            // for lst borrow, numa <-> lst conversions should use sellPrice
+            numaProvidedEstimate = (numaProvidedEstimate * 1 ether) / sellfee;
+
             uint maxNumaProfitForLiquidations = vaultManager.tokenToNuma(
                 maxLstProfitForLiquidations,
                 last_lsttokenvalueWei,
                 decimals,
                 criticalScaleForNumaPriceAndSellFee
             );
+            // sherlock 86 42
+            // for lst borrow, numa <-> lst conversions should use sellPrice
+            maxNumaProfitForLiquidations = (maxNumaProfitForLiquidations * 1 ether) / sellfee;
+
 
             uint numaLiquidatorProfit;
             // we don't revert if liquidation is not profitable because it might be profitable
@@ -1208,7 +1252,11 @@ contract NumaVault is Ownable2Step, ReentrancyGuard, Pausable, INumaVault {
             }
 
             uint vaultProfit;
-            if (numaLiquidatorProfit > maxNumaProfitForLiquidations) {
+            // cap profit
+            // sherlock 101 153
+            // cap only if there is no bad debt, because if we are in bad debt it means this is a partial liquidation which
+            // should not be capped
+            if ((badDebt == 0) && (numaLiquidatorProfit > maxNumaProfitForLiquidations)) {
                 vaultProfit =
                     numaLiquidatorProfit -
                     maxNumaProfitForLiquidations;
@@ -1318,4 +1366,14 @@ contract NumaVault is Ownable2Step, ReentrancyGuard, Pausable, INumaVault {
         }
         return (extSize > 0);
     }
+
+
+    function borrowAllowed(address _ctokenAddress) external view returns (bool allowed)
+    {
+        allowed = true;
+        if (_ctokenAddress == address(cNuma))
+        {
+            allowed = vaultManager.numaBorrowAllowed();
+        }
+    }
 }