Uploaded files for judging

001.md

@@ -0,0 +1,69 @@
+Savory Sable Mantis
+
+medium
+
+# Inadequate Validation of Token Balance Data in ``getBalances`` Function
+## Summary
+The ``getBalances`` function in the provided smart contract lacks proper validation of the token balance data returned by the ``staticcall``. This can potentially lead to security vulnerabilities and inaccurate token balance reporting.
+
+## Vulnerability Detail
+The ``getBalances`` function iterates through an array of token addresses and uses ``staticcall`` to invoke the ``balanceOf`` function on each token contract. However, the code only checks the success of the ``staticcall`` and the length of the data returned. It does not verify the actual content of the data, making it susceptible to malicious or misbehaving token contracts.
+
+## Impact
+The inadequate validation of token balance data can have the following security and usability impacts:
+
+- Security Risks: Malicious or misconfigured token contracts can return incorrect data, leading to security vulnerabilities or manipulation of reported token balances.
+- Inaccurate Reporting: Users relying on the ``getBalances`` function may receive incorrect balance information, affecting the integrity of their transactions and decisions.
+
+## Code Snippet
+https://github.com/sherlock-audit/2023-10-real-wagmi/blob/main/wagmi-leverage/contracts/Vault.sol#L37
+
+```solidity
+function getBalances(
+        address[] calldata tokens
+    ) external view returns (uint256[] memory balances) {
+        bytes memory callData = abi.encodeWithSelector(IERC20.balanceOf.selector, address(this));
+        uint256 length = tokens.length;
+        balances = new uint256[](length);
+        for (uint256 i; i < length; ) {
+            (bool success, bytes memory data) = tokens[i].staticcall(callData);
+            require(success && data.length >= 32);
+            balances[i] = abi.decode(data, (uint256));
+            unchecked {
+                ++i;
+            }
+        }
+    }
+```
+
+## Tool used
+
+Manual Review
+
+## Recommendation
+It is recommended to enhance the security and accuracy of the getBalances function by performing thorough data validation. This can be achieved by implementing the following changes to the code:
+
+- Check that the staticcall is successful.
+- Verify the length of the data returned to ensure it matches the expected length for an uint256.
+- Validate the content of the data to ensure it represents a non-negative token balance.
+
+```solidity
+function getBalances(
+    address[] calldata tokens
+) external view returns (uint256[] memory balances) {
+    bytes memory callData = abi.encodeWithSelector(IERC20.balanceOf.selector, address(this));
+    uint256 length = tokens.length;
+    balances = new uint256[](length);
+    for (uint256 i = 0; i < length; i++) {
+        (bool success, bytes memory data) = tokens[i].staticcall(callData);
+        require(success && data.length == 32);
++      uint256 tokenBalance = abi.decode(data, (uint256));
+-       balances[i] = abi.decode(data, (uint256));
++      require(tokenBalance >= 0, "Negative token balance");
++      balances[i] = tokenBalance;
+        unchecked {
+            ++i;
+         }
+    }
+}
+```

001/011.md

@@ -0,0 +1,26 @@
+Quiet Sage Wren
+
+high
+
+# LiquidityBorrowingManager.sol#takeOverDebt() - wrong key used when pushing loans to the new borrow
+## Summary
+The ``takeOverDebt()`` function is supposed, upon met criteria, to transfer a borrowing to a different owner, by creating a new borrowing key for the mapping and passing the old loans to the new borrow struct. There is a critical flaw in this functionality, that would not allow for correct overtaking.
+
+## Vulnerability Detail
+The function updates the old borrow's fees up to the moment of taking over and then generates a new key and a new borrow struct. However, when the function ``_addKeysAndLoansInfo`` is invoked to transfer the old loans to the new key, instead of the ``newBorrowingKey``, the old ``borrowingKey`` variable is passed instead.
+This would lead to old loans being pushed again in the array for the old key, potentially reverting on exceeding the maximum allowed loans per positions.
+Taking over would not be possible in such cases.
+
+## Impact
+Impossible to take over loans, which is a core functionality of the protocol
+
+## Code Snippet
+https://github.com/sherlock-audit/2023-10-real-wagmi/blob/main/wagmi-leverage/contracts/LiquidityBorrowingManager.sol#L431-L441
+
+## Tool used
+
+Manual Review
+
+## Recommendation
+Pass the correct value to the function as such:
+``_addKeysAndLoansInfo(newBorrowing.borrowedAmount > 0, newBorrowingKey, oldLoans)``

001/017.md

@@ -0,0 +1,67 @@
+Dry Watermelon Wolverine
+
+high
+
+# Whenever a user wants to `takeOverDebt` will never work
+## Summary
+In `LiquidityBorrowingManager.sol` a user can `takeOverDebt` for a specific borrower by providing the borrower's `borrowingKey`: 
+
+```solidity
+function takeOverDebt(bytes32 borrowingKey, uint256 collateralAmt)
+```
+
+## Vulnerability Detail
+
+In order for a user to successfully take over a borrower's debt, he has to provide : 
+
+```solidity
+(collateralAmt <= minPayment).revertError(
+                ErrLib.ErrorCode.COLLATERAL_AMOUNT_IS_NOT_ENOUGH
+            );
+```
+
+`collateralAmt` is the amount of collateral to be provided by the new borrower.
+`minPayment` is the minimum payment required based on the collateral balance for the old borrower.
+
+Then loans and keys are removed from the old borrower
+
+```solidity
+_removeKeysAndClearStorage(oldBorrowing.borrower, borrowingKey, oldLoans);
+```
+
+After that the
+ ```solidity
+(uint256 feesDebt, bytes32 newBorrowingKey, BorrowingInfo storage newBorrowing) = _initOrUpdateBorrowing(
+                oldBorrowing.saleToken,
+                oldBorrowing.holdToken,
+                accLoanRatePerSeconds
+            );
+```
+is called which returns the msg.sender's (new borrower) `bytes32 newBorrowingKey` then:
+
+```solidity
+// Add the new borrowing key and old loans to the newBorrowing
+_addKeysAndLoansInfo(newBorrowing.borrowedAmount > 0, borrowingKey, oldLoans);
+```
+
+The problem is that the old loans and the initialization of the borrower are added again to the **OLD** borrower because `borrowingKey` is used in `_addKeysAndLoansInfo` rather than the new borrower's `newBorrowingKey`.
+
+
+## Impact
+
+User can't take over another borrower's debt.
+
+## Code Snippet
+
+https://github.com/sherlock-audit/2023-10-real-wagmi/blob/main/wagmi-leverage/contracts/LiquidityBorrowingManager.sol#L395-L453
+
+https://github.com/sherlock-audit/2023-10-real-wagmi/blob/main/wagmi-leverage/contracts/LiquidityBorrowingManager.sol#L915-L956
+
+## Tool used
+
+Manual Review
+
+## Recommendation
+
+**--** `_addKeysAndLoansInfo(newBorrowing.borrowedAmount > 0, borrowingKey, oldLoans);`
+**++** `_addKeysAndLoansInfo(newBorrowing.borrowedAmount > 0, newBorrowingKey, oldLoans);`

001/038.md

@@ -0,0 +1,39 @@
+Dandy Taupe Barracuda
+
+high
+
+# takeOverDebt function assigns loans and adds keys to the previous borrowing key, not the new one
+## Summary
+A main goal of the `LiquidityBorrowingManager.takeOverDebt()` function is to transfer ownership of the debt to a function caller if the caller pays collateral  for the undercollateralized position. The position's transfer happens at `_addKeysAndLoansInfo()` function. However, the argument for the function is not the `newBorrowingKey`, but the old one from which the position should be transferred from.
+## Vulnerability Detail
+`takeOverDebt()` function takes a `borrowingKey` as its input which is then used to remove the borrowing key from the storage and to delete all information associated with it in a corresponding call to the `_removeKeysAndClearStorage()` function. Then, an initialization of a borrowing happens at the `_initOrUpdateBorrowing()` function, which returns a `newBorrowingKey` to which the ownership of a position should be assigned. However, the `_addKeysAndLoansInfo()` function does not use it.
+```solidity
+_removeKeysAndClearStorage(oldBorrowing.borrower, borrowingKey, oldLoans);
+        // Initialize a new borrowing using the same saleToken, holdToken
+        (
+            uint256 feesDebt,
+            bytes32 newBorrowingKey,
+            BorrowingInfo storage newBorrowing
+        ) = _initOrUpdateBorrowing(
+                oldBorrowing.saleToken,
+                oldBorrowing.holdToken,
+                accLoanRatePerSeconds
+            );
+        // Add the new borrowing key and old loans to the newBorrowing
+        _addKeysAndLoansInfo(newBorrowing.borrowedAmount > 0, borrowingKey, oldLoans); //@audit
+```
+Since borrowing keys are hashes of msg.sender's address and tokens, the ownership stays with the previous owner. This means that the caller of the function loses his collateral by repaying the debt for the loan which should be transferred to him but which instead stays with the previous owner.
+## Impact
+Loss of funds.
+## Code Snippet
+https://github.com/sherlock-audit/2023-10-real-wagmi/blob/main/wagmi-leverage/contracts/LiquidityBorrowingManager.sol#L429-L441
+https://github.com/sherlock-audit/2023-10-real-wagmi/blob/main/wagmi-leverage/contracts/LiquidityBorrowingManager.sol#L921
+## Tool used
+
+Manual Review
+
+## Recommendation
+Pass the `newBorrowingKey` as an argument to `_addKeysAndLoansInfo()`
+```solidity
+_addKeysAndLoansInfo(newBorrowing.borrowedAmount > 0, newBorrowingKey, oldLoans);
+```

001/053.md

@@ -0,0 +1,32 @@
+Ancient Daffodil Caribou
+
+medium
+
+# old borrowing key is used instead of `newBorrowingKey` when adding old loans to the newBorrowing in LiquidityBorrowingManager.takeOverDebt()
+## Summary
+when `_addKeysAndLoansInfo()` is called within LiquidityBorrowingManager.takeOverDebt(), old Borrowing Key is used and not `newBorrowingKey`  see [here](https://github.com/sherlock-audit/2023-10-real-wagmi/blob/main/wagmi-leverage/contracts/LiquidityBorrowingManager.sol#L440-L441)
+
+
+## Vulnerability Detail
+The old borrowing key credentials are deleted in  `_removeKeysAndClearStorage(oldBorrowing.borrower, borrowingKey, oldLoans);` see  [here](https://github.com/sherlock-audit/2023-10-real-wagmi/blob/main/wagmi-leverage/contracts/LiquidityBorrowingManager.sol#L429)
+
+And a new borrowing key is created with the holdToken, saleToken, and the address of the user who wants to take over the borrowing in the `_initOrUpdateBorrowing()`. see [here](https://github.com/sherlock-audit/2023-10-real-wagmi/blob/main/wagmi-leverage/contracts/LiquidityBorrowingManager.sol#L430-L439)
+
+
+now the old borrowing key whose credentials are already deleted is used to update the old loans in `_addKeysAndLoansInfo()` instead of the `newBorrowingKey` generated in  `_initOrUpdateBorrowing()` see [here](https://github.com/sherlock-audit/2023-10-real-wagmi/blob/main/wagmi-leverage/contracts/LiquidityBorrowingManager.sol#L440-L441)
+
+## Impact
+wrong borrowing Key is used (i.e the old borrowing key) when adding old loans to `newBorrowing` 
+
+Therefore the wrong borrowing key (i.e the old borrowing key) will be added as borrowing key for tokenId of old Loans in `tokenIdToBorrowingKeys` in _addKeysAndLoansInfo()
+
+(i.e when the bug of `update bool` being false, is corrected, devs should understand :))
+
+## Code Snippet
+https://github.com/sherlock-audit/2023-10-real-wagmi/blob/main/wagmi-leverage/contracts/LiquidityBorrowingManager.sol#L440-L441
+## Tool used
+
+Manual Review
+
+## Recommendation
+use newBorrowingKey when calling  `_addKeysAndLoansInfo()` instead of old borrowing key.

001/064.md

@@ -0,0 +1,33 @@
+Careful Seafoam Bat
+
+high
+
+# In `takeOverDebt`, wrong parameter `borrowingKey` is used to call `_addKeysAndLoansInfo`
+## Summary
+
+In `takeOverDebt`, wrong parameter `borrowingKey` is used to call `_addKeysAndLoansInfo`
+
+## Vulnerability Detail
+
+The original purpose of the function `takeOverDebt` is taking over debt by transferring ownership of a borrowing to the current caller. So it uses `_addKeysAndLoansInfo` to add the oldLoans to the new caller. However, it uses wrong parameter `borrowingKey` which is the old borrowingkey.
+
+## Impact
+
+New caller loses the loans.
+
+## Code Snippet
+
+https://github.com/sherlock-audit/2023-10-real-wagmi/blob/main/wagmi-leverage/contracts/LiquidityBorrowingManager.sol#L441
+
+```solidity
+     // Add the new borrowing key and old loans to the newBorrowing
+        _addKeysAndLoansInfo(newBorrowing.borrowedAmount > 0, borrowingKey, oldLoans);
+```
+
+## Tool used
+
+Manual Review
+
+## Recommendation
+
+Replace `borrowingKey` with `newBorrowingKey`

001/073.md

@@ -0,0 +1,33 @@
+Steep Boysenberry Grasshopper
+
+high
+
+# `takeOverDebt` is not setting the new borrowing for the new borrowe/trader, resulting him paying funds without getting anything in return
+## Summary
+
+We have 2 users Bob and Alice, Bob borrows a borrowing, a borroing key is computed which is a result of `Keys.computeBorrowingKey(msg.sender, saleToken, holdToken)`. This key gets saved in `userBorrowingKeys(Bob)`. Bob's borrowing is now liquidatable, for whatever reason, Alice decides to take over Bob's debt, she calls `takeOverDebt` with the key of Bob's borrowing and the needed `collateralAmt` to take over the debt. The `takeOverDebt` function calculates the new borrowing and removed the old borrowing correctly, but fails to set it the new borrowing for alice as it is setting `_addKeysAndLoansInfo(newBorrowing.borrowedAmount > 0, borrowingKey, oldLoans);` instead of `_addKeysAndLoansInfo(newBorrowing.borrowedAmount > 0, newBorrowingKey, oldLoans);`, this results `userBorrowingKeys(alice)` to be missing the new borrowing key, and `borrowingsInfo(newBorrowingKey)` to have `borrower` set to `address(0)` instead of `alice`. This results in Alice paying funds without getting anything in return.
+
+## Vulnerability Detail
+
+Adding the `borrowing` instead of the `newBorrowing` _addKeysAndLoansInfo(newBorrowing.borrowedAmount > 0, borrowingKey, oldLoans); instead of _addKeysAndLoansInfo(newBorrowing.borrowedAmount > 0, newBorrowingKey, oldLoans);
+
+## Impact
+
+The person who pays to `takeOverDebt` will pay money however, he will get nothing in return.
+
+## Code Snippet
+
+https://github.com/sherlock-audit/2023-10-real-wagmi/blob/main/wagmi-leverage/contracts/LiquidityBorrowingManager.sol#L441
+
+```solidity
+ _addKeysAndLoansInfo(newBorrowing.borrowedAmount > 0, borrowingKey, oldLoans);
+```
+
+
+## Tool used
+
+Manual Review
+
+## Recommendation
+
+Use `_addKeysAndLoansInfo(newBorrowing.borrowedAmount > 0, newBorrowingKey, oldLoans);` instead of `_addKeysAndLoansInfo(newBorrowing.borrowedAmount > 0, borrowingKey, oldLoans);`

001/124.md

@@ -0,0 +1,48 @@
+Festive Daffodil Grasshopper
+
+high
+
+# The takeOverDebt uses the wrong borrowingKey
+## Summary
+
+The takeOverDebt should remove the old borrowingKey and add a new borrowingKey, but the code implementation is wrong and the added borrowingKey is still old.
+
+## Vulnerability Detail
+
+```solidity
+        // Retrieve the old loans associated with the borrowing key and remove them from storage
+        LoanInfo[] memory oldLoans = loansInfo[borrowingKey];
+        _removeKeysAndClearStorage(oldBorrowing.borrower, borrowingKey, oldLoans);
+        // Initialize a new borrowing using the same saleToken, holdToken
+        (
+            uint256 feesDebt,
+            bytes32 newBorrowingKey,
+            BorrowingInfo storage newBorrowing
+        ) = _initOrUpdateBorrowing(
+                oldBorrowing.saleToken,
+                oldBorrowing.holdToken,
+                accLoanRatePerSeconds
+            );
+        // Add the new borrowing key and old loans to the newBorrowing
+        // @audit using the wrong borrowingKey
+        _addKeysAndLoansInfo(newBorrowing.borrowedAmount > 0, borrowingKey, oldLoans);
+```
+
+- The wrong borrowingKey caused the new user to use the loanInfo corresponding to the previous user's borrowingKey, causing confusion in the internal accounting system.
+- When the user borrows again, _initOrUpdateBorrowing will consider that is an init state, and a new state will be created based on newBorrowingKey instead of updating the borrowingKey.
+
+## Impact
+
+Wrong borrowingKey caused confusion in the internal accounting system and affected user funds
+
+## Code Snippet
+
+- https://github.com/sherlock-audit/2023-10-real-wagmi/blob/b33752757fd6a9f404b8577c1eae6c5774b3a0db/wagmi-leverage/contracts/LiquidityBorrowingManager.sol#L441
+
+## Tool used
+
+Manual Review
+
+## Recommendation
+
+use newBorrowingKey instead of borrowingKey