From 66ecde79275e91a8fbacbae4bfeb44a177b2fc92 Mon Sep 17 00:00:00 2001 From: Rares Stanciu <1048185+rcstanciu@users.noreply.github.com> Date: Tue, 21 Jan 2025 00:09:33 +0200 Subject: [PATCH] Update README.md --- README.md | 821 +++++++++++++++++++++++++++--------------------------- 1 file changed, 411 insertions(+), 410 deletions(-) diff --git a/README.md b/README.md index c00e03d..cfca375 100644 --- a/README.md +++ b/README.md @@ -1,10 +1,10 @@ -# Issue H-1: CNumaToken.leverageStrategy() can be re-entered, causing all the vault funds to be moved to a cToken, crashing NUMA price. - -Source: https://github.com/sherlock-audit/2024-12-numa-audit-judging/issues/39 - -## Found by -Vidus, jokr, juaan - +# Issue H-1: CNumaToken.leverageStrategy() can be re-entered, causing all the vault funds to be moved to a cToken, crashing NUMA price. + +Source: https://github.com/sherlock-audit/2024-12-numa-audit-judging/issues/39 + +## Found by +Vidus, jokr, juaan + ### Summary [`CNumaToken.leverageStrategy()`](https://github.com/sherlock-audit/2024-12-numa-audit/blob/ae1d7781efb4cb2c3a40c642887ddadeecabb97d/Numa/contracts/lending/CNumaToken.sol#L141) has a `_collateral` token parameter which allows an attacker to pass in a custom `_collateral` token, which acts as a wrapper around the actual collateral token, while also receiving callbacks to enable reentrancy. @@ -185,40 +185,41 @@ contract FakeCollateral { } ``` + ### Mitigation -_No response_ - -# Issue H-2: Vault is vulnerable to inflation attack which can cause complete loss of user funds - -Source: https://github.com/sherlock-audit/2024-12-numa-audit-judging/issues/253 - -## Found by -0xlucky, Abhan1041, AestheticBhai, KupiaSec, Oblivionis, ZZhelev, blutorque, jokr, juaan, novaman33, smbv-1923 - -### Summary - -Attacker can attack the first depositors in the vault and can steal all users funds. this attack is also famously known has first deposit bug too. while doing this attack , there is no loss of attacker funds, but there is complete loss of user funds. he can complete this attack by front running and then backrunning , means sandwiching user funds. this problem takes place , due to improper use of exchange rate when total supply is 0. - -### Root Cause - +_No response_ + +# Issue H-2: Vault is vulnerable to inflation attack which can cause complete loss of user funds + +Source: https://github.com/sherlock-audit/2024-12-numa-audit-judging/issues/253 + +## Found by +0xlucky, Abhan1041, AestheticBhai, KupiaSec, Oblivionis, ZZhelev, blutorque, jokr, juaan, novaman33, smbv-1923 + +### Summary + +Attacker can attack the first depositors in the vault and can steal all users funds. this attack is also famously known has first deposit bug too. while doing this attack , there is no loss of attacker funds, but there is complete loss of user funds. he can complete this attack by front running and then backrunning , means sandwiching user funds. this problem takes place , due to improper use of exchange rate when total supply is 0. + +### Root Cause + https://github.com/sherlock-audit/2024-12-numa-audit/blob/main/Numa/contracts/lending/CErc20.sol#L60C1-L63C6 https://github.com/sherlock-audit/2024-12-numa-audit/blob/main/Numa/contracts/lending/CToken.sol#L510C1-L515C1 -here root cause is total cash in formula is being calculated with balanceOF(address(this)), which can donated direclty too. and price can be inflated - -### Internal pre-conditions - -_No response_ - -### External pre-conditions - -In this attack , attacker should be the first depositor, and while deploying on ethereum, he can frontrun and can be the first depositor. - -### Attack Path - +here root cause is total cash in formula is being calculated with balanceOF(address(this)), which can donated direclty too. and price can be inflated + +### Internal pre-conditions + +_No response_ + +### External pre-conditions + +In this attack , attacker should be the first depositor, and while deploying on ethereum, he can frontrun and can be the first depositor. + +### Attack Path + while depositing when , total supply of minting token is 0, attacker will deposit , 1 wei of asset and will be minted with 1 wei of share. so now total supply would be 1 wei. @@ -229,27 +230,27 @@ he can now, redeem his 1 wei of share, and in return he can get all amount of as https://github.com/sherlock-audit/2024-12-numa-audit/blob/main/Numa/contracts/lending/CToken.sol#L374C4-L401C6 -in link we can see the formula which is being used for exchangerate. - -### Impact - -this can lead user loss of funds, and attacker will get benefited from this. - -### PoC - -_No response_ - -### Mitigation - -1000 wei ( some amount) shares should be burned while first depositing. this is done by uniswap too - -# Issue M-1: Debasing/rebasing periods can be decreased by 50% by a malicious actor - -Source: https://github.com/sherlock-audit/2024-12-numa-audit-judging/issues/18 - -## Found by -000000 - +in link we can see the formula which is being used for exchangerate. + +### Impact + +this can lead user loss of funds, and attacker will get benefited from this. + +### PoC + +_No response_ + +### Mitigation + +1000 wei ( some amount) shares should be burned while first depositing. this is done by uniswap too + +# Issue M-1: Debasing/rebasing periods can be decreased by 50% by a malicious actor + +Source: https://github.com/sherlock-audit/2024-12-numa-audit-judging/issues/18 + +## Found by +000000 + ### Vulnerability Detail Upon debasing, we have the following calculation: @@ -274,15 +275,15 @@ It resets the time to the last block time before the update. However, this can s Synthetics will derate slower than intended, which will keep the CF low as users are not incentivized to sell them ### Mitigation -Refactor the formula - -# Issue M-2: OracleUtils.ethLeftSide() is not correct for some tokens, leading to incorrect nuAsset pricing - -Source: https://github.com/sherlock-audit/2024-12-numa-audit-judging/issues/38 - -## Found by -juaan - +Refactor the formula + +# Issue M-2: OracleUtils.ethLeftSide() is not correct for some tokens, leading to incorrect nuAsset pricing + +Source: https://github.com/sherlock-audit/2024-12-numa-audit-judging/issues/38 + +## Found by +juaan + ### Summary [`OracleUtils::ethLeftSide()`](https://github.com/sherlock-audit/2024-12-numa-audit/blob/ae1d7781efb4cb2c3a40c642887ddadeecabb97d/Numa/contracts/libraries/OracleUtils.sol#L261-L270) is used to check whether ETH is in the numerator or the denominator of the price feed, in order to correctly price the paired asset. @@ -329,15 +330,15 @@ _No response_ ### Mitigation Check the first 4 bytes of the pricefeed's description string, and return true only if the first 4 bytes are the same as “ETH/” -This ensures that the function is always correct - -# Issue M-3: CF minimum can be bypassed when minting nuAssets - -Source: https://github.com/sherlock-audit/2024-12-numa-audit-judging/issues/41 - -## Found by -juaan - +This ensures that the function is always correct + +# Issue M-3: CF minimum can be bypassed when minting nuAssets + +Source: https://github.com/sherlock-audit/2024-12-numa-audit-judging/issues/41 + +## Found by +juaan + ### Summary CF is checked before minting nuAssets (instead of after), allowing CF to be massively decreased past the warning. This allows assets to be minted even past the critical CF of 110%, breaking the invariant stated in the README. @@ -456,67 +457,67 @@ modifier notInWarningCF() { } ``` -This ensures that the CF is checked after minting the nuAssets, so it can't be bypassed. - -## Discussion - -**tibthecat** - +This ensures that the CF is checked after minting the nuAssets, so it can't be bypassed. + +## Discussion + +**tibthecat** + Not sure about that one. Needs to check with team. The goal is to block minting when CF has already reached WarningCF, not to prevent from reaching this warningCF. -Will check with team, but my current opinion is that it's invalid. - - - -# Issue M-4: No RWAs have a chainlink feed in ETH, so RWAs cannot be minted as nuAssets - -Source: https://github.com/sherlock-audit/2024-12-numa-audit-judging/issues/53 - -## Found by -juaan - -### Summary - +Will check with team, but my current opinion is that it's invalid. + + + +# Issue M-4: No RWAs have a chainlink feed in ETH, so RWAs cannot be minted as nuAssets + +Source: https://github.com/sherlock-audit/2024-12-numa-audit-judging/issues/53 + +## Found by +juaan + +### Summary + A key intention of the protocol is to allow RWAs with chainlink feeds to be represented with synthetic nuAssets. The issue is that the protocol only works with chainlink feeds where the asset is priced with ETH. -All the RWAs like gold, oil, etc on chainlink are only available with USD pairs, not ETH. - -### Root Cause - -The protocol only works with chainlink feeds where the asset is priced with ETH, but all the RWAs like gold, crude oil, etc on chainlink are only available with USD pairs, not ETH. - -### Internal pre-conditions - -_No response_ - -### External pre-conditions - -_No response_ - -### Attack Path - -_No response_ - -### Impact - -Protocol in it's current form cannot work with RWAs. - -### PoC - -_No response_ - -### Mitigation - -Have a way to convert the ASSET/USD pairs into ASSET/ETH using the ETH/USD price feed. - -# Issue M-5: Deprecated markets allow profitable exploitation of bad debt liquidations - -Source: https://github.com/sherlock-audit/2024-12-numa-audit-judging/issues/67 - -## Found by -t0x1c - +All the RWAs like gold, oil, etc on chainlink are only available with USD pairs, not ETH. + +### Root Cause + +The protocol only works with chainlink feeds where the asset is priced with ETH, but all the RWAs like gold, crude oil, etc on chainlink are only available with USD pairs, not ETH. + +### Internal pre-conditions + +_No response_ + +### External pre-conditions + +_No response_ + +### Attack Path + +_No response_ + +### Impact + +Protocol in it's current form cannot work with RWAs. + +### PoC + +_No response_ + +### Mitigation + +Have a way to convert the ASSET/USD pairs into ASSET/ETH using the ETH/USD price feed. + +# Issue M-5: Deprecated markets allow profitable exploitation of bad debt liquidations + +Source: https://github.com/sherlock-audit/2024-12-numa-audit-judging/issues/67 + +## Found by +t0x1c + ## Summary When markets are deprecated in the protocol, bad debt positions can be liquidated using regular liquidation functions instead of the dedicated bad debt liquidation path. This bypasses important safeguards and allows liquidators to extract a profit, worsening the protocol's position even further. @@ -757,24 +758,24 @@ Likelihood: Low/Medium. Requires an event where a market has been deprecated. Overall Severity: Medium ## Mitigation -Add a check that even for deprecated markets, regular (shortfall) liquidation path is not allowed for badDebt positions. - -## Discussion - -**tibthecat** - +Add a check that even for deprecated markets, regular (shortfall) liquidation path is not allowed for badDebt positions. + +## Discussion + +**tibthecat** + As discussed in the dashboard, I think it's LOW because protocol could liquidate any bad debt position before deprecating. -But still, it's better for us to fix that and use the liquidatebaddebt path even when market is deprecated. - - - -# Issue M-6: Incorrect liquidation mechanics either causes revert on liquidation due to insufficient seizeTokens or causes transition into bad debt - -Source: https://github.com/sherlock-audit/2024-12-numa-audit-judging/issues/101 - -## Found by -t0x1c - +But still, it's better for us to fix that and use the liquidatebaddebt path even when market is deprecated. + + + +# Issue M-6: Incorrect liquidation mechanics either causes revert on liquidation due to insufficient seizeTokens or causes transition into bad debt + +Source: https://github.com/sherlock-audit/2024-12-numa-audit-judging/issues/101 + +## Found by +t0x1c + ## Summary The protocol's liquidation mechanics are fundamentally flawed in two distinct ways that emerge when performing liquidations on positions. Either: @@ -1041,12 +1042,12 @@ Add the following 2 checks: 2. If a full liquidation attempt ( possible when `borrowBalance < minBorrowAmountAllowPartialLiquidation` ) would result in `seizeTokens` to be greater than the cToken collateral balance of the borrower, then the liquidator should still be allowed to go ahead and be awarded all the available cTokens in borrower's balance. -This possibility of a reduced liquidation incentive should be properly documented so that liquidators know the risk in advance. - -## Discussion - -**tibthecat** - +This possibility of a reduced liquidation incentive should be properly documented so that liquidators know the risk in advance. + +## Discussion + +**tibthecat** + I think it works like compound and as intended for the bad debt part which can be profitable only if there is way to swap collateral at profit on some LP. About that comment in the dashboard: @@ -1059,17 +1060,17 @@ For part 1, if I'm understanding correctly, due to the existance of minBorrowAmo Changing this to medium severity."\_ -I disagree because our goal IS that liquidator liquidate the maximum possible. Because of our maxProfitPerliquidation, we don't want liquidators to split their liquidations. We added that parameter (minBorrowAmountAllowPartialLiquidation) in case there would not be enough liquidity on the chain to perform the liquidation. - - - -# Issue M-7: leverageStrategy will revert due users interest rate accrual - -Source: https://github.com/sherlock-audit/2024-12-numa-audit-judging/issues/120 - -## Found by -KupiaSec, jokr - +I disagree because our goal IS that liquidator liquidate the maximum possible. Because of our maxProfitPerliquidation, we don't want liquidators to split their liquidations. We added that parameter (minBorrowAmountAllowPartialLiquidation) in case there would not be enough liquidity on the chain to perform the liquidation. + + + +# Issue M-7: leverageStrategy will revert due users interest rate accrual + +Source: https://github.com/sherlock-audit/2024-12-numa-audit-judging/issues/120 + +## Found by +KupiaSec, jokr + ### Summary In the `CNumaToken.leverageStrategy()` function, after borrowing from the market using the `borrowInternalNoTransfer` function, a check is performed to ensure that the user's borrow amount changes only by `borrowAmount` using a `require` statement. However, this check will fail because the user's principal borrow amount will increase by more than `borrowAmount` due to the interest accrued on the user's borrow position. @@ -1151,22 +1152,22 @@ Instead of directly fetching the user's previous borrow amount from the state us - uint accountBorrowBefore = accountBorrows[msg.sender].principal; + uint accountBorrowBefore = borrowBalanceStored(msg.sender); ``` - - -# Issue M-8: User will Received Less Amount Of Numa - -Source: https://github.com/sherlock-audit/2024-12-numa-audit-judging/issues/161 - -## Found by -Nave765, onthehunt - -### Summary - + + +# Issue M-8: User will Received Less Amount Of Numa + +Source: https://github.com/sherlock-audit/2024-12-numa-audit-judging/issues/161 + +## Found by +Nave765, onthehunt + +### Summary + User will lose some Numa due to precision loss whenever user calls `NumaPrinter::burnAssetInputToNuma`. - - -### Root Cause - + + +### Root Cause + `BASE_1000` is being used in collateral factor calculation, criticalScaleForNumaPriceAndSellFee, and criticalDebaseFactor within `VaultManager::getSynthScaling` function. https://github.com/sherlock-audit/2024-12-numa-audit/blob/main/Numa/contracts/NumaProtocol/VaultManager.sol#L550-L558 @@ -1223,41 +1224,41 @@ Assuming user try to deposit 500k worth of NUMA in nuAsset, due to low amount of return (costWithoutFee - amountToBurn, amountToBurn); } } -``` - -### Internal pre-conditions - -_No response_ - -### External pre-conditions - -_No response_ - -### Attack Path - -No Attack Required - -### Impact - +``` + +### Internal pre-conditions + +_No response_ + +### External pre-conditions + +_No response_ + +### Attack Path + +No Attack Required + +### Impact + User will get less amount when they try to exchange NuAsset to NUMA. - - -### PoC - -_No response_ - -### Mitigation - + + +### PoC + +_No response_ + +### Mitigation + Consider changing the precision to 1e18 or bigger than 1000. - - -# Issue M-9: Before transferring `CToken`, the `accrueInterest()` function should be called first. - -Source: https://github.com/sherlock-audit/2024-12-numa-audit-judging/issues/168 - -## Found by -KupiaSec - + + +# Issue M-9: Before transferring `CToken`, the `accrueInterest()` function should be called first. + +Source: https://github.com/sherlock-audit/2024-12-numa-audit-judging/issues/168 + +## Found by +KupiaSec + ### Summary Before transferring `CToken`, the `NumaComptroller.transferAllowed()` function is called first to prevent any imbalance between the user's collateral and debt value. @@ -1327,30 +1328,30 @@ And illegitimate transfers can lead to an imbalance between the sender's collate ### Mitigation -Invoke `accrueInterest()` before the transfer. - -## Discussion - -**tibthecat** - -This is coming from compound V2 fork. Is compound V2 vulnerable to that too? - - - -# Issue M-10: Precision loss in setMaxSpotOffsetBps function leads to Incorrect Numa Prices - -Source: https://github.com/sherlock-audit/2024-12-numa-audit-judging/issues/175 - -## Found by -jokr - -### Summary - +Invoke `accrueInterest()` before the transfer. + +## Discussion + +**tibthecat** + +This is coming from compound V2 fork. Is compound V2 vulnerable to that too? + + + +# Issue M-10: Precision loss in setMaxSpotOffsetBps function leads to Incorrect Numa Prices + +Source: https://github.com/sherlock-audit/2024-12-numa-audit-judging/issues/175 + +## Found by +jokr + +### Summary + Due to precision loss in `NumaOracle.setMaxSpotOffsetBps()`, the spot price is modified (increased or decreased) by incorrect percentages, resulting in incorrect prices. - - -### Root Cause - + + +### Root Cause + Due to precision loss during the calculation of `maxSpotOffsetPlus1SqrtBps` and `maxSpotOffsetMinus1SqrtBps`, the spot price from the LP increases or decreases by more than the desired percentage. @@ -1376,26 +1377,26 @@ For example, let’s say `_maxSpotOffsetBps = 145` (1.45% as mentioned in the do In this case, `maxSpotOffsetPlus1SqrtBps` should ideally be `10072`. However, since `Math.sqrt(10000 + 145) = 100.72`, it will be rounded down to `100` in Solidity. As a result, `maxSpotOffsetPlus1SqrtBps` will be set to `10000` instead of `10072`. This means that, instead of increasing the spot price by 1.45%, the spot price will remain unchanged. Similarly, for `maxSpotOffsetMinus1SqrtBps`, it will be set to `9900` instead of `9927`. This results in the spot price decreasing by 1.99% instead of the intended 1.45%. - - -### Internal pre-conditions - -_No response_ - -### External pre-conditions - -_No response_ - -### Attack Path - -_No response_ - -### Impact - -Incorrect prices result in conversions between Numa and nuAssets occurring at inaccurate rates. - -### PoC - + + +### Internal pre-conditions + +_No response_ + +### External pre-conditions + +_No response_ + +### Attack Path + +_No response_ + +### Impact + +Incorrect prices result in conversions between Numa and nuAssets occurring at inaccurate rates. + +### PoC + ```solidity Initial values: @@ -1421,25 +1422,25 @@ Decreased price: 3302.642438949233 Increased price: 3369.6994581667514 Percentage change (decrease): -1.99% Percentage change (increase): 0.00% -``` - -### Mitigation - -Increase the precision of `_maxSpotOffsetBps` to avoid precision losses. - -# Issue M-11: No slippage check for leverageStrategy function - -Source: https://github.com/sherlock-audit/2024-12-numa-audit-judging/issues/182 - -## Found by -jokr - -### Summary - -When a user opens a leveraged position using the CNumaToken.leverageStrategy() function, there is no slippage protection to limit the price at which tokens are bought by the strategy contract. As a result, if the tokens are purchased at an unfavorable price, the user may incur more debt than expected. Additionally, the function lacks a deadline parameter to ensure that the transaction is executed within a specified timeframe. Without this parameter, if the transaction is delayed, the user has no control over the price at which the tokens are purchased, increasing their risk. - -### Root Cause - +``` + +### Mitigation + +Increase the precision of `_maxSpotOffsetBps` to avoid precision losses. + +# Issue M-11: No slippage check for leverageStrategy function + +Source: https://github.com/sherlock-audit/2024-12-numa-audit-judging/issues/182 + +## Found by +jokr + +### Summary + +When a user opens a leveraged position using the CNumaToken.leverageStrategy() function, there is no slippage protection to limit the price at which tokens are bought by the strategy contract. As a result, if the tokens are purchased at an unfavorable price, the user may incur more debt than expected. Additionally, the function lacks a deadline parameter to ensure that the transaction is executed within a specified timeframe. Without this parameter, if the transaction is delayed, the user has no control over the price at which the tokens are purchased, increasing their risk. + +### Root Cause + In [CNumaToken.sol:193](https://github.com/sherlock-audit/2024-12-numa-audit/blob/main/Numa/contracts/lending/CNumaToken.sol#L193), there is no slippage check for the `borrowAmount`. As a result, if the tokens are purchased at a worse price than expected, the user may incur more debt than intended. ```solidity @@ -1480,18 +1481,18 @@ In [CNumaToken.sol:193](https://github.com/sherlock-audit/2024-12-numa-audit/blo } ``` -The `closeLeverageStrategy` function also lacks slippage protection. - -### Internal pre-conditions - -_No response_ - -### External pre-conditions - -_No response_ - -### Attack Path - +The `closeLeverageStrategy` function also lacks slippage protection. + +### Internal pre-conditions + +_No response_ + +### External pre-conditions + +_No response_ + +### Attack Path + Let's assume 1 Numa = 0.1 rETH. 1. The user calls `leverageStrategy(10, 40, cNuma)` to open a 5x leveraged position in Numa tokens. @@ -1500,18 +1501,18 @@ Let's assume 1 Numa = 0.1 rETH. 4. The cReth contract will then borrow rETH against the provided collateral (50 Numa in step 3) and exchange it for Numa to repay the 40 Numa flash loan taken in step 2. 5. The user expects to incur 4 rETH of debt (40 Numa * 0.1 rETH per Numa). However, there is no control over the price at which the swap occurs. If the swap happens at a worse price, for example, 1 Numa = 0.15 rETH, the user will incur 6 rETH of debt instead of the expected 4 rETH. This price discrepancy results in a loss for the user. - - -### Impact - -User will incur more debt that expected due to lack of slippage check. - -### PoC - -_No response_ - -### Mitigation - + + +### Impact + +User will incur more debt that expected due to lack of slippage check. + +### PoC + +_No response_ + +### Mitigation + Add slippage protection for `borrowAmount` in `leverageStrategy` function. @@ -1553,17 +1554,17 @@ Add slippage protection for `borrowAmount` in `leverageStrategy` function. } ``` -Also add slippage protection for `closeLeverageStrategy` function. - -# Issue M-12: Numa tokens fee on transfer can be bypassed - -Source: https://github.com/sherlock-audit/2024-12-numa-audit-judging/issues/184 - -The protocol has acknowledged this issue. - -## Found by -jokr - +Also add slippage protection for `closeLeverageStrategy` function. + +# Issue M-12: Numa tokens fee on transfer can be bypassed + +Source: https://github.com/sherlock-audit/2024-12-numa-audit-judging/issues/184 + +The protocol has acknowledged this issue. + +## Found by +jokr + ### Summary The Numa token is designed in a way that if users want to transfer Numa tokens to a specific address present in `isIncludedInFees`, a fee will be applied. Additionally, if the `spender` address is in the `wlSpenders` list, no fee is charged @@ -1607,32 +1608,32 @@ _No response_ ### Mitigation -Change fee on transfer implementation of Numa token - -## Discussion - -**tibthecat** - -Irrelevant as current deployed Numa token does not have "on-transfer fee" anymore. And numa.sol should not have been in the audit scope. - -**c-plus-plus-equals-c-plus-one** - +Change fee on transfer implementation of Numa token + +## Discussion + +**tibthecat** + +Irrelevant as current deployed Numa token does not have "on-transfer fee" anymore. And numa.sol should not have been in the audit scope. + +**c-plus-plus-equals-c-plus-one** + > Irrelevant as current deployed Numa token does not have "on-transfer fee" anymore. And numa.sol should not have been in the audit scope. ![image](https://github.com/user-attachments/assets/44e294a9-01b5-4dc7-b037-80ad20e5d504) - - - - -# Issue M-13: Buy fee PID is updated with wrong amounts leading to unexpected fee growth - -Source: https://github.com/sherlock-audit/2024-12-numa-audit-judging/issues/199 - -## Found by -Afriaudit, OpaBatyo, zarkk01 - -### Summary - + + + + +# Issue M-13: Buy fee PID is updated with wrong amounts leading to unexpected fee growth + +Source: https://github.com/sherlock-audit/2024-12-numa-audit-judging/issues/199 + +## Found by +Afriaudit, OpaBatyo, zarkk01 + +### Summary + ## Summary `updateBuyFeePID` is invoked with Numa amount before fee, instead of the actual numa that was minted ## Description @@ -1678,56 +1679,56 @@ Further proof validating this issue can be seen in `sell` where `updateBuyFeePID vaultManager.updateBuyFeePID(_numaAmount, false); -``` - -### Root Cause - -- In [`NumaVault.buyNoMax`](https://github.com/sherlock-audit/2024-12-numa-audit/blob/ae1d7781efb4cb2c3a40c642887ddadeecabb97d/Numa/contracts/NumaProtocol/NumaVault.sol#L527), `updateBuyFeePID` is invoked with wrong, inflated amount instead of the actual numa that was minted - -### Internal pre-conditions - -none - -### External pre-conditions - -none - -### Attack Path - -none, wrong logic - -### Impact - +``` + +### Root Cause + +- In [`NumaVault.buyNoMax`](https://github.com/sherlock-audit/2024-12-numa-audit/blob/ae1d7781efb4cb2c3a40c642887ddadeecabb97d/Numa/contracts/NumaProtocol/NumaVault.sol#L527), `updateBuyFeePID` is invoked with wrong, inflated amount instead of the actual numa that was minted + +### Internal pre-conditions + +none + +### External pre-conditions + +none + +### Attack Path + +none, wrong logic + +### Impact + - wrong fee calculation -- loss for users - -### PoC - -_No response_ - -### Mitigation - -Invoke `updateBuyFeePID` with the actual minted amount in `NumaVault.buyNoMax` - -## Discussion - -**tibthecat** - -I don't think it makes a big difference. But I will check with team. - - - -# Issue M-14: Vaults can be purposefully bricked by leaving small amounts of rETH - -Source: https://github.com/sherlock-audit/2024-12-numa-audit-judging/issues/220 - -The protocol has acknowledged this issue. - -## Found by -OpaBatyo - -### Summary - +- loss for users + +### PoC + +_No response_ + +### Mitigation + +Invoke `updateBuyFeePID` with the actual minted amount in `NumaVault.buyNoMax` + +## Discussion + +**tibthecat** + +I don't think it makes a big difference. But I will check with team. + + + +# Issue M-14: Vaults can be purposefully bricked by leaving small amounts of rETH + +Source: https://github.com/sherlock-audit/2024-12-numa-audit-judging/issues/220 + +The protocol has acknowledged this issue. + +## Found by +OpaBatyo + +### Summary + ## Summary Any numa holder can sell their numa in any vault, regardless where the tokens came from, allowing Numa holders to have preferential vaults and brick smaller ones by leaving dust amounts of rETH. ## Description @@ -1743,38 +1744,38 @@ Vault holds 1 rETH (1e18 wei) currently valued at 3700 USD. Numa holder comes with their liquidity from another vault and burns it, leaving 0.0001 rETH (1e14 wei) or around 0.37 USD Any further deposits to this vault can be at most a fraction of 0.37 USD, even if `max_percent = 1000` -Users won't be able to deposit a significant amount in this vault, effectively being pushed away from it or forced to interact with a bigger one. Large liquidity providers can collude and perform such attacks to increase their rewards and interest accrued in a preferential vault. - -### Root Cause - +Users won't be able to deposit a significant amount in this vault, effectively being pushed away from it or forced to interact with a bigger one. Large liquidity providers can collude and perform such attacks to increase their rewards and interest accrued in a preferential vault. + +### Root Cause + - [`NumaVault.buy`](https://github.com/sherlock-audit/2024-12-numa-audit/blob/ae1d7781efb4cb2c3a40c642887ddadeecabb97d/Numa/contracts/NumaProtocol/NumaVault.sol#L441) allows to only deposit, at most, a fraction of the current liquidity -- There are no checks in `NumaVault.sell` whether or not the burnt Numa was minted in the same vault - -### Internal pre-conditions - -None - -### External pre-conditions - -none - -### Attack Path - +- There are no checks in `NumaVault.sell` whether or not the burnt Numa was minted in the same vault + +### Internal pre-conditions + +None + +### External pre-conditions + +none + +### Attack Path + 1. Whale user mints large amounts of Numa in a big vault 2. User sells it in smaller vaults, leaving their liquidity at negligible values 3. Other protocol users can't make a significant deposit in the other vault so they opt for the bigger one -4. Whale user benefits from the extra liquidity/fees/rewards in the big vault - -### Impact - +4. Whale user benefits from the extra liquidity/fees/rewards in the big vault + +### Impact + - unexpected behaviour -- protocol can be gamed - -### PoC - -_No response_ - -### Mitigation - -Track numa balances internally and allow users to sell tokens only from the vault that initially minted them. Additionally, add a boolean `max_percent_toggle` in the vault and perform a MAX deposit check only when it's on. This way in the scenario where the vault is left with 0.37 USD worth of rETH, admins can turn the `max_percent` check off in order to have the vault's liquidity restored before turning it on again. - +- protocol can be gamed + +### PoC + +_No response_ + +### Mitigation + +Track numa balances internally and allow users to sell tokens only from the vault that initially minted them. Additionally, add a boolean `max_percent_toggle` in the vault and perform a MAX deposit check only when it's on. This way in the scenario where the vault is left with 0.37 USD worth of rETH, admins can turn the `max_percent` check off in order to have the vault's liquidity restored before turning it on again. +