sherlock-audit / 2024-02-leverage-contracts-judging

1 stars 0 forks source link

FastTiger - In `collectLoansFees` function, the caller has to input the array of tokens manualy because the caller does not obtain the caller's token list in `LiquidityBorrowingManager.sol#harvest` function. #19

Closed sherlock-admin closed 6 months ago

sherlock-admin commented 6 months ago

FastTiger

medium

In collectLoansFees function, the caller has to input the array of tokens manualy because the caller does not obtain the caller's token list in LiquidityBorrowingManager.sol#harvest function.

Summary

The caller may feel uncomfortable having to manually input the token array, and gas loss may occur due to unnecessary token input.

Vulnerability Detail

LiquidityBorrowingManager.sol#collectLoansFees function is as follows.

    function collectLoansFees(address[] calldata tokens) external {
        mapping(address => uint256) storage collection = loansFeesInfo[msg.sender];
        uint256[] memory amounts = _collect(collection, msg.sender, tokens);

        emit CollectLoansFees(msg.sender, tokens, amounts);
    }

As you could see above the code snippet, the caller may feel uncomfortable having to manually input the token array. But the caller does not obtain the caller's token list in harvest function.

https://github.com/RealWagmi/wagmi-leverage/blob/main/contracts/LiquidityBorrowingManager.sol#L548

There are thousands of ERC20 tokens in Uniswap V3 and the caller(the owner of position(NFT)) has many positions. Therefore the caller doesn't know exactly what token he has. Also the caller has to input all address of the tokens. As a result, the caller may feel uncomfortable having to manually input the token array, and gas loss may occur due to unnecessary token input. And then, the caller may not received fees on some tokens.

Impact

The caller may feel uncomfortable having to manually input the token array and he may not received fees on some tokens.

Code Snippet

https://github.com/sherlock-audit/2024-02-leverage-contracts/blob/main/wagmi-leverage/contracts/LiquidityBorrowingManager.sol#L548 https://github.com/sherlock-audit/2024-02-leverage-contracts/blob/main/wagmi-leverage/contracts/LiquidityBorrowingManager.sol#L109C5-L114C6

Tool used

Manual Review

Recommendation

Add to follow lines in LiquidityBorrowingManager.sol#harvest function.


++  mapping(address => address[]) tokenList;

    function harvest(bytes32 borrowingKey) external nonReentrant returns (uint256 harvestedAmt) {

        ...

        for (uint256 i; i < loans.length; ) {
            LoanInfo memory loan = loans[i];
            // Get the owner address of the loan's token ID using the underlyingPositionManager contract.
            address creditor = _getOwnerOf(loan.tokenId);
            // Check if the owner of the loan's token ID is equal to the `msg.sender`.
            if (creditor != address(0)) {
                // Update the liquidity cache based on the loan information.
                _upNftPositionCache(zeroForSaleToken, loan, cache);
                uint256 feesAmt = FullMath.mulDiv(feesOwed, cache.holdTokenDebt, borrowedAmount);
                // Calculate the fees amount based on the total fees owed and holdTokenDebt.

                loansFeesInfo[creditor][cache.holdToken] += feesAmt;

++              address[] arrayToken=tokenList[creditor];
++              uint256 j;
++              for(j=0;j<arrayToken.length;){
++                  if(holdToken==arrayToken[j])
++                      break;
++                  unchecked{
++                      j++;
++                  }
++              }
++              if(j==arrayToken.length)
++                 tokenList[creditor].push(holdToken);

                harvestedAmt += feesAmt;
            }
            unchecked {
                ++i;
            }
        }

        borrowing.feesOwed -= harvestedAmt;

        emit Harvest(borrowingKey, harvestedAmt);
    }
nevillehuang commented 6 months ago

Invalid, there is no security risk presented here. This is a feature request