diff --git a/Audit_Report.pdf b/Audit_Report.pdf new file mode 100644 index 0000000..14fe90e Binary files /dev/null and b/Audit_Report.pdf differ diff --git a/README.md b/README.md index 4c8d222..73a7c8d 100644 --- a/README.md +++ b/README.md @@ -4,6 +4,7 @@ Source: https://github.com/sherlock-audit/2024-10-debita-judging/issues/156 ## Found by 0xc0ffEE + ### Summary The incorrect logic in function `veNFTAerodrome::getDataByReceipt()` will cause the lenders and borrowers unable to claim liquidation token after the NFT auction sold @@ -150,12 +151,23 @@ Ran 1 test for test/fork/Loan/ltv/OracleOneLenderLoanReceipt.t.sol:DebitaAggrega 1/ Update the function `getDataByReceipt()` to handle the case non-exist token, instead of reverting 2/ OR update the logic to fetch the `decimals` in functions `claimCollateralAsNFTLender` and `claimCollateralNFTAsBorrower` +## Discussion + +**sherlock-admin2** + +The protocol team fixed this issue in the following PRs/commits: +https://github.com/DebitaFinance/Debita-V3-Contracts/commit/8eb4deaff92b143dfb838f0eda8c5adeca2fac8c + + + + # Issue H-2: Nobody can buy the `TaxTokenReceipt` NFT from auction Source: https://github.com/sherlock-audit/2024-10-debita-judging/issues/388 ## Found by 0x37, 0xPhantom2, KaplanLabs, KiroBrejka, bbl4de, dhank, dimulski, tmotfl, xiaoming90 + ### Summary Nobody can buy the `TaxTokenReceipt` NFT from auction due to the overrided `transferFrom` function. The `transferFrom` function is overrided with the following checks: @@ -213,12 +225,23 @@ _No response_ Add a check to the `TaxTokenReceipt` NFT that ensures that an auction is active (Has an index different than 0 in the `auctionFactoryDebita::AuctionOrderIndex` mapping) and it is part of the `Debita` system (as it is indeed part of the system) +## Discussion + +**sherlock-admin2** + +The protocol team fixed this issue in the following PRs/commits: +https://github.com/DebitaFinance/Debita-V3-Contracts/commit/77653c1b2b5aacdbf4ae340d50504e056b8d8540 + + + + # Issue H-3: Managed veAERO NFT can be exploited to steal funds from lenders Source: https://github.com/sherlock-audit/2024-10-debita-judging/issues/535 ## Found by KaplanLabs, xiaoming90 + ### Summary _No response_ @@ -269,12 +292,25 @@ _No response_ _No response_ +## Discussion + +**sherlock-admin2** + +The protocol team fixed this issue in the following PRs/commits: +https://github.com/DebitaFinance/Debita-V3-Contracts/commit/992eb89cb38543a8fa4d168fef79e2f1c8ab67e2 + + + + # Issue H-4: No one can sell `TaxTokensReceipts` NFT receipt to the buy order Source: https://github.com/sherlock-audit/2024-10-debita-judging/issues/560 +The protocol has acknowledged this issue. + ## Found by 0x37, KiroBrejka, bbl4de, dimulski, xiaoming90 + ### Summary _No response_ @@ -390,6 +426,7 @@ Source: https://github.com/sherlock-audit/2024-10-debita-judging/issues/890 ## Found by 0x37, 0xPhantom2, 4lifemen, Audinarey, BengalCatBalu, CL001, Cybrid, DenTonylifer, Greed, Greese, IzuMan, KiroBrejka, KungFuPanda, Pro\_King, Valy001, alexbabits, araj, dhank, dimulski, durov, kazan, lanrebayode77, merlin, newspacexyz, nikhilx0111, pashap9990, shaflow01, t.aksoy, utsav, xiaoming90, ydlee + ### Summary After sellNFT is completed, the NFT should be transferred to the order creator, but this is not done. @@ -454,12 +491,23 @@ Logs: After the buyOrder is completed,the NFT should be transferred to the order creator +## Discussion + +**sherlock-admin2** + +The protocol team fixed this issue in the following PRs/commits: +https://github.com/DebitaFinance/Debita-V3-Contracts/commit/d6f3b76c256713f0aa132a015ced6eb60ec389cb + + + + # Issue M-1: Lend offer can be deleted multiple times Source: https://github.com/sherlock-audit/2024-10-debita-judging/issues/119 ## Found by 0x37, 0xAristos, 0xSolus, KlosMitSoss, Vidus, bbl4de, copperscrewer, liquidbuddha, newspacexyz, onthehunt, theweb3mechanic + ### Summary Lack of check in addFunds() function. This will cause one lend offer can be deleted twice. @@ -534,12 +582,23 @@ When we delete the lend order, we should set it to inactive. This will prevent c } else { ``` +## Discussion + +**sherlock-admin2** + +The protocol team fixed this issue in the following PRs/commits: +https://github.com/DebitaFinance/Debita-V3-Contracts/commit/307e2360dd9aaac443f17547466c51718d4cefd3 + + + + # Issue M-2: Borrowers can not extend loans which has maximum duration less than 24 hours Source: https://github.com/sherlock-audit/2024-10-debita-judging/issues/153 ## Found by 0x37, 0xc0ffEE, KaplanLabs, bbl4de, dhank, farismaulana, nikhil840096, ydlee + ### Summary The logics to calculate the missing borrow fee is incorrect, which will cause the borrowers can not extend loans having maximum duration less than 24 hours because of arithmetic underflow @@ -636,12 +695,23 @@ Consider updating like below + feeOfMaxDeadline = minFEE; ``` +## Discussion + +**sherlock-admin2** + +The protocol team fixed this issue in the following PRs/commits: +https://github.com/DebitaFinance/Debita-V3-Contracts/commit/2958108a7a7307830953dec9bf4f3178a6cff434 + + + + # Issue M-3: The precision loss in the fee percentage for connecting offers results in the borrower paying less than the expected fee. Source: https://github.com/sherlock-audit/2024-10-debita-judging/issues/208 ## Found by nikhil840096, ydlee + ### Summary Some of the borrowed principal tokens is charged as a fee for connecting transactions. The percentage of the fee is calculated according to `DebitaV3Aggregator.sol:391`. There is a non-negligible precision loss in the calculation process. Since the default `feePerDay` is `4`, the maximum loss could reach up to `1/4` of the daily fee, which is significant, especially when the amount borrowed is substantial. @@ -691,12 +761,23 @@ _No response_ When calculating the fee percentage, rounding up; or multiplying by a multiple to increase precision. +## Discussion + +**sherlock-admin2** + +The protocol team fixed this issue in the following PRs/commits: +https://github.com/DebitaFinance/Debita-V3-Contracts/commit/c946cdd90c8d4841fa254359a863d3b574e34566 + + + + # Issue M-4: The fee calculation in extendLoan function has a error Source: https://github.com/sherlock-audit/2024-10-debita-judging/issues/211 ## Found by 0x37, 0xPhantom2, 0xc0ffEE, 0xe4669da, ExtraCaterpillar, Falendar, KaplanLabs, Maroutis, Nave765, bbl4de, dany.armstrong90, davidjohn241018, dhank, dimulski, durov, jsmi, momentum, newspacexyz, shaflow01, ydlee + ### Summary When a borrower extends the loan duration, they are required to pay additional fees for the extended time. However, due to a calculation error, this fee may be incorrect, potentially causing the user to pay more than necessary. @@ -767,12 +848,25 @@ _No response_ ``` ``` +## Discussion + +**sherlock-admin2** + +The protocol team fixed this issue in the following PRs/commits: +https://github.com/DebitaFinance/Debita-V3-Contracts/commit/63f8c4b1e4e7df734bf09260bd951f2c3e0da736 + + + + # Issue M-5: Incorrect calculation of extended loan days leads to unfair borrower fees Source: https://github.com/sherlock-audit/2024-10-debita-judging/issues/236 +The protocol has acknowledged this issue. + ## Found by newspacexyz, prosper + ### Summary The miscalculation of extended loan days in the `extendLoan` function will cause borrowers to face unfair fees as the function incorrectly calculates the fee based on `offer.maxDeadline` instead of using the actual extended days derived from `nextDeadline()` and `m_loan.startedAt`. This leads to inflated fee deductions during loan extensions. @@ -836,6 +930,7 @@ Source: https://github.com/sherlock-audit/2024-10-debita-judging/issues/246 ## Found by 0x37, 0xPhantom2, 0xloscar01, 0xlrivo, Ace-30, Audinarey, BengalCatBalu, ExtraCaterpillar, Feder, Honour, KlosMitSoss, Moksha, Vasquez, ahmedovv, almantare, aman, araj, arman, bbl4de, befree3x, dany.armstrong90, dimah7, dimulski, eeshenggoh, farismaulana, jjk, jsmi, liquidbuddha, momentum, nikhil840096, onthehunt, pepocpeter, prosper, s0x0mtee, stakog, t.aksoy, tjonair, tourist, utsav, ydlee + ### Summary The missing active order check in `DLOImplementation::addFunds` will allow an attacker to halt the cancellation of lend orders for every lender and prevent non-perpetual lend orders from being fully matched as the attacker will execute the following attack path: @@ -1303,12 +1398,23 @@ Add a check in `DLOImplementation::addFunds` to prevent adding funds to an inact ); ``` +## Discussion + +**sherlock-admin2** + +The protocol team fixed this issue in the following PRs/commits: +https://github.com/DebitaFinance/Debita-V3-Contracts/commit/206c6ede06ab1d84943bd0b8431d6c0a9c9faaa7 + + + + # Issue M-7: An attacker can wipe the orderbook in buyOrderFactory.sol Source: https://github.com/sherlock-audit/2024-10-debita-judging/issues/326 ## Found by 0x37, AdamSzymanski, VAD37, Vidus, copperscrewer, liquidbuddha, tourist + ### Summary A malicious actor can wipe the complete buy order orderbook in `buyOrderFactory.sol`. The attack - excluding gas costs - does not bear any financial burden on the attacker. As a result of the exploit, the orderbook will be temporarily inaccessible in the factory, leading to a DoS state in buy order matching, and in closing and selling existing positions. @@ -1467,12 +1573,23 @@ contract BuyOrderTest is Test { Apply reentrancy protection on the function `sellNFT(uint receiptID)`: https://github.com/sherlock-audit/2024-11-debita-finance-v3/blob/main/Debita-V3-Contracts/contracts/buyOrders/buyOrder.sol#L92 +## Discussion + +**sherlock-admin2** + +The protocol team fixed this issue in the following PRs/commits: +https://github.com/DebitaFinance/Debita-V3-Contracts/commit/f3195e007b0c22dc0173a344157c182726dbf2ec + + + + # Issue M-8: Lender may loose part of the interest he has accrued if he makes his lend offer perpetual after a loan has been extended by the borrower Source: https://github.com/sherlock-audit/2024-10-debita-judging/issues/345 ## Found by dimulski + ### Summary The ``DebitaV3Loan.sol`` contract allows borrowers to extend their loan against certain fees by calling the [extendLoan()](https://github.com/sherlock-audit/2024-11-debita-finance-v3/blob/main/Debita-V3-Contracts/contracts/DebitaV3Loan.sol#L547-L664) function: @@ -1590,12 +1707,23 @@ _No response_ Consider implementing a separate function just for claiming interest. +## Discussion + +**sherlock-admin2** + +The protocol team fixed this issue in the following PRs/commits: +https://github.com/DebitaFinance/Debita-V3-Contracts/commit/8008bb515d7f6eafc5c98a444d985cd6f9416c37 + + + + # Issue M-9: Mixed Token Price Will Be Inflated or Deflated Source: https://github.com/sherlock-audit/2024-10-debita-judging/issues/362 ## Found by dimulski, jsmi + ### Summary The `MixOracle::getThePrice()` function contains a logical error, causing the calculated price of a token to be incorrectly inflated or deflated. This issue arises when token pairs with differing decimal scales are used, leading to inaccurate pricing data. @@ -1680,12 +1808,23 @@ In line `61`, replace `decimalsToken1` with `decimalsToken0`. + (10 ** decimalsToken0); ``` +## Discussion + +**sherlock-admin2** + +The protocol team fixed this issue in the following PRs/commits: +https://github.com/DebitaFinance/Debita-V3-Contracts/commit/c5573af2b866686390a82eef67817454624ed9c7 + + + + # Issue M-10: Precision loss leads to locked incentives in `DebitaIncentives::claimIncentives()` Source: https://github.com/sherlock-audit/2024-10-debita-judging/issues/385 ## Found by BengalCatBalu, KlosMitSoss, Maroutis, VAD37, dany.armstrong90, jsmi, pashap9990 + ### Summary When a lender or borrwer calls `DebitaIncentives::claimIncentives()` to claim a share of the incentives for a specific token pair they interacted with during an epoch, their share is calculated as a [percentage](https://github.com/sherlock-audit/2024-11-debita-finance-v3/blob/1465ba6884c4cc44f7fc28e51f792db346ab1e33/Debita-V3-Contracts/contracts/DebitaIncentives.sol#L161). This percentage is determined based on the amount they lent or borrowed through the protocol during that epoch, relative to the total amount lent and borrowed by all users in the same period. @@ -1836,12 +1975,23 @@ function testUnclaimableIncentives() public { Consider adding a mechanism that allows incentivizers to withdraw their unclaimed incentives from all their past incentivized epochs after a specified period following the end of the last incentivized epoch (e.g., two epochs later). +## Discussion + +**sherlock-admin2** + +The protocol team fixed this issue in the following PRs/commits: +https://github.com/DebitaFinance/Debita-V3-Contracts/commit/f9fc693f1fb78447b44a11be8808b1d482b6e0ae + + + + # Issue M-11: Auctioned `taxTokensReceipt` NFT Blocks Last Claimant Due to Insufficient Funds Source: https://github.com/sherlock-audit/2024-10-debita-judging/issues/470 ## Found by 0x37, KaplanLabs, bbl4de, dimulski + ### Summary In the [`Auction::buyNFT`](https://github.com/sherlock-audit/2024-11-debita-finance-v3/blob/1465ba6884c4cc44f7fc28e51f792db346ab1e33/Debita-V3-Contracts/contracts/auctions/Auction.sol#L109) function, users can purchase the current NFT in an auction using the same type of tokens as the underlying asset of the NFT. For example, `taxTokensReceipt` created with FoT tokens must be bought with the same FoT tokens. @@ -1905,12 +2055,23 @@ _No response_ Take into account the fee on transfer when buying the `taxTokenReceipt` NFT. +## Discussion + +**sherlock-admin2** + +The protocol team fixed this issue in the following PRs/commits: +https://github.com/DebitaFinance/Debita-V3-Contracts/commit/7c2bd9e63c95e38f22f3478d36f72c33e8b17117 + + + + # Issue M-12: A borrower may pay more interest that he has specified, if orders are matched by a malicious actor Source: https://github.com/sherlock-audit/2024-10-debita-judging/issues/480 ## Found by 0x37, BengalCatBalu, dimulski, h4rs0n, newspacexyz + ### Summary In the ``DebitaV3Aggregator.sol`` contract, anybody can match borrow and lend orders by calling the [matchOffersV3()](https://github.com/sherlock-audit/2024-11-debita-finance-v3/blob/main/Debita-V3-Contracts/contracts/DebitaV3Aggregator.sol#L274-L647) function. When borrowers create their borrow orders they specify a maximum APR they are willing to pay. However the [matchOffersV3()](https://github.com/sherlock-audit/2024-11-debita-finance-v3/blob/main/Debita-V3-Contracts/contracts/DebitaV3Aggregator.sol#L274-L647) function allows the caller to specify an array of 29 different lend orders to be matched against each borrow offer. Another important parameter is the ``uint[] memory lendAmountPerOrder`` which specifies the amount of principal tokens each lend order will provide. A malicious user can provide smaller amounts for a part of the ``lendAmountPerOrder`` params, such that the borrower pays a higher APR on this amounts. The problem arises due to the fact that there are no minimum restrictions on the ``lendAmountPerOrder`` (except it being bigger than 0), and the fact that solidity rounds down on division. @@ -2163,12 +2324,23 @@ To run the test use: ``forge test -vvv --mt test_InflateAPR`` ### Mitigation _No response_ +## Discussion + +**sherlock-admin2** + +The protocol team fixed this issue in the following PRs/commits: +https://github.com/DebitaFinance/Debita-V3-Contracts/commit/deaf6819f98d9d8f5ead178cd21ed80921d5f340 + + + + # Issue M-13: Interest paid for non perpetual loan during loan extension is lost when the borrower repays debt Source: https://github.com/sherlock-audit/2024-10-debita-judging/issues/483 ## Found by 0x37, Audinarey, ExtraCaterpillar, KaplanLabs, dimulski, newspacexyz, robertodf, t.aksoy + ### Summary When a borrower call `extendLoan()`to extend a loan to the max deadline, interest is accrued to the lender up until the time the loan is extended and the lender's `interestPaid` is updated as well for accounting purpose when calculating the interest with `calculateInterestToPay()` @@ -2246,12 +2418,23 @@ File: DebitaV3Loan.sol ``` +## Discussion + +**sherlock-admin2** + +The protocol team fixed this issue in the following PRs/commits: +https://github.com/DebitaFinance/Debita-V3-Contracts/commit/c6047ca43070746cbf8adcab12784e583d1de5d5 + + + + # Issue M-14: The `MixOracle.getThePrice` function calculates the price incorrectly using the `TarotOracle.getResult` function as the TWAP price Source: https://github.com/sherlock-audit/2024-10-debita-judging/issues/501 ## Found by KupiaSec + ### Summary The `MixOracle.getThePrice` function calculates the price using the `TarotOracle` contract and the `pyth` oracle. However, it incorrectly uses the `TarotOracle.getResult` function as the TWAP price, which disrupts the matching mechanism for lend and borrow orders. @@ -2433,12 +2616,23 @@ File: code\Debita-V3-Contracts\contracts\oracles\MixOracle\MixOracle.sol } ``` +## Discussion + +**sherlock-admin2** + +The protocol team fixed this issue in the following PRs/commits: +https://github.com/DebitaFinance/Debita-V3-Contracts/commit/febd7ff204f60af400cd4ff00706da0bb7d47609 + + + + # Issue M-15: MixOracle is broken due to hardcoded position Source: https://github.com/sherlock-audit/2024-10-debita-judging/issues/540 ## Found by tjonair, xiaoming90 + ### Summary _No response_ @@ -2549,12 +2743,23 @@ _No response_ Consider not hardcoding the position (`token1`) as the key of the mapping used within `MixOracle`. Instead, allow the deployer to specify which token (`token0` or `token1`) the `MixOracle` is supposed to support. +## Discussion + +**sherlock-admin2** + +The protocol team fixed this issue in the following PRs/commits: +https://github.com/DebitaFinance/Debita-V3-Contracts/commit/83b5cf41c6100a4b31be2779bb66e7c41ea957a6 + + + + # Issue M-16: Users can be griefed due to lack of minimum size within the Loan and Offer Source: https://github.com/sherlock-audit/2024-10-debita-judging/issues/557 ## Found by xiaoming90 + ### Summary _No response_ @@ -2603,12 +2808,23 @@ Having a maximum number of offers (e.g., 100) within a single Loan is insufficie Thus, it is recommended to impose the minimum size for each loan and/or offer, so that malicious aggregators cannot create many small/tiny loans and offers to grief the users. +## Discussion + +**sherlock-admin2** + +The protocol team fixed this issue in the following PRs/commits: +https://github.com/DebitaFinance/Debita-V3-Contracts/commit/c7567f5dbd9d8e224e6e3a684cc396a3829775e1 + + + + # Issue M-17: Borrower can obtain principle tokens without paying collateral tokens Source: https://github.com/sherlock-audit/2024-10-debita-judging/issues/558 ## Found by xiaoming90 + ### Summary _No response_ @@ -2690,12 +2906,25 @@ uint userUsedCollateral = (lendAmountPerOrder[i] * (10 ** decimalsCollateral)) / + require(userUsedCollateral > 0, "userUsedCollateral is zero") ``` +## Discussion + +**sherlock-admin2** + +The protocol team fixed this issue in the following PRs/commits: +https://github.com/DebitaFinance/Debita-V3-Contracts/commit/a60b40e8b76b7d23cff1e7ef8310819ad251f0e3 + + + + # Issue M-18: Incentive Creator's Tokens Permanently Locked in Zero-Activity Epochs Source: https://github.com/sherlock-audit/2024-10-debita-judging/issues/616 +The protocol has acknowledged this issue. + ## Found by 0x37, BengalCatBalu, KaplanLabs, dimulski, h4rs0n, jo13, newspacexyz, t.aksoy, xiaoming90 + ### Summary The lack of token recovery mechanism in DebitaIncentives.sol will cause permanent loss of incentive tokens for incentive creators as tokens remain locked in the contract during epochs with zero lending/borrowing activity. @@ -2762,6 +2991,7 @@ Source: https://github.com/sherlock-audit/2024-10-debita-judging/issues/708 ## Found by BengalCatBalu, CL001, Feder, KaplanLabs, KlosMitSoss, Pablo, bbl4de, dimulski, lanrebayode77, mike-watson, pashap9990, tjonair, xiaoming90 + ### Summary An attacker, with the aid of flash-loan can steal the entire borrow and lending incentives. @@ -2846,12 +3076,23 @@ https://github.com/sherlock-audit/2024-11-debita-finance-v3/blob/main/Debita-V3- 1. Prevent repayment in the same block 2. It might be helpful to prevent lender == borrower == connector +## Discussion + +**sherlock-admin2** + +The protocol team fixed this issue in the following PRs/commits: +https://github.com/DebitaFinance/Debita-V3-Contracts/commit/bc889a3d624b8376c9be43f5421a78448bdaca20 + + + + # Issue M-20: Loan Extension Fails Due to Unused Time Calculation Source: https://github.com/sherlock-audit/2024-10-debita-judging/issues/766 ## Found by 0x37, 0xPhantom2, 0xmujahid002, Audinarey, BengalCatBalu, Falendar, Honour, dany.armstrong90, dimulski, jsmi, mladenov, moray5554, newspacexyz, nikhil840096, nikhilx0111, yovchev\_yoan, zkillua + ### Summary The `extendLoan` function in `DebitaV3Loan::extendLoan` has redundant time calculation logic that causes transaction reversions when borrowers attempt to extend loans near their deadline. @@ -2930,12 +3171,23 @@ The calculation flow: Remove the unused `extendedTime` calculation as it serves no purpose and can cause legitimate loan extensions to fail. +## Discussion + +**sherlock-admin2** + +The protocol team fixed this issue in the following PRs/commits: +https://github.com/DebitaFinance/Debita-V3-Contracts/commit/dd1ad26725ba4835bc6406acf1289c1fbd33f8f2 + + + + # Issue M-21: DebitaIncentives::updateFunds will exit prematurely and not update whitelisted pairs causing loss of funds to lenders and borrowers Source: https://github.com/sherlock-audit/2024-10-debita-judging/issues/870 ## Found by 0x37, 0xe4669da, 0xloscar01, BengalCatBalu, DenTonylifer, ExtraCaterpillar, Honour, Pro\_King, Ryonen, bbl4de, dimulski, jjk, jsmi, liquidbuddha, merlin, newspacexyz, robertodf, t.aksoy, tmotfl + ### Summary The `DebitaIncentives::updateFunds` function iterates over the `lenders` array, verifying whether the principle and collateral pair for each lend offer is whitelisted. If a non-whitelisted pair is encountered, the function exits prematurely, causing it to skip the processing of all subsequent pairs, even if they are valid and whitelisted. @@ -3557,12 +3809,25 @@ After applying the change, running the test case provided in the PoC will output borrowAmountPerEpoch: 5000000000000000000 ``` +## Discussion + +**sherlock-admin2** + +The protocol team fixed this issue in the following PRs/commits: +https://github.com/DebitaFinance/Debita-V3-Contracts/commit/f185f42fcdbeca0bb9005767c1302c8daf24e940 + + + + # Issue M-22: Previous owner can steal unclaimed bribes from new owner of veNFTVault Source: https://github.com/sherlock-audit/2024-10-debita-judging/issues/875 +The protocol has acknowledged this issue. + ## Found by 0xc0ffEE, DenTonylifer, Flashloan44, Greese, KiroBrejka, VAD37, eeshenggoh, xiaoming90 + ### Summary Previous owner can steal unclaimed bribes from new owner of `veNFT`, because transfering ownership of `veNFT` does not change the manager (which can claim bribes, vote).