Debt DAO contest - rbserver's results

A cryptonative credit marketplace for fully anon and trustless loans to DAOs.

General Information

Platform: Code4rena

Start Date: 03/11/2022

Pot Size: $115,500 USDC

Total HM: 17

Participants: 120

Period: 7 days

Judge: LSDan

Total Solo HM: 1

Id: 174

League: ETH

Debt DAO

Findings Distribution

Researcher Performance

Rank: 33/120

Findings: 4

Award: $260.18

QA:
grade-b

🌟 Selected for report: 0

🚀 Solo Findings: 0

Awards

8.0811 USDC - $8.08

Labels

bug
2 (Med Risk)
satisfactory
duplicate-39

External Links

Lines of code

https://github.com/debtdao/Line-of-Credit/blob/audit/code4rena-2022-11-03/contracts/utils/LineLib.sol#L59-L74 https://github.com/debtdao/Line-of-Credit/blob/audit/code4rena-2022-11-03/contracts/modules/credit/LineOfCredit.sol#L223-L244

Vulnerability details

Impact

Lending and repaying ETH by calling functions like the LineOfCredit.addCredit function will call the LineLib.receiveTokenOrETH function. When calling the LineLib.receiveTokenOrETH function for ETH and a specified ETH amount, if the user accidentally sends more ETH than the specified amount, the extra ETH amount will be locked in the line of credit contract; this is because that there is no other way to withdraw such extra ETH amount from the contract. This is possible especially for the non-technical users who rely on a front-end application to call the LineLib.receiveTokenOrETH function since there is no guarantee that such front-end application will not malfunction or become compromised; when such front-end application malfunctions or becomes compromised, an amount that is more than the specified ETH amount when calling the LineLib.receiveTokenOrETH function can be sent and deducted from the user's ETH balance without her or his consent. The affected user basically loses the extra ETH amount that is sent since it is locked in the line of credit contract.

https://github.com/debtdao/Line-of-Credit/blob/audit/code4rena-2022-11-03/contracts/utils/LineLib.sol#L59-L74

    function receiveTokenOrETH(
      address token,
      address sender,
      uint256 amount
    )
      external
      returns (bool)
    {
        if(token == address(0)) { revert TransferFailed(); }
        if(token != Denominations.ETH) { // ERC20
            ...
        } else { // ETH
            if(msg.value < amount) { revert TransferFailed(); }
        }
        return true;
    }

https://github.com/debtdao/Line-of-Credit/blob/audit/code4rena-2022-11-03/contracts/modules/credit/LineOfCredit.sol#L223-L244

    function addCredit(
        uint128 drate,
        uint128 frate,
        uint256 amount,
        address token,
        address lender
    )
        external
        payable
        override
        whileActive
        mutualConsent(lender, borrower)
        returns (bytes32)
    {
        LineLib.receiveTokenOrETH(token, lender, amount);

        bytes32 id = _createCredit(lender, token, amount);

        require(interestRate.setRate(id, drate, frate));
        
        return id;
    }

Proof of Concept

Please add the following test in contracts\tests\LineOfCredit.t.sol. This test will pass to demonstrate the described scenario.

    function test_extraETHAmountAccidentallySentWhenLendingETHIsLockedInLineOfCreditContract() public {
        vm.prank(borrower);
        line.addCredit(dRate, fRate, 1 ether, Denominations.ETH, lender);

        // before lender calls the addCredit function, the line of credit contract owns 0 ETH
        assertEq(address(line).balance, 0);

        vm.startPrank(lender);

        // lender accidentally sends 2 ETH when calling the addCredit function for lending up to 1 ETH
        bytes32 id = line.addCredit{value: 2 ether}(dRate, fRate, 1 ether, Denominations.ETH, lender);

        // the line of credit contract owns 2 ETH at this moment
        assertEq(address(line).balance, 2 ether);

        // lender is not able to call the withdraw function with an amount input that is larger than the deposited ETH amount
        vm.expectRevert(ILineOfCredit.NoLiquidity.selector);
        line.withdraw(id, 1 ether + 1);

        // calling the withdraw function with the amount input being the deposited ETH amount does not withdraw the extra 1 ETH that was accidentally sent
        line.withdraw(id, 1 ether);
        assertEq(address(line).balance, 1 ether);

        // calling the close function also does not withdraw the extra 1 ETH that was accidentally sent
        line.close(id);
        assertEq(address(line).balance, 1 ether);

        vm.stopPrank();
    }

Tools Used

VSCode

https://github.com/debtdao/Line-of-Credit/blob/audit/code4rena-2022-11-03/contracts/utils/LineLib.sol#L70-L72 can be updated to the following code.

        } else { // ETH
            if(msg.value != amount) { revert TransferFailed(); }
        }

#0 - c4-judge

2022-11-17T15:56:49Z

dmvt marked the issue as duplicate of #25

#1 - c4-judge

2022-12-06T15:08:35Z

dmvt marked the issue as satisfactory

#2 - C4-Staff

2022-12-20T06:44:54Z

liveactionllama marked the issue as duplicate of #39

Findings Information

Labels

bug
2 (Med Risk)
satisfactory
duplicate-355

Awards

141.941 USDC - $141.94

External Links

Lines of code

https://github.com/debtdao/Line-of-Credit/blob/audit/code4rena-2022-11-03/contracts/utils/LineLib.sol#L59-L74 https://github.com/debtdao/Line-of-Credit/blob/audit/code4rena-2022-11-03/contracts/modules/credit/LineOfCredit.sol#L223-L244

Vulnerability details

Impact

The LineLib.receiveTokenOrETH function is called when calling functions like the LineOfCredit.addCredit function for lending and repaying amounts of an ERC20 token. When calling the LineLib.receiveTokenOrETH function for an ERC20 token, if the user accidentally sends some ETH, such ETH amount will be locked in the line of credit contract since there is no other way to withdraw it from the contract. This could happen especially for the non-technical users who rely on a front-end application to call such function; if the front-end application malfunctions or becomes compromised, some of the user's ETH balance can be sent without her or his consent when calling such function. Because the sent ETH amount is locked in the line of credit contract, the affected user essentially loses it.

https://github.com/debtdao/Line-of-Credit/blob/audit/code4rena-2022-11-03/contracts/utils/LineLib.sol#L59-L74

    function receiveTokenOrETH(
      address token,
      address sender,
      uint256 amount
    )
      external
      returns (bool)
    {
        if(token == address(0)) { revert TransferFailed(); }
        if(token != Denominations.ETH) { // ERC20
            IERC20(token).safeTransferFrom(sender, address(this), amount);
        } else { // ETH
            ...
        }
        return true;
    }

https://github.com/debtdao/Line-of-Credit/blob/audit/code4rena-2022-11-03/contracts/modules/credit/LineOfCredit.sol#L223-L244

    function addCredit(
        uint128 drate,
        uint128 frate,
        uint256 amount,
        address token,
        address lender
    )
        external
        payable
        override
        whileActive
        mutualConsent(lender, borrower)
        returns (bytes32)
    {
        LineLib.receiveTokenOrETH(token, lender, amount);

        bytes32 id = _createCredit(lender, token, amount);

        require(interestRate.setRate(id, drate, frate));
        
        return id;
    }

Proof of Concept

Please add the following test in contracts\tests\LineOfCredit.t.sol. This test will pass to demonstrate the described scenario.

    function test_ETHAccidentallySentWhenLendingERC20TokenIsLockedInLineOfCreditContract() public {
        vm.prank(borrower);
        line.addCredit(dRate, fRate, 1 ether, address(supportedToken1), lender);
        
        // before lender calls the addCredit function, the line of credit contract owns 0 ETH
        assertEq(address(line).balance, 0);

        vm.startPrank(lender);

        // lender accidentally sends 1 ETH when calling the addCredit function for lending supportedToken1 that is an ERC20 token
        bytes32 id = line.addCredit{value: 1 ether}(dRate, fRate, 1 ether, address(supportedToken1), lender);

        // the line of credit contract owns 1 ETH at this moment
        assertEq(address(line).balance, 1 ether);

        // lender is not able to call the withdraw function with an amount input that is larger than the deposited amount for supportedToken1
        vm.expectRevert(ILineOfCredit.NoLiquidity.selector);
        line.withdraw(id, 1 ether + 1);

        // calling the withdraw function with the amount input being the deposited amount for supportedToken1 does not withdraw the 1 ETH that was accidentally sent
        line.withdraw(id, 1 ether);
        assertEq(address(line).balance, 1 ether);

        // calling the close function also does not withdraw the 1 ETH that was accidentally sent
        line.close(id);
        assertEq(address(line).balance, 1 ether);

        vm.stopPrank();
    }

Tools Used

VSCode

https://github.com/debtdao/Line-of-Credit/blob/audit/code4rena-2022-11-03/contracts/utils/LineLib.sol#L68-L69 can be updated to the following code.

        if(token != Denominations.ETH) { // ERC20
            require(msg.value == 0, "Should only receive ERC20");
            IERC20(token).safeTransferFrom(sender, address(this), amount);

#0 - c4-judge

2022-11-17T15:51:59Z

dmvt marked the issue as duplicate of #25

#1 - c4-judge

2022-11-17T19:30:18Z

dmvt marked the issue as nullified

#2 - c4-judge

2022-12-06T15:11:13Z

dmvt marked the issue as not nullified

#3 - c4-judge

2022-12-06T15:11:18Z

dmvt marked the issue as not a duplicate

#4 - c4-judge

2022-12-06T15:12:06Z

dmvt marked the issue as duplicate of #89

#5 - c4-judge

2022-12-06T17:41:52Z

dmvt marked the issue as satisfactory

#6 - C4-Staff

2022-12-20T06:05:21Z

liveactionllama marked the issue as duplicate of #355

Findings Information

Awards

48.8098 USDC - $48.81

Labels

bug
2 (Med Risk)
satisfactory
edited-by-warden
duplicate-367

External Links

Lines of code

https://github.com/debtdao/Line-of-Credit/blob/audit/code4rena-2022-11-03/contracts/modules/credit/LineOfCredit.sol#L223-L244 https://github.com/debtdao/Line-of-Credit/blob/audit/code4rena-2022-11-03/contracts/modules/credit/LineOfCredit.sol#L435-L454 https://github.com/debtdao/Line-of-Credit/blob/audit/code4rena-2022-11-03/contracts/utils/CreditLib.sol#L125-L161 https://github.com/debtdao/Line-of-Credit/blob/audit/code4rena-2022-11-03/contracts/modules/credit/LineOfCredit.sol#L340-L367

Vulnerability details

Impact

This protocol's current implementation does not support the usage of rebasing tokens, which can have valid Chainlink oracles for their USD pairs, such as AMPL / USD. For example, after the lender agrees to lend a rebasing token to the borrower, both the lender and borrower call the LineOfCredit.addCredit function for this rebasing token. Calling the LineOfCredit.addCredit function eventually calls the CreditLib.create function, which sets the credit's deposit provided by the lender. Later, the borrower can call the LineOfCredit.borrow function to borrow from the deposited amount for this rebasing token. After the LineOfCredit.addCredit function is called and before the LineOfCredit.borrow function is called, it is possible that this rebasing token's rebasing event occurs, which decreases its total supply. After such rebasing event has happened, the line of credit contract's balance of this rebasing token can become insufficient for lending all of the deposited amount; hence, calling the LineOfCredit.borrow function to borrow all of the deposited amount would revert, and the borrower is unable to borrow such amount that was agreed by her or him and the lender.

https://github.com/debtdao/Line-of-Credit/blob/audit/code4rena-2022-11-03/contracts/modules/credit/LineOfCredit.sol#L223-L244

    function addCredit(
        uint128 drate,
        uint128 frate,
        uint256 amount,
        address token,
        address lender
    )
        external
        payable
        override
        whileActive
        mutualConsent(lender, borrower)
        returns (bytes32)
    {
        LineLib.receiveTokenOrETH(token, lender, amount);

        bytes32 id = _createCredit(lender, token, amount);

        require(interestRate.setRate(id, drate, frate));
        
        return id;
    }

https://github.com/debtdao/Line-of-Credit/blob/audit/code4rena-2022-11-03/contracts/modules/credit/LineOfCredit.sol#L435-L454

    function _createCredit(
        address lender,
        address token,
        uint256 amount
    )
        internal
        returns (bytes32 id)
    {
        id = CreditLib.computeId(address(this), lender, token);
        // MUST not double add the credit line. otherwise we can not _close()
        if(credits[id].lender != address(0)) { revert PositionExists(); }

        credits[id] = CreditLib.create(id, amount, lender, token, address(oracle));

        ids.push(id); // add lender to end of repayment queue
        
        unchecked { ++count; }

        return id;
    }

https://github.com/debtdao/Line-of-Credit/blob/audit/code4rena-2022-11-03/contracts/utils/CreditLib.sol#L125-L161

  function create(
      bytes32 id,
      uint256 amount,
      address lender,
      address token,
      address oracle
  )
      external 
      returns(ILineOfCredit.Credit memory credit)
  {
      int price = IOracle(oracle).getLatestAnswer(token);
      if(price <= 0 ) { revert NoTokenPrice(); }

      uint8 decimals;
      if(token == Denominations.ETH) {
          decimals = 18;
      } else {
          (bool passed, bytes memory result) = token.call(
              abi.encodeWithSignature("decimals()")
          );
          decimals = !passed ? 18 : abi.decode(result, (uint8));
      }

      credit = ILineOfCredit.Credit({
          lender: lender,
          token: token,
          decimals: decimals,
          deposit: amount,
          principal: 0,
          interestAccrued: 0,
          interestRepaid: 0
      });

      emit AddCredit(lender, token, amount, id);

      return credit;
  }

https://github.com/debtdao/Line-of-Credit/blob/audit/code4rena-2022-11-03/contracts/modules/credit/LineOfCredit.sol#L340-L367

    function borrow(bytes32 id, uint256 amount)
        external
        override
        whileActive
        onlyBorrower
        returns (bool)
    {
        Credit memory credit = _accrue(credits[id], id);

        if(amount > credit.deposit - credit.principal) { revert NoLiquidity(); }

        credit.principal += amount;

        credits[id] = credit; // save new debt before healthcheck

        // ensure that borrowing doesnt cause Line to be LIQUIDATABLE
        if(_updateStatus(_healthcheck()) != LineLib.STATUS.ACTIVE) { 
            revert NotActive();
        }

        LineLib.sendOutTokenOrETH(credit.token, borrower, amount);

        emit Borrow(id, amount);

        _sortIntoQ(id);

        return true;
    }

Proof of Concept

First, please add contracts\mock\MockRebasingToken.sol as follows.

// SPDX-License-Identifier: UNLICENSED

pragma solidity ^0.8.0;

contract MockRebasingToken {
    uint256 private constant INITIAL_SUPPLY_ = 100_000_000 * 10**18;
    uint256 private constant TOTAL_ = type(uint256).max - (type(uint256).max % INITIAL_SUPPLY_);

    uint256 private _totalSupply;
    uint256 private mul_;
    mapping(address => uint256) private balance_;
    constructor() {
        _totalSupply = INITIAL_SUPPLY_;
        balance_[msg.sender] = TOTAL_;
        mul_ = TOTAL_ / _totalSupply;
    }

    function balanceOf(address _addr) public view returns (uint256) {
        return balance_[_addr] / mul_;
    }

    function transfer(address _to, uint256 _value) public returns (bool) {
        uint256 value_ = _value * mul_;

        balance_[msg.sender] = balance_[msg.sender] - value_;
        balance_[_to] = balance_[_to] + value_;

        return true;
    }

    function transferFrom(address _from, address _to, uint256 _value) external returns (bool) {
        uint256 value_ = _value * mul_;

        balance_[_from] = balance_[_from] - value_;
        balance_[_to] = balance_[_to] + value_;

        return true;
    }

    function rebase(int256 _supplyDelta) external returns (uint256) {
        if (_supplyDelta == 0) {
            return _totalSupply;
        }

        if (_supplyDelta < 0) {
            _totalSupply = _totalSupply - uint256(_supplyDelta * (-1));
        } else {
            _totalSupply = _totalSupply + uint256(_supplyDelta);
        }

        mul_ = TOTAL_ / _totalSupply;

        return _totalSupply;
    }
}

Then, please add the following code in contracts\tests\LineOfCredit.t.sol.

  1. Import MockRebasingToken.sol as follows.
import "../mock/MockRebasingToken.sol";
  1. Add the following test. This test will pass to demonstrate the described scenario.
    function test_failToBorrowRebasingToken() public {
        // deploy the mock rebasing token
        vm.prank(lender);
        MockRebasingToken mockRebasingToken = new MockRebasingToken();

        SimpleOracle oracle_ = new SimpleOracle(address(mockRebasingToken), address(supportedToken2));

        LineOfCredit line_ = new LineOfCredit(
            address(oracle_),
            arbiter,
            borrower,
            ttl
        );
        assertEq(uint(line_.init()), uint(LineLib.STATUS.ACTIVE));

        uint256 amount = 10 ether;

        // lender agrees to lend up to 10 ether of the mock rebasing token to borrower
        vm.prank(borrower);
        line_.addCredit(dRate, fRate, amount, address(mockRebasingToken), lender);

        vm.prank(lender);
        bytes32 id = line_.addCredit(dRate, fRate, amount, address(mockRebasingToken), lender);

        // After the addCredit function is called by both borrower and lender and before borrower calls the borrow function,
        //   the mock rebasing token's rebasing event occurs, which decreases its total supply.
        mockRebasingToken.rebase(-10000000000);

        // Calling the borrow function by borrower reverts at this moment
        //   because the line of credit contract's balance of the mock rebasing token has become insufficient after such rebasing event has happened.
        vm.prank(borrower);
        vm.expectRevert(stdError.arithmeticError);
        line_.borrow(id, amount);
    }

Tools Used

VSCode

A blocklist can be used to block the usage of rebasing tokens, which is similar to what have been done in some other protocols. If blocking rebasing tokens, the documentation needs to be updated so users are aware of this.

#0 - c4-judge

2022-11-17T15:51:37Z

dmvt marked the issue as duplicate of #26

#1 - c4-judge

2022-12-06T16:40:52Z

dmvt marked the issue as satisfactory

#2 - C4-Staff

2022-12-20T06:01:34Z

liveactionllama marked the issue as duplicate of #367

[L-01] POSSIBLE DOS WHEN CALLING LineLib.sendOutTokenOrETH FUNCTION FOR ETH

Calling the following LineLib.sendOutTokenOrETH function for ETH executes payable(receiver).transfer(amount). Since the transfer function only forwards a fixed 2300 gas stipend, if the to address corresponds to a contract that has gas-intensive logics, which need more than 2300 gas, in its receive function, executing payable(receiver).transfer(amount) will revert. Functions like LineOfCredit.withdraw and LineOfCredit._close would call the LineLib.sendOutTokenOrETH function with the receiver input being credit.lender, which was set when the credit was created and cannot be changed; if credit.lender happens to be a contract that has gas-intensive logics, which need more than 2300 gas, in its receive function, calling the LineLib.sendOutTokenOrETH function will revert and DOS will occur. As a result, the receiver address, such as credit.lender, for the LineLib.sendOutTokenOrETH function cannot receive the specified amount of the corresponding token.

https://github.com/debtdao/Line-of-Credit/blob/audit/code4rena-2022-11-03/contracts/utils/LineLib.sol#L34-L51

    function sendOutTokenOrETH(
      address token,
      address receiver,
      uint256 amount
    )
      external
      returns (bool)
    {
        if(token == address(0)) { revert TransferFailed(); }
        
        // both branches revert if call failed
        if(token!= Denominations.ETH) { // ERC20
            IERC20(token).safeTransfer(receiver, amount);
        } else { // ETH
            payable(receiver).transfer(amount);
        }
        return true;
    }

https://github.com/debtdao/Line-of-Credit/blob/audit/code4rena-2022-11-03/contracts/modules/credit/LineOfCredit.sol#L370-L385

    function withdraw(bytes32 id, uint256 amount)
        external
        override
        returns (bool)
    {
        Credit memory credit = credits[id];

        ...

        LineLib.sendOutTokenOrETH(credit.token, credit.lender, amount);

        return true;
    }

https://github.com/debtdao/Line-of-Credit/blob/audit/code4rena-2022-11-03/contracts/modules/credit/LineOfCredit.sol#L483-L507

    function _close(Credit memory credit, bytes32 id) internal virtual returns (bool) {
        ...
        if (credit.deposit + credit.interestRepaid > 0) {
            LineLib.sendOutTokenOrETH(
                credit.token,
                credit.lender,
                credit.deposit + credit.interestRepaid
            );
        }

        ...
    }

https://github.com/debtdao/Line-of-Credit/blob/audit/code4rena-2022-11-03/contracts/utils/LineLib.sol#L47-L49 can be updated to the following code.

        } else { // ETH
            (bool success, ) = receiver.call{value: amount}("");
            require(success, "ETH transfer failed");
        }

[L-02] MISSING TWO-STEP PROCEDURES FOR SETTING escrow line, spigot owner, and spigot operator

The following Escrow.updateLine, Spigot.updateOwner, and Spigot.updateOperator functions can be called to set the escrow line and the spigot owner and operator. Because these functions do not include a two-step procedure, it is possible to assign these roles to wrong addresses. When this occurs, the functions that are only callable by these roles can become no-ops or be maliciously called if the incorrectly assigned addresses happen to be malicious. To reduce the potential attack surface, please consider implementing a two-step procedure for assigning each of these roles.

https://github.com/debtdao/Line-of-Credit/blob/audit/code4rena-2022-11-03/contracts/modules/escrow/Escrow.sol#L74-L76

    function updateLine(address _line) external returns(bool) {
      return state.updateLine(_line);
    }

https://github.com/debtdao/Line-of-Credit/blob/audit/code4rena-2022-11-03/contracts/modules/spigot/Spigot.sol#L159-L161

    function updateOwner(address newOwner) external returns (bool) {
        return state.updateOwner(newOwner);
    }

https://github.com/debtdao/Line-of-Credit/blob/audit/code4rena-2022-11-03/contracts/modules/spigot/Spigot.sol#L170-L172

    function updateOperator(address newOperator) external returns (bool) {
        return state.updateOperator(newOperator);
    }

[L-03] MISSING address(0) CHECKS FOR CRITICAL ADDRESS INPUTS

To prevent unintended behaviors, critical addresses inputs should be checked against address(0).

Please consider checking _escrow in the following constructor. https://github.com/debtdao/Line-of-Credit/blob/audit/code4rena-2022-11-03/contracts/modules/credit/EscrowedLine.sol#L16-L18

  constructor(address _escrow) {
    escrow = IEscrow(_escrow);
  }

Please consider checking oracle_, arbiter_, and borrower_ in the following constructor. https://github.com/debtdao/Line-of-Credit/blob/audit/code4rena-2022-11-03/contracts/modules/credit/LineOfCredit.sol#L49-L62

    constructor(
        address oracle_,
        address arbiter_,
        address borrower_,
        uint256 ttl_
    ) {
        oracle = IOracle(oracle_);
        arbiter = arbiter_;
        borrower = borrower_;
        deadline = block.timestamp + ttl_;  //the deadline is the term/maturity/expiry date of the Line of Credit facility
        interestRate = new InterestRateCredit();

        emit DeployLine(oracle_, arbiter_, borrower_);
    }

Please consider checking spigot_ in the following constructor. https://github.com/debtdao/Line-of-Credit/blob/audit/code4rena-2022-11-03/contracts/modules/credit/SpigotedLine.sol#L53-L67

    constructor(
        address oracle_,
        address arbiter_,
        address borrower_,
        address spigot_,
        address payable swapTarget_,
        uint256 ttl_,
        uint8 defaultRevenueSplit_
    ) LineOfCredit(oracle_, arbiter_, borrower_, ttl_) {
        require(defaultRevenueSplit_ <= SpigotedLineLib.MAX_SPLIT);

        spigot = ISpigot(spigot_);
        defaultRevenueSplit = defaultRevenueSplit_;
        swapTarget = swapTarget_;
    }

Please consider checking _oracle, _line, and _borrower in the following constructor. https://github.com/debtdao/Line-of-Credit/blob/audit/code4rena-2022-11-03/contracts/modules/escrow/Escrow.sol#L42-L52

    constructor(
        uint32 _minimumCollateralRatio,
        address _oracle,
        address _line,
        address _borrower
    ) {
        minimumCollateralRatio = _minimumCollateralRatio;
        oracle = _oracle;
        borrower = _borrower;
        state.line = _line;
    }

Please consider checking _registry in the following constructor. https://github.com/debtdao/Line-of-Credit/blob/audit/code4rena-2022-11-03/contracts/modules/oracle/Oracle.sol#L15-L17

    constructor(address _registry) {
        registry = FeedRegistryInterface(_registry);
    }

Please consider checking _owner, _treasury, and _operator in the following constructor. https://github.com/debtdao/Line-of-Credit/blob/audit/code4rena-2022-11-03/contracts/modules/spigot/Spigot.sol#L31-L39

    constructor (
        address _owner,
        address _treasury,
        address _operator
    ) {
        state.owner = _owner;
        state.operator = _operator;
        state.treasury = _treasury;
    }

[L-04] UNRESOLVED TODO/QUESTION COMMENT

A comment regarding todo/question indicates that there is an unresolved action item for implementation, which needs to be addressed before the protocol deployment. Please review the todo/question comment in the following code.

https://github.com/debtdao/Line-of-Credit/blob/audit/code4rena-2022-11-03/contracts/utils/SpigotLib.sol#L105-L122

    function claimEscrow(SpigotState storage self, address token)
        external
        returns (uint256 claimed) 
    {
        ...

        self.escrowed[token] = 0; // keep 1 in escrow for recurring call gas optimizations?

        ...
    }

[N-01] COMMENTED OUT CODE CAN BE REMOVED

The following line of code is commented out. To improve readability and maintainability, please consider removing it.

https://github.com/debtdao/Line-of-Credit/blob/audit/code4rena-2022-11-03/contracts/modules/credit/EscrowedLine.sol#L10

// import { SecuredLine } from "./SecuredLine.sol";

[N-02] CONSTANTS CAN BE USED INSTEAD OF MAGIC NUMBERS

To improve readability and maintainability, constants can be used instead of magic numbers. Please consider replacing the following magic numbers, such as 18, with constants.

contracts\utils\CreditLib.sol
  140: decimals = 18;  

contracts\utils\EscrowLib.sol
  113: deposit.assetDecimals = 18; 
  137: deposit.assetDecimals = 18; 

contracts\utils\SpigotLib.sol
  90: uint256 escrowedAmount = claimed * self.settings[revenueContract].ownerSplit / 100; 

[N-03] MISSING REASON STRINGS IN require STATEMENTS

When the reason strings are missing in the require statements, it is unclear about why certain conditions revert. Please add descriptive reason strings for the following require statements.

contracts\modules\credit\EscrowedLine.sol
  64: require(escrow_.liquidate(amount, targetToken, to));
  90: require(escrow.updateLine(newLine));  

contracts\modules\credit\LineOfCredit.sol
  112: require(uint(status) >= uint( LineLib.STATUS.ACTIVE));
  241: require(interestRate.setRate(id, drate, frate));    
  259: require(interestRate.setRate(id, drate, frate));    
  326: require(amount <= credit.principal + credit.interestAccrued);   

contracts\modules\credit\SpigotedLine.sol
  62: require(defaultRevenueSplit_ <= SpigotedLineLib.MAX_SPLIT); 
  143: require(amount <= unusedTokens[credit.token]);    
  160: require(msg.sender == borrower);    
  239: require(msg.sender == arbiter); 

contracts\utils\EscrowLib.sol
  91: require(amount > 0);    
  105: require(msg.sender == ILineOfCredit(self.line).arbiter());  
  161: require(amount > 0);    
  198: require(amount > 0);    
  216: require(msg.sender == self.line); 

contracts\utils\SpigotedLineLib.sol
  147: require(ISpigot(spigot).updateOwner(newLine));  

contracts\utils\SpigotLib.sol
  96: require(LineLib.sendOutTokenOrETH(token, self.treasury, claimed - escrowedAmount));
  128: require(revenueContract != address(this));  
  130: require(self.settings[revenueContract].transferOwnerFunction == bytes4(0));  
  155: require(success);   
  180: require(newOwner != address(0));    
  189: require(newOperator != address(0));    
  201: require(newTreasury != address(0));    

[N-04] SPELLING TYPOS IN COMMENTS

swithc can be changed to switch and priviliegad can be changed to privileged in the following comments. https://github.com/debtdao/Line-of-Credit/blob/audit/code4rena-2022-11-03/contracts/modules/credit/EscrowedLine.sol#L84-L85

   * @notice helper function to allow borrower to easily swithc collateral to a new Line after repyment
   *(@dev priviliegad internal function.

itselgf can be changed to itself and priviliegad can be changed to privileged in the following comments. https://github.com/debtdao/Line-of-Credit/blob/audit/code4rena-2022-11-03/contracts/modules/credit/SpigotedLine.sol#L84-L85

      * @notice requires Spigot contract itselgf to be transfered to Arbiter and sold off to a 3rd party before declaring insolvent
      *(@dev priviliegad internal function.

[N-05] MISSING NATSPEC COMMENTS

NatSpec comments provide rich code documentation. NatSpec comments are missing for the following functions. Please consider adding them.

contracts\modules\credit\LineOfCredit.sol
  64: function init() external virtual returns(LineLib.STATUS) {
  69: function _init() internal virtual returns(LineLib.STATUS) {  
  121: function _healthcheck() internal virtual returns (LineLib.STATUS) { 
  157: function _canDeclareInsolvent() internal virtual returns(bool) {    
  167: function _updateOutstandingDebt()    

contracts\modules\credit\SecuredLine.sol
  97: function _healthcheck() internal override(EscrowedLine, LineOfCredit) returns(LineLib.STATUS) { 

contracts\modules\credit\SpigotedLine.sol
  78: function unused(address token) external view returns (uint256) {    

contracts\modules\interest-rate\InterestRateCredit.sol
  42: function _accrueInterest(   

contracts\modules\spigot\Spigot.sol
  41: function owner() external view returns (address) {  
  45: function operator() external view returns (address) {  
  49: function treasury() external view returns (address) {  
  146: function updateOwnerSplit(address revenueContract, uint8 ownerSplit)    
  220: function getSetting(address revenueContract)    

contracts\utils\CreditLib.sol
  73: function getOutstandingDebt(  

contracts\utils\MutualConsent.sol
  38: function _mutualConsent(address _signerOne, address _signerTwo) internal returns(bool) {    
  65: function _getNonCaller(address _signerOne, address _signerTwo) internal view returns(address) {    

contracts\utils\SpigotedLineLib.sol
  120: function trade( 
  151: function canDeclareInsolvent(address spigot, address arbiter) external view returns (bool) {  

contracts\utils\SpigotLib.sol
  29: function _claimRevenue(SpigotState storage self, address revenueContract, address token, bytes calldata data)   

[N-06] INCOMPLETE NATSPEC COMMENTS

NatSpec comments provide rich code documentation. @param and/or @return comments are missing for the following functions. Please consider completing NatSpec comments for them.

contracts\modules\credit\EscrowedLine.sol
  89: function _rollover(address newLine) internal virtual returns(bool) {  

contracts\modules\credit\LineOfCredit.sol
  421: function _updateStatus(LineLib.STATUS status_) internal returns(LineLib.STATUS) {   
  435: function _createCredit( 
  465: function _repay(Credit memory credit, bytes32 id, uint256 amount)   
  483: function _close(Credit memory credit, bytes32 id) internal virtual returns (bool) { 

contracts\modules\credit\SecuredLine.sol
  80: function liquidate( 

contracts\modules\credit\SpigotedLine.sol
  73: function _init() internal virtual override(LineOfCredit) returns(LineLib.STATUS) {  

contracts\modules\escrow\Escrow.sol
  57: function line() external view override returns(address) {   
  74: function updateLine(address _line) external returns(bool) {
  100: function enableCollateral(address token) external returns (bool) {   

contracts\modules\spigot\Spigot.sol
  70: function claimRevenue(address revenueContract, address token, bytes calldata data)  
  108: function operate(address revenueContract, bytes calldata data) external returns (bool) {    
  125: function addSpigot(address revenueContract, Setting memory setting) external returns (bool) {    
  137: function removeSpigot(address revenueContract)    
  159: function updateOwner(address newOwner) external returns (bool) {
  170: function updateOperator(address newOperator) external returns (bool) {
  181: function updateTreasury(address newTreasury) external returns (bool) {    
  194: function updateWhitelistedFunction(bytes4 func, bool allowed) external returns (bool) {    
  206: function getEscrowed(address token) external view returns (uint256) {    
  216: function isWhitelisted(bytes4 func) external view returns(bool) {    

contracts\utils\CreditLib.sol
  125: function create(  
  168: function repay( 
  202: function withdraw(  

contracts\utils\SpigotedLineLib.sol
  169: function updateSplit(address spigot, address revenueContract, LineLib.STATUS status, uint8 defaultSplit) external returns (bool) {  
  194: function releaseSpigot(address spigot, LineLib.STATUS status, address borrower, address arbiter, address to) external returns (bool) { 
  217: function sweep(address to, address token, uint256 amount, LineLib.STATUS status, address borrower, address arbiter) external returns (bool) { 

#0 - c4-judge

2022-12-07T17:56:25Z

dmvt marked the issue as grade-b

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter