sherlock-audit / 2024-04-teller-finance-judging

10 stars 9 forks source link

samuraii77 - Users can mint any amount of loan NFTs for free #284

Closed sherlock-admin2 closed 5 months ago

sherlock-admin2 commented 5 months ago

samuraii77

high

Users can mint any amount of loan NFTs for free

Summary

Users can mint any amount of loan NFTs for free

Vulnerability Detail

A user can submit a bid using the TellerV2::submitBid() function.

function submitBid(
        address _lendingToken,
        uint256 _marketplaceId,
        uint256 _principal,
        uint32 _duration,
        uint16 _APR,
        string calldata _metadataURI,
        address _receiver
    ) public override whenNotPaused returns (uint256 bidId_) {
        bidId_ = _submitBid(
            _lendingToken,
            _marketplaceId,
            _principal,
            _duration,
            _APR,
            _metadataURI,
            _receiver
        );
    }
function _submitBid(
        address _lendingToken,
        uint256 _marketplaceId,
        uint256 _principal,
        uint32 _duration,
        uint16 _APR,
        string calldata _metadataURI,
        address _receiver
    ) internal virtual returns (uint256 bidId_) {
        address sender = _msgSenderForMarket(_marketplaceId);

        (bool isVerified, ) = marketRegistry.isVerifiedBorrower(
            _marketplaceId,
            sender
        );

        require(isVerified, "Not verified borrower");

        require(
            marketRegistry.isMarketOpen(_marketplaceId),
            "Market is not open"
        );

        // Set response bid ID.
        bidId_ = bidId;

        // Create and store our bid into the mapping
        Bid storage bid = bids[bidId];
        bid.borrower = sender;
        bid.receiver = _receiver != address(0) ? _receiver : bid.borrower
        bid.marketplaceId = _marketplaceId;
        bid.loanDetails.lendingToken = IERC20(_lendingToken);
        bid.loanDetails.principal = _principal;
        bid.loanDetails.loanDuration = _duration;
        bid.loanDetails.timestamp = uint32(block.timestamp);

        // Set payment cycle type based on market setting (custom or monthly)
        (bid.terms.paymentCycle, bidPaymentCycleType[bidId]) = marketRegistry
            .getPaymentCycle(_marketplaceId);

        bid.terms.APR = _APR;

        bidDefaultDuration[bidId] = marketRegistry.getPaymentDefaultDuration( 
            _marketplaceId
        );

        bidExpirationTime[bidId] = marketRegistry.getBidExpirationTime(
            _marketplaceId
        );

        bid.paymentType = marketRegistry.getPaymentType(_marketplaceId);

        bid.terms.paymentCycleAmount = V2Calculations
            .calculatePaymentCycleAmount(
                bid.paymentType,
                bidPaymentCycleType[bidId],
                _principal,
                _duration,
                bid.terms.paymentCycle,
                _APR
            );

        uris[bidId] = _metadataURI;
        bid.state = BidState.PENDING;

        emit SubmittedBid(
            bidId,
            bid.borrower,
            bid.receiver,
            keccak256(abi.encodePacked(_metadataURI))
        );

        // Store bid inside borrower bids mapping
        borrowerBids[bid.borrower].push(bidId);

        // Increment bid id counter
        bidId++;
    }

The thing is, he can submit it with 0 collateral and 0 principal. Then, he can accept that same bid using the TellerV2::lenderAcceptBid().

function lenderAcceptBid(uint256 _bidId)
        external
        override
        pendingBid(_bidId, "lenderAcceptBid")
        whenNotPaused
        returns (
            uint256 amountToProtocol,
            uint256 amountToMarketplace,
            uint256 amountToBorrower
        )
    {
        // Retrieve bid
        Bid storage bid = bids[_bidId];

        address sender = _msgSenderForMarket(bid.marketplaceId);

        (bool isVerified, ) = marketRegistry.isVerifiedLender(
            bid.marketplaceId,
            sender
        );
        require(isVerified, "Not verified lender");

        require(
            !marketRegistry.isMarketClosed(bid.marketplaceId),
            "Market is closed"
        );

        require(!isLoanExpired(_bidId), "Bid has expired");

        // Set timestamp
        bid.loanDetails.acceptedTimestamp = uint32(block.timestamp);
        bid.loanDetails.lastRepaidTimestamp = uint32(block.timestamp);

        // Mark borrower's request as accepted
        bid.state = BidState.ACCEPTED;

        // Declare the bid acceptor as the lender of the bid
        bid.lender = sender;

        // Tell the collateral manager to deploy the escrow and pull funds from the borrower if applicable
        collateralManager.deployAndDeposit(_bidId);

        // Transfer funds to borrower from the lender
        amountToProtocol = bid.loanDetails.principal.percent(protocolFee());
        amountToMarketplace = bid.loanDetails.principal.percent(
            marketRegistry.getMarketplaceFee(bid.marketplaceId)
        );
        amountToBorrower =
            bid.loanDetails.principal - amountToProtocol - amountToMarketplace;

        //transfer fee to protocol
        if (amountToProtocol > 0) {
            bid.loanDetails.lendingToken.safeTransferFrom(
                sender,
                owner(),
                amountToProtocol
            );
        }

        //transfer fee to marketplace
        if (amountToMarketplace > 0) {
            bid.loanDetails.lendingToken.safeTransferFrom(
                sender,
                marketRegistry.getMarketFeeRecipient(bid.marketplaceId),
                amountToMarketplace
            );
        }

        //transfer funds to borrower
        if (amountToBorrower > 0) {
            bid.loanDetails.lendingToken.safeTransferFrom(
                sender,
                bid.receiver,
                amountToBorrower
            );
        }

        // Record volume filled by lenders
        lenderVolumeFilled[address(bid.loanDetails.lendingToken)][sender] += bid
            .loanDetails
            .principal;
        totalVolumeFilled[address(bid.loanDetails.lendingToken)] += bid
            .loanDetails
            .principal;

        // Add borrower's active bid
        _borrowerBidsActive[bid.borrower].add(_bidId);

        // Emit AcceptedBid
        emit AcceptedBid(_bidId, sender);

        emit FeePaid(_bidId, "protocol", amountToProtocol);
        emit FeePaid(_bidId, "marketplace", amountToMarketplace);
    }

There is no principal to pay out so this will cost him absolutely nothing. As he has became the lender of that loan, he can now call the TellerV2::claimLoanNFT() function:

function claimLoanNFT(uint256 _bidId)
        external
        acceptedLoan(_bidId, "claimLoanNFT")
        whenNotPaused
    {
        // Retrieve bid
        Bid storage bid = bids[_bidId];

        address sender = _msgSenderForMarket(bid.marketplaceId);
        require(sender == bid.lender, "only lender can claim NFT");

        // set lender address to the lender manager so we know to check the owner of the NFT for the true lender
        bid.lender = address(USING_LENDER_MANAGER);

        // mint an NFT with the lender manager
        lenderManager.registerLoan(_bidId, sender);
    }

This will mint him an NFT with the only cost of paying gas fees. This can be done again and again by any user.

Impact

Users can mint any amount of loan NFTs for free

Code Snippet

https://github.com/sherlock-audit/2024-04-teller-finance/blob/defe55469a2576735af67483acf31d623e13592d/teller-protocol-v2-audit-2024/packages/contracts/contracts/TellerV2.sol#L578-L594 https://github.com/sherlock-audit/2024-04-teller-finance/blob/defe55469a2576735af67483acf31d623e13592d/teller-protocol-v2-audit-2024/packages/contracts/contracts/TellerV2.sol#L283-L301 https://github.com/sherlock-audit/2024-04-teller-finance/blob/defe55469a2576735af67483acf31d623e13592d/teller-protocol-v2-audit-2024/packages/contracts/contracts/TellerV2.sol#L481-L558

Tool used

Manual Review

Recommendation

One way to fix this would be to not allow borrows of 0 principal tokens.

nevillehuang commented 4 months ago

Low/informational, this NFT would likely have no monetary value given no collateral is backing, so spamming mint would have no substantial impact. A possible DoS impact could be present here but the economic cost and impact wasn't highlighted during the time of the audit, and I believe would incur a unreasonable amount of gas cost anyways to reach uint(256).max number of NFT ids.