A market owner can put borrowers in a very unfavorable position and steal money out of lenders

Summary

Vulnerability Detail

Users can create their own markets with their own terms. Then, lenders and borrowers can use that market to have different deals. However, a market owner can easily steal money out of the borrowers and leave them in an extremely unfavorable situation, leave them without any principal to take but with the obligation to pay back the loan which means that they can either pay it out even though they did not receive any principal amount or decide to not pay it and have their collateral taken as well as have their account reputation marked by the system.

The scenario that I will explain will be in a case where a lender and a market owner have teamed up (they can even be the same person). Another possible scenario doesn't include a lender on the side of the market owner but just the market owner frontrunning.

Imagine the following scenario (Bob is the market owner, Alice is the borrower, Eve is the lender:

Bob creates his own market with very favorable terms for borrowers and lenders using MarketRegistry::createMarket()
Alice submits a bid to borrow funds using Bob's market as she likes the terms Bob has set using TellerV2::submitBid() (the one with the Collateral[] variable included). Let's say Alice wants a principal of 1000 tokens and has given 1500 tokens as collateral.

Bob and Eve want to steal Alice's collateral. Bob calls the setMarketFeePercent() function and sets the new market fee to a value of 100% - protocol fee percent using setMarketFeePercent(). Let's say the protocol fee is 10% so the new market fee is 90%.

function setMarketFeePercent(uint256 _marketId, uint16 _newPercent)
    public
    ownsMarket(_marketId)
{
    require(_newPercent >= 0 && _newPercent <= 10000, "invalid percent");
    if (_newPercent != markets[_marketId].marketplaceFeePercent) {
        markets[_marketId].marketplaceFeePercent = _newPercent;
        emit SetMarketFee(_marketId, _newPercent);
    }
}

Then Eve accepts the bid created by Alice.
```
    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
        );
    }
```
This is a small snippet out of lenderAcceptBid() used for accepting bids created by borrowers. First of all, it takes the amountToProtocol out of the principal, that is 10% out of the 1000 tokens principal, so 100 tokens. Then, the fee for the marketplace will be 90% as that was just set by Bob and the protocol doesn't use a cached value of the fee but instead fetches it at that very moment. The amountToMarketplace will be 90% out of 1000 tokens, 900 tokens. amountToBorrower will be 1000 - 100 - 900, so 0 tokens. Then, the fees for the protocol are transferred to them, the amount for the marketplace is sent to Bob or someone Bob has set to receive the fees and Alice receives a grand total of 0 tokens. However, she has still deposited 1500 tokens as collateral to an escrow contract.
```
    // Tell the collateral manager to deploy the escrow and pull funds from the borrower if applicable
    collateralManager.deployAndDeposit(_bidId);
```
She also still has 1000 tokens as principal on paper however as seen above, she actually got 0. That means that she either has to pay that principal amount + interest or her loan will default and she will have her collateral taken leaving her in an extremely unfavourable situation. In the end, Eve and Bob used 100 tokens (100 for the protocol while the 900 get sent to a trusted address) and they got 1000 tokens + interest if Alice pays out the loan or 1500 tokens in collateral if she doesn't. The other possible scenario is the market owner just frontrunning the acceptance of the bid causing a situation where the lender is in a neutral situation, he paid the tokens to the protocol and the market but he will still receive the interest + tokens or the collateral and the borrower is in the same unpleasant situation.

Impact

A market owner can steal money out of borrowers

Code Snippet

https://github.com/sherlock-audit/2024-04-teller-finance/blob/defe55469a2576735af67483acf31d623e13592d/teller-protocol-v2-audit-2024/packages/contracts/contracts/MarketRegistry.sol#L636-L645 https://github.com/sherlock-audit/2024-04-teller-finance/blob/defe55469a2576735af67483acf31d623e13592d/teller-protocol-v2-audit-2024/packages/contracts/contracts/TellerV2.sol#L524-L558

Tool used

Manual Review

Recommendation

Cache the market fee at the time of bid submission.

Duplicate of #125

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

samuraii77 - A market owner can put borrowers in a very unfavorable position and steal money out of lenders #216

A market owner can put borrowers in a very unfavorable position and steal money out of lenders

Summary

Vulnerability Detail

Impact

Code Snippet

Tool used

Recommendation