# Audit Recap #4: Teller
*We regularly publish short recaps on a decentralized audit in which we participated. This time, we cover the audit of [Teller](https://teller.org).*


*brainbot is a web3 service provider, offering consulting and development services as well as [smart contract audits](https://brainbot.com/smart-contract-audits/). To gain more experience in auditing, our security researchers regularly participate in [decentralized audits](https://medium.com/@brainbot/decentralised-audits-are-here-to-stay-bcee6d1118a8). In this series, we will publish recaps of audits in which we participated in order to provide some insight into the functioning, the smart contract architecture and our findings for the respective protocols.*

## Teller V2
The goal of this content is to provide an insight of TellerV2 as audited on Sherlock as well as some interesting findings on the protocol.

TellerV2 is an open order-book lending / borrowing protocol. Market owners deploy new markets specifying parameters for the borrowers and lenders of the new market. Different markets should be used for different type of credits (e.g. consumer credits, crypto investment loans, etc …).

Loans can be collateralized or not. Only ERC20 tokens can be used as loan principal. However, ERC20, ERC721, and ERC1155 tokens can be used as collateral. When a borrower does not repay their loan in time, they can be liquidated by either the borrower who will forfeit its loan and receive the collateral or by a liquidator who will repay the loan in the name of the borrower to receive the collateral in place of the lender.

## Scope
The scope of the audit includes the main contract for lending / borrowing (`TellerV2.sol`), the contracts for holding collateral of loans (`CollateralManager.sol` and `CollateralEscrowV1.sol`), a contract used to tokenize a loan and allow its transfer (`LenderManager.sol`), a contract for lender to commit to accepting any loan that satisfy certain conditions (`LenderCommitmentForwarder.sol`), and a contract to allow context forwarding in order to execute a loan / borrow request in the name of another user (`TellerV2Context.sol`).

It does not include contracts responsible for market creation and update of market parameters, calculations of loan repayments, protocol fees, the reputation system to keep track of bad borrowers, or the contract used to reward lenders of a loan. However, some of these contracts play an important role in the main logic contract `TellerV2` and the interactions in between in-scope contracts and out-of-scope contracts should still be investigated.

### submitBid()

The entry point to the protocol is through one of the `TellerV2.submitBid()` functions, it is called by a borrower to make a loan request. The parameters of the function are the following:

- `_lendingToken`: the address of the ERC20 principal token
- `_marketplaceId`: the id of the market place in which the loan is requested
- `_principal`: the amount of principal requested
- `_duration`: the duration in seconds of the loan
- `_APR`: the interest rate for the loan
- `_metadataURI`: URI of metadata used by lender to check if they want to accept the loan
- `_collateralInfo`: an array of collateral specifying address, type, id (for ERC721 and ERC1155), and amount (not necessary for uncollateralized loans)

The parameters for loan repayment cycle and loan default duration are taken from the `marketRegistry` using the provided `_marketplaceId`.

The function also checks that the borrower address is accepted by the market registry. This can be used to restrain loan application to KYC'd addresses.

When a user calls this function, they commit to a loan with selected parameters. `TellerV2` calls `collateralManager.commitCollateral()` to commit collateral for the user. Collateral is not yet withdrawn from the user. It is only withdrawn and put into escrow when the loan is accepted.

### acceptBid()

Once a loan has been committed via `submitBid()`, lenders can call `TellerV2.lenderAcceptBid(_bidId)` to accept the loan. The address of lender is checked by the market registry to verify the lender is accepted by the market owner. The loan is marked as accepted and the timestamp for the accepted time and last repayment time are set to the current block timestamp. Collateral is taken from the user via `collateralManager.deployAndDeposit()`, if the borrower spent the collateral they committed to in between borrow request and loan creation, the function will revert. The function then pays protocol and market owner fees and transfer the loan principal minus the fees to the borrower.

### repayLoan()

There are three functions to repay a loan: `repayLoanMinimum()` repays the minimum amount due on a loan, `repayLoanFull()` fully repays the loan and withdraws the collateral to the borrower, and `repayLoan()` repays a custom amount of the loan in between minimum and full repayment. The functions all call the internal `_repayLoan()` which calculates the interests to be paid out, repays the amount of principal specified to the lender plus the interests, withdraws the collateral if necessary, updates the last repaid timestamp, and updates the reputation of the borrower via `reputationManager.updateAccountReputation()`.

### liquidation

If a loan has not been repaid for a period longer than its default duration (taken from the market registry at loan request) it is liquidateable. There is a grace period of 24 hours during which only the lender can liquidate the loan to receive the collateral. After this period, anyone can call `liquidateLoanFull()` to repay the borrower's debt and withdraw its collateral.

### CollateralManager

The `CollateralManager` exposes a function to commit collateral, which is called by `TellerV2` when a user requests a loan with collateral. It also exposes `deployAndDeposit()` called by `TellerV2` when a loan is accepted to deploy an escrow contract and deposit the collateral of the borrower into the escrow. Lastly, it allows for withdrawal of collateral to the appropriate address. The withdrawal is only possible when the loan is repaid or defaulted. The collateral will be sent to the borrower when loan has been repaid or to the liquidator / lender when loan is defaulted.

The `CollateralEscrowV1` contract is a simple contract that can store ERC20, ERC721, ERC1155 and let the collateral manager withdraw the held tokens.

## Findings

In this section I’ll describe a couple of interesting vulnerabilities found for this audit.

### Lack of access control in commitCollateral

The function `CollateralManager.commitCollateral()` used by `TellerV2` when a borrower requests a loan to commit collateral does not have any access control. It takes as input the bid ID and the collateral info and commits the borrower of bid ID to the input collateral.

Anyone can call this function to commit any amount of any collateral for any bid ID at any time.

When the loan is accepted, the committed to collateral will be taken from the user and used as collateral in an escrow.

    function commitCollateral(
        uint256 _bidId,
        Collateral[] calldata _collateralInfo
    ) public returns (bool validation_) {
        address borrower = tellerV2.getLoanBorrower(_bidId);
        (validation_, ) = checkBalances(borrower, _collateralInfo);

        if (validation_) {
            for (uint256 i; i < _collateralInfo.length; i++) {
                Collateral memory info = _collateralInfo[i];
                _commitCollateral(_bidId, info);
            }
        }
    }

This can result in a myriad of different attacks: anyone (including an evil lender) can increase the collateral committed by the borrower before accepting a loan to receive more collateral in case of liquidation, anyone can increase committed collateral past the value held by the borrower to prevent the loan from ever being accepted (the accepting transaction will revert when attempting to withdraw the collateral from the borrower), or the borrower can lower its collateral to zero right before the loan is accepted.

Multiple auditors (including myself) submitted multiple issues for the different angles of attack resulting from this same single lack of access control. The judges considered most angles of attacks as separate vulnerabilities and rewarded submitters for the creative ways to exploit the flaw.

I personally believe that every attacks rooting from the same problem should be treated as one issue to encourage finding root causes and fixing problems rather than encourage wasting time on finding all the ways a vulnerability could be exploited.

The different valid issues exploiting `commitCollateral()` are:
- [#170](https://github.com/sherlock-audit/2023-03-teller-judging/issues/170) `commitCollateral()` can be called by borrower to front-run bid acceptance and remove their collateral
- [#169](https://github.com/sherlock-audit/2023-03-teller-judging/issues/169) `commitCollateral()` can be called by anyone before the bid is accepted to add a malicious token that is paused after the loan is accepted to prevent withdrawal of collateral
- [#168](https://github.com/sherlock-audit/2023-03-teller-judging/issues/168) `commitCollateral()` can be called on an active loan by the borrower to add a malicious token that can be paused to prevent withdrawal of collateral

### Cannot repay a loan when lender is blacklisted

The internal function `TellerV2._repayLoan()` attempts to transfer the loan token back to the lender. If the loan token implements a blacklist like the common USDC token, the transfer may be impossible and the repayment will fail.

    function _repayLoan(...) internal virtual {
        ...
        bid.loanDetails.lendingToken.safeTransferFrom(
            _msgSenderForMarket(bid.marketplaceId),
            lender,
            paymentAmount
        );
        ...
    }

During repayment the loan lender is computed by:

    function getLoanLender(uint256 _bidId)
        public
        view
        returns (address lender_)
    {
        lender_ = bids[_bidId].lender;

        if (lender_ == address(lenderManager)) {
            return lenderManager.ownerOf(_bidId);
        }
    }

If the lender controls a blacklisted address, they can use the lenderManager to selectively transfer the loan to / from the blacklisted whenever they want.

Any lender can prevent repayment of a loan and its liquidation. In particular, a lender can wait until a loan is almost completely repaid, transfer the loan to a blacklisted address (even one they do not control) to prevent the loan to be fully repaid / liquidated. The loan will default and borrower will not be able to withdraw their collateral. If the lender controls a blacklisted address, they can additionally withdraw the collateral of the user.

## What's next?
We'll continue to regularly publish audit recaps for different protocols. Meanwhile, you can also have a look at our other publications on [Medium](https://medium.com/@brainbot).

You can also find our previous audit recaps on our [overview page](https://hackmd.io/@brainbot-services).

## Hard Facts
**Decentralized Audit Platform:** [Sherlock](https://www.sherlock.xyz)
**Audited Protocol:** [Taurus Protocol](https://app.sherlock.xyz/audits/contests/62)
**Security Researcher:** Côme du Crest (ranked 2/230 in this contest)