Skip to content

Latest commit

 

History

History
198 lines (165 loc) · 6.93 KB

081.md

File metadata and controls

198 lines (165 loc) · 6.93 KB

dingo

medium

[M-01] If marketOwner transfer ownership to address(0), no one will be available to accept bids forever.

Summary

SC: TellerV2.sol, MarketRegistry.sol

Suppose marketOwner who wants abandon his market to confirm his customers(borrowers and lenders) that market settings will not be updated in future (like fee %, _paymentDefaultDuration etc). So marketOwner calls transferMarketOwnership() and put address(0) at Marketregistry.sol.

As a result no one could give loan to borrower by calling lenderAcceptBid() due to the fact that amountToMarketplace will be 0 and almost every ERC20 token contains require(to != address(0), "ERC20: transfer to the zero address"); Code from lenderAcceptBid():

        bid.loanDetails.lendingToken.safeTransferFrom(
            sender,
            marketRegistry.getMarketFeeRecipient(bid.marketplaceId),
            amountToMarketplace
        );

Market could permanently stuck despite of marketFee variable (0 or other).

Vulnerability Detail

Step-by-step test:

  1. Borrower submitBid.
  2. Lender lenderAcceptBid.
  3. Borrower repayLoanFull.
  4. MarketOwner transferOwnership to address(0);
  5. Borrower submitBid
  6. Lender trying to lenderAcceptBid but get error. Run: forge test --match-path tests/TellerV2/M-01.t.sol -vvvv Foundry test:
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
import "forge-std/Test.sol";
import "../../contracts/TellerV2.sol";
import "../tokens/TestERC20Token.sol";
import "./User.sol";

import "../../contracts/EAS/TellerAS.sol";
import "../../contracts/EAS/TellerASEIP712Verifier.sol";
import "../../contracts/EAS/TellerASRegistry.sol";

import {ReputationManager} from "../../contracts/ReputationManager.sol";
import "../../contracts/CollateralManager.sol";
import "../../contracts/LenderManager.sol";
import {MarketRegistry} from "../../contracts/MarketRegistry.sol";
import "../../contracts/LenderCommitmentForwarder.sol";


contract MyTest is Test {

    address public eoa1 = vm.addr(1);
    address public eoa2 = vm.addr(2);
    address public escrow = vm.addr(3);
    TellerV2 tellerV2;
    TestERC20Token lendingToken;
    TestERC20Token lendingTokenZeroDecimals;
    User borrower;
    User lender;
    User receiver;
    User marketOwner;
    User feeRecipient;
    User trustedForwarder;
    MarketRegistry marketRegistry;
    ReputationManager reputationManager;
    CollateralManager collateralManager;
    LenderManager lenderManager;
    LenderCommitmentForwarder lenderCommitmentForwarder;
   
    TellerAS tellerAS;
    TellerASEIP712Verifier tellerASEIP712Verifier;
    TellerASRegistry tellerASRegistry;

    function setUp() public {

        borrower                  = new User();
        lender                    = new User();
        receiver                  = new User();
        marketOwner               = new User();
        feeRecipient              = new User();
        trustedForwarder          = new User(); //~
        tellerV2                  = new TellerV2(address(this)); //trustedForwarder in const
        tellerASEIP712Verifier    = new TellerASEIP712Verifier();
        tellerASRegistry          = new TellerASRegistry();
        tellerAS                  = new TellerAS(tellerASRegistry,tellerASEIP712Verifier);

        marketRegistry            = new MarketRegistry();
        reputationManager         = new ReputationManager();
        collateralManager         = new CollateralManager();
        lenderManager             = new LenderManager(marketRegistry);
        lenderCommitmentForwarder = new LenderCommitmentForwarder(address(tellerV2),address(marketRegistry));

        lendingToken              = new TestERC20Token("Wrapped Ether", "WETH", 1e30, 18);
        lendingTokenZeroDecimals  = new TestERC20Token(
            "Wrapped Ether",
            "WETH",
            1e16,
            0
        );
        deal({token: address(lendingToken), to: address(lender), give: 100e18});
        deal({token: address(lendingToken), to: address(borrower), give: 100e18});
        vm.prank(address(lender));
        lendingToken.approve(address(tellerV2),100e18);
        vm.prank(address(borrower));
        lendingToken.approve(address(tellerV2),100e18);
        //log
        
        reputationManager.initialize(address(tellerV2));
        marketRegistry.initialize(tellerAS);
        collateralManager.initialize(escrow,address(tellerV2));
        tellerV2.initialize(
            10,  //_protocolFee
            address(marketRegistry),
            address(reputationManager),
            address(lenderCommitmentForwarder),
            address(collateralManager),
            address(lenderManager)
        );
    }

    function testStuckMarketAfterOwnerTransf() public {
        
        vm.startPrank(address(marketOwner));
        marketRegistry.createMarket(
            address(marketOwner),
            30 days, 
            10,      //_paymentDefaultDuration 
            1 days,  //_bidExpirationTime
            0,   //feePercent  <<==Fee is 0
            false,    //lenderAttest
            false,    //borrowerAttest
            "mytestURI"
        );
        vm.stopPrank();

       
       vm.prank(address(borrower));
       tellerV2.submitBid(
            address(lendingToken), // lending token
            1, //_marketplaceId
            100, // principal
            365 days, // duration
            20_00, // interest rate 
            "", // metadata URI
            eoa1 // receiver
        );

        vm.prank(address(lender));
        tellerV2.lenderAcceptBid(0);
        
        vm.prank(address(borrower));
        tellerV2.repayLoanFull(0);

        vm.prank(address(borrower));
        tellerV2.submitBid(
            address(lendingToken), // lending token
            1, //_marketplaceId
            100, // principal
            365 days, // duration
            20_00, // interest rate 
            "", // metadata URI
            eoa1 // receiver
        );

        vm.prank(address(marketOwner));
        marketRegistry.transferMarketOwnership(1,address(0));

        vm.prank(address(lender));
        tellerV2.lenderAcceptBid(1);
        
        vm.prank(address(borrower));
        tellerV2.repayLoanFull(1);
    }
}

As a result we see error: image Let's transfer to address(1): image

Impact

Market can stuck forever and no one could issue loans.

Code Snippet

https://github.com/sherlock-audit/2023-03-teller/blob/main/teller-protocol-v2/packages/contracts/contracts/TellerV2.sol#L529 https://github.com/sherlock-audit/2023-03-teller/blob/main/teller-protocol-v2/packages/contracts/contracts/MarketRegistry.sol#L466 https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC20/ERC20.sol#L224

Tool used

Manual review

Recommendation

Add in transferMarketOwnership() require(_newOwner != address(0)).