P2P Loan - Debt DAO

In the LineOfCredit contract, the borrower has two possibilities to pay off the debt: by calling depositAndRepay() and then close() functions, or by calling a single depositAndClose() function.LineOfCredit``depositAndRepay()``close()``depositAndClose()

The assessment revealed that both functions depositAndClose() and depositAndRepay() revert due to integer overflow. The error occurs due to principalUsd parameter subtraction done in _repay function. The principalUsd parameter is supposed to have a non-zero value; however, due to condition check in _createCredit where principal parameter is always 0, the principalUsd parameter is not updated.depositAndClose()``depositAndRepay()``principalUsd``_repay``principalUsd``_createCredit``principal``principalUsd

LineOfCredit.sol

function addCredit(
        uint128 drate,
        uint128 frate,
        uint256 amount,
        address token,
        address lender
    )
        external
        virtual
        override
        whileActive
        mutualConsent(lender, borrower)
        returns (bytes32)
    {
        bool success = IERC20(token).transferFrom(
            lender,
            address(this),
            amount
        );
        require(success, "Loan: no tokens to lend");

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

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

        return id;
    }

LineOfCredit.sol

  function _createCredit(
        address lender,
        address token,
        uint256 amount,
        uint256 principal
    ) internal returns (bytes32 id) {
        id = LoanLib.computePositionId(address(this), lender, token);

        // MUST not double add position. otherwise we can not _close()
        require(
            credits[id].lender == address(0),
            "Loan: position exists"
        );

        (bool passed, bytes memory result) = token.call(
            abi.encodeWithSignature("decimals()")
        );
        uint8 decimals = !passed ? 18 : abi.decode(result, (uint8));

        uint256 value = LoanLib.getValuation(oracle, token, amount, decimals);
        require(value > 0 , "Loan: token cannot be valued");

        credits[id] = Credit({
            lender: lender,
            token: token,
            decimals: decimals,
            deposit: amount,
            principal: principal,
            interestAccrued: 0,
            interestRepaid: 0
        });

        ids.push(id); // add lender to end of repayment queue

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

        if(principal > 0) {
            principalUsd += value;
            emit Borrow(id, principal, value);
        }

        return id;
    }

LineOfCredit.sol

  function _repay(bytes32 id, uint256 amount)
        internal
        returns (bool)
    {
        Credit memory credit = credits[id];
        int price = oracle.getLatestAnswer(credit.token);

        if (amount <= credit.interestAccrued) {
            credit.interestAccrued -= amount;
            uint256 val = LoanLib.calculateValue(price, amount, credit.decimals);
            interestUsd -= val;

            credit.interestRepaid += amount;
            emit RepayInterest(id, amount, val);
        } else {
            uint256 principalPayment = amount - credit.interestAccrued;

            uint256 iVal = LoanLib.calculateValue(price, credit.interestAccrued, credit.decimals);
            uint256 pVal = LoanLib.calculateValue(price, principalPayment, credit.decimals);

            emit RepayInterest(id, credit.interestAccrued, iVal);
            emit RepayPrincipal(id, principalPayment, pVal);

            // update global credit denominated in usd
            interestUsd -= iVal;
            principalUsd -= pVal;

            // update individual credit position denominated in token
            credit.principal -= principalPayment;
            credit.interestRepaid += credit.interestAccrued;
            credit.interestAccrued = 0;

            // if credit fully repaid then remove lender from repayment queue
            if (credit.principal == 0) ids = LoanLib.stepQ(ids);
        }

        credits[id] = credit;

        return true;
    }

1. All necessary contracts are deployed and initialized: RevenueToken, SimpleOracle, LoanLib, LineOfCredit.
2. As the borrower and lender, add credit position,
3. As the borrower, borrow() all deposits.
4. As the borrower, attempt to call depositAndClose to pay the debt.
5. Observe that transaction reverts due to integer overflow.
6. As the borrower, attempt to call depositAndRepay to pay the debt.
7. Observe that transaction reverts due to integer overflow.1. All necessary contracts are deployed and initialized: RevenueToken, SimpleOracle, LoanLib, LineOfCredit.
8. As the borrower and lender, add credit position,
9. As the borrower, borrow() all deposits. borrow()4. As the borrower, attempt to call depositAndClose to pay the debt. depositAndClose5. Observe that transaction reverts due to integer overflow.
10. As the borrower, attempt to call depositAndRepay to pay the debt. depositAndRepay5. Observe that transaction reverts due to integer overflow.

\

SOLVED: The Debt DAO team solved this issue in commit 51dab65755c9978333b147f99db007a93bd0f81c: the principalUsd parameter and related calculations are now removed. SOLVED:Debt DAO team51dab65755c9978333b147f99db007a93bd0f81cprincipalUsd

\

In the LineOfCredit contract, a lender has the possibility to withdraw accrued interest via withdrawInterest() function. The function does not record the fact of withdrawal; thus, the function can be called multiple times until the contract has a positive token balance. LineOfCredit``withdrawInterest()

As a result, in the case of multiple lenders recorded in the contract, one lender can extract liquidity from other lenders. Alternatively, a lender can pull unborrowed deposits and force borrowers to pay off higher debt than expected, or force default.

LineOfCredit.sol

    function withdrawInterest(bytes32 id)
        external
        override
        returns (uint256)
    {
        require(
            msg.sender == credits[id].lender,
            "Loan: only lender can withdraw"
        );

        _accrueInterest(id);

        uint256 amount = credits[id].interestAccrued;

        bool success = IERC20(credits[id].token).transfer(
            credits[id].lender,
            amount
        );
        require(success, "Loan: withdraw failed");

        emit WithdrawProfit(id, amount);

        return amount;
    }

Scenario 1 - steal other lenders' liquidity Scenario 1 - steal other lenders' liquidity1. All necessary contracts are deployed and initialized: RevenueToken, SimpleOracle, LoanLib, LineOfCredit. 2. As borrower and lender1, add credit position for 10_000_000_000_000_000 tokens and drawn rate set to 3000. This action registers the first credit position. 3. As the borrower and the lender2, add credit position for 10_000_000_000_000_000 tokens and drawn rate set to 3000. This action registers the second credit position. 4. As borrower, borrow 10_000_000_000_000_000 tokens from the first position. 5. Forward blockchain time for 20 days. 6. As the lender1, call withdrawInterest for the first credit position ten times. 7. As the borrower, call depositAndClose to pay off the debt and close the first position. 8. As the borrower, attempt to borrow 10_000_000_000_000_000 tokens from the second position. 9. Observe that transaction reverts due to ERC20: transfer amount exceeds balance error. Note that LineOfCredit balance is below 10_000_000_000_000_000.1. All necessary contracts are deployed and initialized: RevenueToken, SimpleOracle, LoanLib, LineOfCredit. 2. As borrower and lender1, add credit position for 10_000_000_000_000_000 tokens and drawn rate set to 3000. This action registers the first credit position. 3. As the borrower and the lender2, add credit position for 10_000_000_000_000_000 tokens and drawn rate set to 3000. This action registers the second credit position. 4. As borrower, borrow 10_000_000_000_000_000 tokens from the first position. borrow5. Forward blockchain time for 20 days. 6. As the lender1, call withdrawInterest for the first credit position ten times. withdrawInterest7. As the borrower, call depositAndClose to pay off the debt and close the first position. depositAndClose8. As the borrower, attempt to borrow 10_000_000_000_000_000 tokens from the second position. borrow9. Observe that transaction reverts due to ERC20: transfer amount exceeds balance error. Note that LineOfCredit balance is below 10_000_000_000_000_000.ERC20: transfer amount exceeds balance``LineOfCredit

Scenario 2 - steal borrower liquidity Scenario 2 - steal borrower liquidity1. All necessary contracts are deployed and initialized: RevenueToken, SimpleOracle, LoanLib, LineOfCredit. 2. As borrower and lender1, add credit position for 10_000_000_000_000_000 tokens and drawn rate set to 3000. 4. As borrower, borrow 5_000_000_000_000_000 tokens. 5. Forward blockchain time for 20 days. 6. As the lender1, call withdrawInterest ten times. 7. As the borrower, attempt to borrow 5_000_000_000_000_000 residual tokens. 8. Observe that transaction reverts due to ERC20: transfer amount exceeds balance error. Note that LineOfCredit balance is below 5_000_000_000_000_000. 9. As the borrower, attempt to call depositAndClose to pay off the debt. 10. Observe that transaction reverts due to ERC20: transfer amount exceeds balance error.1. All necessary contracts are deployed and initialized: RevenueToken, SimpleOracle, LoanLib, LineOfCredit. 2. As borrower and lender1, add credit position for 10_000_000_000_000_000 tokens and drawn rate set to 3000. 4. As borrower, borrow 5_000_000_000_000_000 tokens. borrow5. Forward blockchain time for 20 days. 6. As the lender1, call withdrawInterest ten times. withdrawInterest7. As the borrower, attempt to borrow 5_000_000_000_000_000 residual tokens. borrow8. Observe that transaction reverts due to ERC20: transfer amount exceeds balance error. Note that LineOfCredit balance is below 5_000_000_000_000_000. ERC20: transfer amount exceeds balance``LineOfCredit9. As the borrower, attempt to call depositAndClose to pay off the debt. depositAndClose10. Observe that transaction reverts due to ERC20: transfer amount exceeds balance error.ERC20: transfer amount exceeds balance

\

SOLVED: The Debt DAO team solved this issue in commit a18167d41eb4e1ccb70bbeceeef0aff1c93b05f9: the withdrawInterest() function is now removed from contract.SOLVED:Debt DAO teama18167d41eb4e1ccb70bbeceeef0aff1c93b05f9withdrawInterest()

\

The LineOfCredit contract tracks unpaid interest valuated in USD by interestUsd parameter. This parameter is updated with addition in _accrueInterest() internal function and subtraction in _repay() internal function. The _accrueInterest() is used by accrueInterest(), setRates(), increaseCredit(), borrow(), withdraw(), and withdrawInterest() functions among the others. The _repay() is used by depositAndRepay() and depositAndClose() functions. LineOfCredit``interestUsd``_accrueInterest()``_repay()``_accrueInterest()``accrueInterest()``setRates()``increaseCredit()``borrow()``withdraw()``withdrawInterest()``_repay()``depositAndRepay()``depositAndClose()

LineOfCredit.sol

    function _accrueInterest(bytes32 id)
        internal
        returns (uint256 accruedToken, uint256 accruedValue)
    {
        Credit memory credit = credits[id];
        // get token demoninated interest accrued
        accruedToken = interestRate.accrueInterest(
            id,
            credit.principal,
            credit.deposit
        );

        // update credits balance
        credit.interestAccrued += accruedToken;

        // get USD value of interest accrued
        accruedValue = LoanLib.getValuation(
            oracle,
            credit.token,
            accruedToken,
            credit.decimals
        );
        interestUsd += accruedValue;

        emit InterestAccrued(id, accruedToken, accruedValue);

        credits[id] = credit; // save updates to intterestAccrued

        return (accruedToken, accruedValue);
    }

LineOfCredit.sol

function _repay(bytes32 id, uint256 amount)
        internal
        returns (bool)
    {
        Credit memory credit = credits[id];
        int price = oracle.getLatestAnswer(credit.token);

        if (amount <= credit.interestAccrued) {
            credit.interestAccrued -= amount;
            uint256 val = LoanLib.calculateValue(price, amount, credit.decimals);
            interestUsd -= val;

            credit.interestRepaid += amount;
            emit RepayInterest(id, amount, val);
        } else {
            uint256 principalPayment = amount - credit.interestAccrued;

            uint256 iVal = LoanLib.calculateValue(price, credit.interestAccrued, credit.decimals);
            uint256 pVal = LoanLib.calculateValue(price, principalPayment, credit.decimals);

            emit RepayInterest(id, credit.interestAccrued, iVal);
            emit RepayPrincipal(id, principalPayment, pVal);

            // update global credit denominated in usd
            interestUsd -= iVal;
            principalUsd -= pVal;

            // update individual credit position denominated in token
            credit.principal -= principalPayment;
            credit.interestRepaid += credit.interestAccrued;
            credit.interestAccrued = 0;

            // if credit fully repaid then remove lender from repayment queue
            if (credit.principal == 0) ids = LoanLib.stepQ(ids);
        }

        credits[id] = credit;

        return true;
    }

\color{black} \color{white} The value of interestUsd parameter is strongly affected by the price returned by Oracle. Thus, if the Oracle returns a higher value than previously, an integer underflow occurs in _repay() function, making debt repayment impossible. To exploit this vulnerability, _accrueInterest() must be called prior to _repay() to update interestUsd parameter.interestUsd``Oracle``Oracle``_repay()``_accrueInterest()``_repay()``interestUsd

LoanLib.sol

    /**
     * @notice         - Gets total valuation for amount of tokens using given oracle. 
     * @dev            - Assumes oracles all return answers in USD with 1e8 decimals
                       - Does not check if price < 0. HAndled in Oracle or Loan
     * @param oracle   - oracle contract specified by loan getting valuation
     * @param token    - token to value on oracle
     * @param amount   - token amount
     * @param decimals - token decimals
     * @return         - total value in usd of all tokens 
     */
    function getValuation(
      IOracle oracle,
      address token,
      uint256 amount,
      uint8 decimals
    )
      external
      returns(uint256)
    {
      return _calculateValue(oracle.getLatestAnswer(token), amount, decimals);
    }

The codebase uses SimpleOracle mock for testing. Based on this contract, the ChangingOracle was prepared that mimics the price increase after ten days.SimpleOracle``ChangingOracle

ChangingOracle.sol

pragma solidity 0.8.9;

import { IOracle } from "../interfaces/IOracle.sol";
import { LoanLib } from "../utils/LoanLib.sol";

contract ChangingOracle is IOracle {

    mapping(address => int) prices;

    uint256 public immutable creationTime;

    constructor(address _supportedToken1, address _supportedToken2) {
        prices[_supportedToken1] = 1000 * 1e8; // 1000 USD
        prices[_supportedToken2] = 2000 * 1e8; // 2000 USD
        creationTime = block.timestamp;
    }

    function init() external returns(bool) {
        return true;
    }

    function changePrice(address token, int newPrice) external {
        prices[token] = newPrice;
    }

    function getLatestAnswer(address token) external returns(int256) {
        // mimic eip4626
        // (bool success, bytes memory result) = token.call(abi.encodeWithSignature("asset()"));
        // if(success && result.length > 0) {
        //     // get the underlying token value (if ERC4626)
        //     // NB: Share token to underlying ratio might not be 1:1
        //     token = abi.decode(result, (address));
        // }
        require(prices[token] != 0, "SimpleOracle: unsupported token");

        //simulate price change
        uint256 difference = block.timestamp - creationTime;
        if (difference > 900000) //900000 = 10 days and 10 hours
            return prices[token] * 10001 / 10000;
        return prices[token];
    }

    function healthcheck() external returns (LoanLib.STATUS status) {
        return LoanLib.STATUS.ACTIVE;
    }

    function loan() external returns (address) {
        return address(0);
    }
}

\color{black} \color{white} 1. All necessary contracts are deployed and initialized: RevenueToken, ChangingOracle, LoanLib, LineOfCredit. 2. As the borrower and lender1, add credit position for 1_000_000_000_000_000 tokens. 3. As the borrower, borrow 1_000_000_000_000_000 tokens. 4. Forward blockchain time for 10 days. 5. Call accrueInterest function. Note that the interestUsd parameter value is updated. 6. Forward blockchain time for 1 day. Note that after 11 days, the ChangingOracle will return higher results. 7. As the borrower, attempt to call depositAndClose. 8. Observe that transaction reverts due to integer overflow.1. All necessary contracts are deployed and initialized: RevenueToken, ChangingOracle, LoanLib, LineOfCredit. ChangingOracle2. As the borrower and lender1, add credit position for 1_000_000_000_000_000 tokens. 3. As the borrower, borrow 1_000_000_000_000_000 tokens. borrow4. Forward blockchain time for 10 days. 5. Call accrueInterest function. Note that the interestUsd parameter value is updated. accrueInterest``interestUsd6. Forward blockchain time for 1 day. Note that after 11 days, the ChangingOracle will return higher results. ChangingOracle7. As the borrower, attempt to call depositAndClose. depositAndClose8. Observe that transaction reverts due to integer overflow.

\

SOLVED: The Debt DAO team solved this issue in commit 51dab65755c9978333b147f99db007a93bd0f81c,cbb2f0f2b68b966e95d2d64a2686de33f4c0496b: the interestUsd parameter and related calculations are now removed. SOLVED:Debt DAO team51dab65755c9978333b147f99db007a93bd0f81ccbb2f0f2b68b966e95d2d64a2686de33f4c0496binterestUsd

\

In the LineOfCredit contract, the lender has the possibility to withdraw all unborrowed deposit previously provided for loan through withdraw() function.LineOfCredit``withdraw()

The assessment revealed that withdrawal of all deposits, before the borrower borrows any amount, can deadlock the contract. The withdraw() function calls _accrueInterest() functions, so a small amount of facility interest is accrued. Eventually, the borrower can't pay off the debt, close the credit, or release the spigots. The whileBorrowing() modifier checks if any principal is borrowed; however, it does not check if any interest is accrued. The whileBorrowing() modifier is used both in LineOfCredit and SpigotedLoan contracts in depositAndClose(), depositAndRepay(), claimAndRepay() and claimAndTrade() functions.withdraw()``_accrueInterest()``whileBorrowing()``whileBorrowing()``LineOfCredit``SpigotedLoan``depositAndClose()``depositAndRepay()``claimAndRepay()``claimAndTrade()

LineOfCredit.sol

    modifier whileBorrowing() {
        require(ids.length > 0 && credits[ids[0]].principal > 0);
        _;
    }

1. All necessary contracts are deployed and initialized: RevenueToken, SimpleOracle, LoanLib, LineOfCredit.
2. As borrower and lender1, add credit position for 1_000_000_000_000_000 tokens.
3. As the lender, withdraw() all deposits.
4. As the borrower, attempt to depositAndClose. Observe that the transaction reverted.
5. As the borrower, attempt to close credit. Observe that the transaction reverted with Loan: close failed. credit owed error.
6. As the borrower, attempt to borrow deposit. Observe that the transaction reverted with Loan: no liquidity error.1. All necessary contracts are deployed and initialized: RevenueToken, SimpleOracle, LoanLib, LineOfCredit.
7. As borrower and lender1, add credit position for 1_000_000_000_000_000 tokens.
8. As the lender, withdraw() all deposits. withdraw()4. As the borrower, attempt to depositAndClose. Observe that the transaction reverted. depositAndClose5. As the borrower, attempt to close credit. Observe that the transaction reverted with Loan: close failed. credit owed error. close``Loan: close failed. credit owed6. As the borrower, attempt to borrow deposit. Observe that the transaction reverted with Loan: no liquidity error.borrow``Loan: no liquidity

\

SOLVED: The Debt DAO team solved this issue in commit b30347d17a90980aa7ca7c0ffb25067f87039c6d: the _close() internal function now checks only that the principal is not zero before closing. It does not check the interestAccrued parameter.SOLVED:Debt DAO teamb30347d17a90980aa7ca7c0ffb25067f87039c6d_close()``principal``interestAccrued

\

The SpigotedLoan contract implements a fallback mechanism to withdraw all unused funds from spigots in case of the borrower default. The sweep() function can be called to send all unused funds (based on unusedTokens collection) to the arbiter when the loan has defaulted and the status is set to INSOLVENT. However, the INSOLVENT status is never assigned to the loan in the solution, whereas the loan can have LIQUIDATABLE status assigned e.g., in healthcheck() function when the debt deadline has passed.SpigotedLoan``sweep()``unusedTokens``INSOLVENT``INSOLVENT``LIQUIDATABLE``healthcheck()

SpigotedLoan.sol

   * @notice - sends unused tokens to borrower if repaid or arbiter if liquidatable
             -  doesnt send tokens out if loan is unpaid but healthy
   * @dev    - callable by anyone 
   * @param token - token to take out
  */
    function sweep(address token) external returns (uint256) {
        if (loanStatus == LoanLib.STATUS.REPAID) {
            return _sweep(borrower, token);
        }
        if (loanStatus == LoanLib.STATUS.INSOLVENT) {
            return _sweep(arbiter, token);
        }

        return 0;
    }

    function _sweep(address to, address token) internal returns (uint256 x) {
        x = unusedTokens[token];
        if (token == address(0)) {
            payable(to).transfer(x);
        } else {
            require(IERC20(token).transfer(to, x));
        }
        delete unusedTokens[token];
    }

\color{black} \color{white} As a result, all unused revenue and credit tokens stored in SpigotedLoan (unusedTokens collection) are locked in the contract. The credit token can be transferred to the lender using claimAndRepay() function, unless the spigot is still owned by the SpigotedLoan contract, and it is providing new revenue. On the other hand, the revenue token is locked permanently. SpigotedLoan``unusedTokens``claimAndRepay()``SpigotedLoan

1. All necessary contracts are deployed and initialized: CreditToken, RevenueToken, SimpleOracle, LoanLib, SpigotedLoan, SimpleRevenueContract. Set the ttl parameter to 1 day.
2. As the borrower and lender1, add credit position for 10_000_000_000_000_000 tokens.
3. As the borrower, borrow() half of the deposit - 5_000_000_000_000_000.
4. As th borrower and arbiter, add new spigot with addSpigot() function and RevenueContract as input.
5. As the borrower, transfer the ownership of RevenueContract contract to the SpigotController.
6. Mint 500_000_000_000_000 revenue tokens to the RevenueContract contract to simulate token gain.
7. Forward blockchain time for 1 day and 1 second.
8. As the borrower, attempt to borrow() the remaining deposit. Observe that the transaction reverted with Loan: can't borrow error.1. All necessary contracts are deployed and initialized: CreditToken, RevenueToken, SimpleOracle, LoanLib, SpigotedLoan, SimpleRevenueContract. Set the ttl parameter to 1 day. ttl2. As the borrower and lender1, add credit position for 10_000_000_000_000_000 tokens.
9. As the borrower, borrow() half of the deposit - 5_000_000_000_000_000. borrow()2. As th borrower and arbiter, add new spigot with addSpigot() function and RevenueContract as input. addSpigot()``RevenueContract3. As the borrower, transfer the ownership of RevenueContract contract to the SpigotController. RevenueContract4. Mint 500_000_000_000_000 revenue tokens to the RevenueContract contract to simulate token gain. RevenueContract5. Forward blockchain time for 1 day and 1 second.
10. As the borrower, attempt to borrow() the remaining deposit. Observe that the transaction reverted with Loan: can't borrow error.borrow()``Loan: can't borrow

1. As the arbiter, call healthcheck() function. Note that the loan's status changed from ACTIVE to LIQUIDATABLE.
2. As the arbiter, call updateOwnerSplit so 100% of revenue will go to the SpigotController contract.7. As the arbiter, call healthcheck() function. Note that the loan's status changed from ACTIVE to LIQUIDATABLE. healthcheck()``ACTIVE``LIQUIDATABLE8. As the arbiter, call updateOwnerSplit so 100% of revenue will go to the SpigotController contract.updateOwnerSplit

1. As the arbiter, call claimRevenue() in SpigotController contract to claim revenue tokens. Note that 100% of tokens (500_000_000_000_000) are transferred to the SpigotController.9. As the arbiter, call claimRevenue() in SpigotController contract to claim revenue tokens. Note that 100% of tokens (500_000_000_000_000) are transferred to the SpigotController.claimRevenue()

1. As the arbiter, call claimAndRepay() in SpigotedLoan contract to trade escrowed revenue tokens and pay off part of the debt. As an input, trade exchange data provide 250_000_000_000_000 revenue tokens that should be exchanged for credit tokens.
2. Observe the SpigotedLoan balances. Note that the contract has 750,000,000,000,000 credit tokens and 250,000,000,000,000 revenue tokens. Also, 250,000,000,000,000 credit tokens and 250,000,000,000,000 revenue tokens are stored in unusedTokens collection.10. As the arbiter, call claimAndRepay() in SpigotedLoan contract to trade escrowed revenue tokens and pay off part of the debt. As an input, trade exchange data provide 250_000_000_000_000 revenue tokens that should be exchanged for credit tokens. claimAndRepay()11. Observe the SpigotedLoan balances. Note that the contract has 750,000,000,000,000 credit tokens and 250,000,000,000,000 revenue tokens. Also, 250,000,000,000,000 credit tokens and 250,000,000,000,000 revenue tokens are stored in unusedTokens collection.unusedTokens

1. As arbiter, call sweep() function with revenue token address as an input. Note that the function returns a 0 value.
2. As arbiter, call sweep() function with credit token address as an input. Note that the function returns a 0 value.
3. Observe that SpigotedLoan balances remain unchanged. 12. As arbiter, call sweep() function with revenue token address as an input. Note that the function returns a 0 value. sweep()13. As arbiter, call sweep() function with credit token address as an input. Note that the function returns a 0 value. sweep()14. Observe that SpigotedLoan balances remain unchanged.

\

SOLVED: The Debt DAO team solved this issue in commit 2f7f0b44a2d257c92d7626f14f579876c7d00fee: the sweep() function now checks loan status against LIQUIDATABLE value.SOLVED:Debt DAO team2f7f0b44a2d257c92d7626f14f579876c7d00feesweep()``LIQUIDATABLE

\

In the Escrow contract, the releaseCollateral() function allows the borrower to withdraw collateral with the assumption that the remaining collateral is still above the minimum threshold. When the debt is paid off, the borrower should be allowed to withdraw all remaining collateral. However, the assessment revealed that withdraw of the remaining collateral is not possible. Escrow``releaseCollateral()

Escrow.sol

    function releaseCollateral(
        uint256 amount,
        address token,
        address to
    ) external returns (uint256) {
        require(amount > 0, "Escrow: amount is 0");
        require(msg.sender == borrower, "Escrow: only borrower can call");
        require(
            deposited[token].amount >= amount,
            "Escrow: insufficient balance"
        );
        deposited[token].amount -= amount;
        require(IERC20(token).transfer(to, amount));
        uint256 cratio = _getLatestCollateralRatio();
        require(
            cratio >= minimumCollateralRatio,
            "Escrow: cannot release collateral if cratio becomes lower than the minimum"
        );
        emit RemoveCollateral(token, amount);

        return cratio;
    }

When the last part of the collateral is released, the releaseCollateral() function updates the deposited[token].amount with 0 value. Then the _getLatestCollateralRatio() returns 0 as collateralValue is also 0. Therefore, it is not possible to pass the assertion cratio >= minimumCollateralRatio as it is always false.releaseCollateral()``deposited[token].amount``_getLatestCollateralRatio()``collateralValue``cratio >= minimumCollateralRatio``false

Escrow.sol

    /**
     * @notice updates the cratio according to the collateral value vs loan value
     * @dev calls accrue interest on the loan contract to update the latest interest payable
     * @return the updated collateral ratio in 18 decimals
     */
    function _getLatestCollateralRatio() internal returns (uint256) {
        ILoan(loan).accrueInterest();
        uint256 debtValue = ILoan(loan).getOutstandingDebt();
        uint256 collateralValue = _getCollateralValue();
        if (collateralValue == 0) return 0;
        if (debtValue == 0) return MAX_INT;

        return _percent(collateralValue, debtValue, 18);
    }

1. All necessary contracts are deployed and initialized: CreditToken, RevenueToken, SimpleOracle, LoanLib, EscrowedLoan.
2. As arbiter, enable the RevenueToken token as collateral.
3. As the borrower, add 200_000_000_000_000 of RevenueToken tokens as collateral.
4. As borrower and lender, add debt position.
5. As the borrower, borrow all deposits.
6. As the borrower, attempt to call releaseCollateral() to withdraw all remaining collateral. Observe that the transaction reverts with Escrow: cannot release collateral if cratio becomes lower than the minimum error. Note that 200_000_000_000_000 of RevenueToken tokens are locked in the Escrow contract.1. All necessary contracts are deployed and initialized: CreditToken, RevenueToken, SimpleOracle, LoanLib, EscrowedLoan.
7. As arbiter, enable the RevenueToken token as collateral. RevenueToken3. As the borrower, add 200_000_000_000_000 of RevenueToken tokens as collateral. RevenueToken2. As borrower and lender, add debt position.
8. As the borrower, borrow all deposits. borrow4. As the borrower, attempt to call releaseCollateral() to withdraw all remaining collateral. Observe that the transaction reverts with Escrow: cannot release collateral if cratio becomes lower than the minimum error. Note that 200_000_000_000_000 of RevenueToken tokens are locked in the Escrow contract.releaseCollateral()``Escrow: cannot release collateral if cratio becomes lower than the minimum``RevenueToken``Escrow

\

SOLVED: The Debt DAO team solved this issue in commit e97e8be0c7ecf8710a996b8f202bbd3a6bee0e2c: the releaseCollateral() function now checks the loan status against REPAID value.SOLVED:Debt DAO teame97e8be0c7ecf8710a996b8f202bbd3a6bee0e2creleaseCollateral()``REPAID

\

In the LineOfCredit contract, the getOutstandingDebt() function allows users to get information about total outstanding debt valuated in USD. The presented amount is a sum of principal and interest. The assessment revealed that the getOutstandingDebt() function does not consider the accrued interest from the last evaluation period. Thus, it presents misleading, understated value to the users as actual debt is higher. LineOfCredit``getOutstandingDebt()``getOutstandingDebt()

LineOfCredit.sol

/**
  * @notice - Returns total credit obligation of borrower.
              Aggregated across all lenders.
              Denominated in USD 1e8.
  * @dev    - callable by anyone
  */
    function getOutstandingDebt() external override returns (uint256) {
        (uint256 p, uint256 i) = _updateOutstandingCredit();
        return p + i;
    }

    function _updateOutstandingCredit()
        internal
        returns (uint256 principal, uint256 interest)
    {
        uint256 len = ids.length;
        if (len == 0) return (0, 0);

        Credit memory credit;
        for (uint256 i = 0; i < len; i++) {
            credit = credits[ids[i]];

            int256 price = oracle.getLatestAnswer(credit.token);

            principal += LoanLib.calculateValue(
                price,
                credit.principal,
                credit.decimals
            );
            interest += LoanLib.calculateValue(
                price,
                credit.interestAccrued,
                credit.decimals
            );
        }

        principalUsd = principal;
        interestUsd = interest;
    }

1. All necessary contracts are deployed and initialized: RevenueToken, SimpleOracle, LoanLib, LineOfCredit.
2. As borrower and lender1, add credit position for 10_000_000_000_000_000 tokens.
3. As the borrower, borrow 500_000_000_000_000 tokens.
4. Forward blockchain time for 10 days.
5. Call getOutstandingDebt() function. Observe that returned value of 100000000 does not include accrued interest.
6. Call accrueInterest() function. Observe that returned value of 27652.
7. As borrower, borrow 500_000_000_000_000 tokens.
8. Forward blockchain time for 10 days.
9. Call the getOutstandingDebt() function. Observe that returned value of 200027652. Note that this value includes the accrued interest from step 6.
10. Call the accrueInterest() function. Observe that returned value of 54757. Note that this value includes the accrued interest only from the last 10 days and was not included in step 9.1. All necessary contracts are deployed and initialized: RevenueToken, SimpleOracle, LoanLib, LineOfCredit.
11. As borrower and lender1, add credit position for 10_000_000_000_000_000 tokens.
12. As the borrower, borrow 500_000_000_000_000 tokens. borrow4. Forward blockchain time for 10 days.
13. Call getOutstandingDebt() function. Observe that returned value of 100000000 does not include accrued interest. getOutstandingDebt()``1000000006. Call accrueInterest() function. Observe that returned value of 27652. accrueInterest()``276527. As borrower, borrow 500_000_000_000_000 tokens. borrow8. Forward blockchain time for 10 days.
14. Call the getOutstandingDebt() function. Observe that returned value of 200027652. Note that this value includes the accrued interest from step 6. getOutstandingDebt()``20002765210. Call the accrueInterest() function. Observe that returned value of 54757. Note that this value includes the accrued interest only from the last 10 days and was not included in step 9.accrueInterest()``54757

\

SOLVED: The Debt DAO team solved this issue in commit c571e48d52f886b2d4c9f930a6bb27cf331501ae: the getOutstandingDebt() function is now removed. The updateOutstandingDebt() function is now added and includes both updated principal and accrued interest.SOLVED:Debt DAO teamc571e48d52f886b2d4c9f930a6bb27cf331501aegetOutstandingDebt()``updateOutstandingDebt()

\

In the LineOfCredit contract, a borrower can add multiple credits with various lenders. The borrower can borrow any amount from any credit position using the borrow(bytes32 id, uint256 amount) function. However, the borrower can only repay first credit position using depositAndRepay() or depositAndClose() functions.LineOfCredit``borrow(bytes32 id, uint256 amount)``depositAndRepay()``depositAndClose()

LineOfCredit.sol

/**
    * @notice - Transfers enough tokens to repay entire credit position from `borrower` to Loan contract.
    * @dev - callable by borrower    
    */
    function depositAndClose()
        external
        override
        whileBorrowing
        onlyBorrower
        returns (bool)
    {
        bytes32 id = ids[0];
        _accrueInterest(id);

        uint256 totalOwed = credits[id].principal + credits[id].interestAccrued;

        // borrower deposits remaining balance not already repaid and held in contract
        bool success = IERC20(credits[id].token).transferFrom(
            msg.sender,
            address(this),
            totalOwed
        );
        require(success, "Loan: deposit failed");
        // clear the credit
        _repay(id, totalOwed);

        require(_close(id));
        return true;
    }

    /**
     * @dev - Transfers token used in credit position from msg.sender to Loan contract.
     * @dev - callable by anyone
     * @notice - see _repay() for more details
     * @param amount - amount of `token` in `id` to pay back
     */

    function depositAndRepay(uint256 amount)
        external
        override
        whileBorrowing
        returns (bool)
    {
        bytes32 id = ids[0];
        _accrueInterest(id);

        require(amount <= credits[id].principal + credits[id].interestAccrued);

        bool success = IERC20(credits[id].token).transferFrom(
            msg.sender,
            address(this),
            amount
        );
        require(success, "Loan: failed repayment");

        _repay(id, amount);
        return true;
    }

\color{black} \color{white} When the borrower borrow() deposit from the second position, repaying the debt will not be possible, as the whileBorrowing() modifier would block that operation. At this point, the close(bytes32 id) function can be called to close the first credit position unless the _accrueInterest() internal function is called and interest is accrued, preventing the closing of the unpaid debt. borrow()``whileBorrowing()``close(bytes32 id)``_accrueInterest()

LineOfCredit.sol

    modifier whileBorrowing() {
        require(ids.length > 0 && credits[ids[0]].principal > 0);
        _;
    }

\color{black} \color{white}

To escape from the situation, the borrower can still borrow() any amount from the first position unless the lender does not withdraw all liquidity. The withdrawal operation accrues the interest, which cannot be paid, as the whileBorrowing() modifier only considers the principal. Moreover, the borrower can't borrow() anymore from the empty deposit. As a result, a deadlock occurs in the contract, and the borrower can't pay off the debt.borrow()``whileBorrowing()``borrow()

The root cause of this issue is the _sortIntoQ(bytes32 p) function, which supposes to shift the credit position with the borrowed deposit to the beginning of the ids collection, but it does not work as expected. Sample invalid flow:_sortIntoQ(bytes32 p)``ids

1. For i = 0; _i = i = 0 as first credit's principal is 0.
2. For i = 1, function returns true, as _i = 0.
3. No position swap occurs.1. For i = 0; _i = i = 0 as first credit's principal is 0.
4. For i = 1, function returns true, as _i = 0.
5. No position swap occurs.

LineOfCredit.sol

function _sortIntoQ(bytes32 p) internal returns (bool) {
        uint256 len = ids.length;
        uint256 _i = 0; // index that p should be moved to

        for (uint256 i = 0; i < len; i++) {
            bytes32 id = ids[i];
            if (p != id) {
                if (credits[id].principal > 0) continue; // `id` should be placed before `p`
                _i = i; // index of first undrawn LoC found
            } else {
                if (_i == 0) return true; // `p` in earliest possible index
                // swap positions
                ids[i] = ids[_i];
                ids[_i] = p;
            }
        }

        return true;
    }

1. All necessary contracts are deployed and initialized: RevenueToken, SimpleOracle, LoanLib, LineOfCredit.
2. As borrower and lender1, add credit position for 10_000_000_000_000_000 tokens. This action registers the first credit position.
3. As a borrower and lender2, add credit position for 10_000_000_000_000_000 tokens. This action registers the second credit position.
4. As the borrower, borrow 10_000_000_000_000_000 tokens from the second position.
5. Note that the ids collection was not updated; the second credit position is not set to the first.
6. Forward blockchain time for 1 second.
7. Call the accrueInterest function. Note that the interestAccrued parameter value in the first credit record is updated.
8. As the borrower, attempt to depositAndRepay with any value. Observe that the transaction reverted.
9. As the borrower, attempt to depositAndClose. Observe that the transaction reverted.
10. As the borrower, attempt to close the first credit position. Observe that the transaction reverted with Loan: close failed. credit owed error.
11. As the lender2, attempt to close the second credit position. Observe that the transaction reverted with Loan: close failed. credit owed error.
12. As the lender1, withdraw all liquidity.
13. As the borrower, attempt to borrow. Observe that the transaction reverted with a Loan: no liquidity error.
14. As the borrower, attempt to close the first credit position again. Observe that the transaction reverted with Loan: close failed. credit owed error. 1. All necessary contracts are deployed and initialized: RevenueToken, SimpleOracle, LoanLib, LineOfCredit.
15. As borrower and lender1, add credit position for 10_000_000_000_000_000 tokens. This action registers the first credit position.
16. As a borrower and lender2, add credit position for 10_000_000_000_000_000 tokens. This action registers the second credit position.
17. As the borrower, borrow 10_000_000_000_000_000 tokens from the second position. borrow5. Note that the ids collection was not updated; the second credit position is not set to the first. ids5. Forward blockchain time for 1 second.
18. Call the accrueInterest function. Note that the interestAccrued parameter value in the first credit record is updated. accrueInterest``interestAccrued``credit7. As the borrower, attempt to depositAndRepay with any value. Observe that the transaction reverted. depositAndRepay8. As the borrower, attempt to depositAndClose. Observe that the transaction reverted. depositAndClose9. As the borrower, attempt to close the first credit position. Observe that the transaction reverted with Loan: close failed. credit owed error. close``Loan: close failed. credit owed10. As the lender2, attempt to close the second credit position. Observe that the transaction reverted with Loan: close failed. credit owed error. close``Loan: close failed. credit owed11. As the lender1, withdraw all liquidity. withdraw12. As the borrower, attempt to borrow. Observe that the transaction reverted with a Loan: no liquidity error. borrow``Loan: no liquidity13. As the borrower, attempt to close the first credit position again. Observe that the transaction reverted with Loan: close failed. credit owed error. close``Loan: close failed. credit owed

\

SOLVED: The Debt DAO team solved this issue in commit 78a2a57081ee1242ab71a09e1f28218de4e03b65: the _sortIntoQ() function has now proper implementation. The borrowed line of credit now moves to the first position to pay off. SOLVED:Debt DAO team78a2a57081ee1242ab71a09e1f28218de4e03b65_sortIntoQ()

\

In the SpigotedLoan contract, a borrower and arbiter can add a spigot. Both of them must agree on a percentage value in ownerSplit parameter from SpigotSettings structure. The ownerSplit parameter determines the amount of revenue tokens that stays in SpigotController contract and are used to pay off the debt.SpigotedLoan``ownerSplit``SpigotSettings``ownerSplit``SpigotController

SpigotedLoan.sol

    function addSpigot(
        address revenueContract,
        SpigotController.SpigotSettings calldata setting
    ) external mutualConsent(arbiter, borrower) returns (bool) {
        return spigot.addSpigot(revenueContract, setting);
    }

Spigot.sol

    struct SpigotSettings {
        address token;                // token to claim as revenue from contract
        uint8 ownerSplit;             // x/100 % to Owner, rest to Treasury
        bytes4 claimFunction;         // function signature on contract to call and claim revenue
        bytes4 transferOwnerFunction; // function signature on conract to call and transfer ownership 
    }

\color{black} \color{white} The updateOwnerSplit() function can be used to update the ownerSplit parameter in particular spigot basing on loan health. If the loan status is active, then defaultRevenueSplit is applied. The defaultRevenueSplit parameter is set in constructor. Based on unit tests, this parameter should have 10 as a value.updateOwnerSplit()``ownerSplit``defaultRevenueSplit``defaultRevenueSplit``10

Spigot.sol

    /**
     * @notice changes the revenue split between borrower treasury and lan repayment based on loan health
     * @dev    - callable `arbiter` + `borrower`
     * @param revenueContract - spigot to update
     */
    function updateOwnerSplit(address revenueContract) external returns (bool) {
        (, uint8 split, , bytes4 transferFunc) = spigot.getSetting(
            revenueContract
        );

        require(transferFunc != bytes4(0), "SpgtLoan: no spigot");

        if (
            loanStatus == LoanLib.STATUS.ACTIVE && split != defaultRevenueSplit
        ) {
            // if loan is healthy set split to default take rate
            spigot.updateOwnerSplit(revenueContract, defaultRevenueSplit);
        } else if (
            loanStatus == LoanLib.STATUS.LIQUIDATABLE && split != MAX_SPLIT
        ) {
            // if loan is in distress take all revenue to repay loan
            spigot.updateOwnerSplit(revenueContract, MAX_SPLIT);
        }

        return true;
    }

\color{black} \color{white} The assessment revealed that the updateOwnerSplit() function could be abused by the borrower or the lender, depending on the situation. If the ownerSplit parameter is set below the defaultRevenueSplit parameter, the lender can call it to increase the split percentage. Then more revenue would be used to pay off the debt. If the ownerSplit parameter is set above the defaultRevenueSplit parameter, the borrower can call it to decrease the split percentage. Then less revenue would be used to pay off the debt, and more revenue would return to the treasury (which is by default set to the borrower). Moreover, the healthy loan is always set to ACTIVE. updateOwnerSplit()``ownerSplit``defaultRevenueSplit``ownerSplit``defaultRevenueSplit``ACTIVE

Additionally, based on the comment, the function is meant to be called by the arbiter or the borrower; however, no such authorization check is implemented.

1. All necessary contracts are deployed and initialized: CreditToken, RevenueToken, SimpleOracle, LoanLib, SpigotedLoan, SimpleRevenueContract. In the SpigotedLoan set the defaultRevenueSplit parameter to 10.
2. As borrower and arbiter, add a new spigot with the addSpigot() function. Set the ownerSplit parameter to 5.
3. As the lender, call the updateOwnerSplit() function for the spigot added in the previous step. Note that the ownerSplit parameter is now set to 10.
4. As borrower and arbiter, add a new spigot with the addSpigot() function. Set the ownerSplit parameter to 20.
5. As the borrower, call the updateOwnerSplit() function for the spigot added in the previous step. Note that the ownerSplit parameter is now set to 10.1. All necessary contracts are deployed and initialized: CreditToken, RevenueToken, SimpleOracle, LoanLib, SpigotedLoan, SimpleRevenueContract. In the SpigotedLoan set the defaultRevenueSplit parameter to 10. SpigotedLoan``defaultRevenueSplit``102. As borrower and arbiter, add a new spigot with the addSpigot() function. Set the ownerSplit parameter to 5. addSpigot()``ownerSplit``53. As the lender, call the updateOwnerSplit() function for the spigot added in the previous step. Note that the ownerSplit parameter is now set to 10. updateOwnerSplit()``ownerSplit``104. As borrower and arbiter, add a new spigot with the addSpigot() function. Set the ownerSplit parameter to 20. addSpigot()``ownerSplit``205. As the borrower, call the updateOwnerSplit() function for the spigot added in the previous step. Note that the ownerSplit parameter is now set to 10.updateOwnerSplit()``ownerSplit``10

\

RISK ACCEPTED: The Debt DAO team accepted the risk of this finding. The implementation is transparent. The assumption is that default values should always be selected.RISK ACCEPTED:Debt DAO team

\

In the SpigotedLoan contract, the borrower can add spigots with revenue contracts that will support repayment of the debt. The revenue contract earns revenue tokens that later can be exchanged for the credit tokens using the claimAndRepay() and claimAndTrade() functions. These functions call the _claimAndTrade() internal function responsible for claiming escrowed tokens from the SpigotController contract and trading the claimed tokens. Also, the _claimAndTrade() function updates the unusedTokens collection with claimed tokens (revenue tokens) that were not traded.SpigotedLoan``claimAndRepay()``claimAndTrade()``_claimAndTrade()``SpigotController``_claimAndTrade()``unusedTokens

The assessment revealed that it is possible to trade less revenue tokens than previously claimed. The rest of the claimed tokens are locked in the SpigotedLoan contract. They remain locked until the debt is paid off by other means, and the borrower can transfer all tokens from unusedTokens collection using the sweep() function. The SpigotedLoan contract has no other function that allows to trade and repay revenue tokens that were previously claimed.SpigotedLoan``unusedTokens``sweep()``SpigotedLoan

This vulnerability remains the same for both cases when the revenue token is either ERC20 token or ether.

This vulnerability can also be abused by the borrower to use less revenue tokens to pay off the debt than was intended, making the revenue split mechanism ineffective.

SpigotedLoan.sol

function _claimAndTrade(
        address claimToken,
        address targetToken,
        bytes calldata zeroExTradeData
    ) internal returns (uint256 tokensBought) {
        uint256 existingClaimTokens = IERC20(claimToken).balanceOf(
            address(this)
        );
        uint256 existingTargetTokens = IERC20(targetToken).balanceOf(
            address(this)
        );

        uint256 tokensClaimed = spigot.claimEscrow(claimToken);

        if (claimToken == address(0)) {
            // if claiming/trading eth send as msg.value to dex
            (bool success, ) = swapTarget.call{value: tokensClaimed}(
                zeroExTradeData
            );
            require(success, "SpigotCnsm: trade failed");
        } else {
            IERC20(claimToken).approve(
                swapTarget,
                existingClaimTokens + tokensClaimed
            );
            (bool success, ) = swapTarget.call(zeroExTradeData);
            require(success, "SpigotCnsm: trade failed");
        }

        uint256 targetTokens = IERC20(targetToken).balanceOf(address(this));

        // ideally we could use oracle to calculate # of tokens to receive
        // but claimToken might not have oracle. targetToken must have oracle

        // underflow revert ensures we have more tokens than we started with
        tokensBought = targetTokens - existingTargetTokens;

        emit TradeSpigotRevenue(
            claimToken,
            tokensClaimed,
            targetToken,
            tokensBought
        );

        // update unused if we didnt sell all claimed tokens in trade
        // also underflow revert protection here
        unusedTokens[claimToken] +=
            IERC20(claimToken).balanceOf(address(this)) -
            existingClaimTokens;
    }

1. All necessary contracts are deployed and initialized: CreditToken, RevenueToken, SimpleOracle, LoanLib, SpigotedLoan, SimpleRevenueContract.
2. As borrower and lender1, add credit position for 10_000_000_000_000_000 tokens.
3. As borrower, borrow() all deposits.
4. As borrower and arbiter, add new spigot with addSpigot() function and RevenueContract as input. Set the ownerSplit parameter to 10.
5. As the borrower, transfer the ownership of the RevenueContract contract to the SpigotController.
6. Mint 500_000_000_000_000 revenue tokens to the RevenueContract contract to simulate token gain.
7. As borrower, call claimRevenue() in SpigotController contract to claim revenue tokens. Note that 90% of tokens (450_000_000_000_000) are transferred to the treasury (which is the borrower), the 10% (50_000_000_000_000) of tokens are transferred to the SpigotController.1. All necessary contracts are deployed and initialized: CreditToken, RevenueToken, SimpleOracle, LoanLib, SpigotedLoan, SimpleRevenueContract.
8. As borrower and lender1, add credit position for 10_000_000_000_000_000 tokens.
9. As borrower, borrow() all deposits. borrow()2. As borrower and arbiter, add new spigot with addSpigot() function and RevenueContract as input. Set the ownerSplit parameter to 10. addSpigot()``RevenueContract``ownerSplit``103. As the borrower, transfer the ownership of the RevenueContract contract to the SpigotController. RevenueContract4. Mint 500_000_000_000_000 revenue tokens to the RevenueContract contract to simulate token gain. RevenueContract5. As borrower, call claimRevenue() in SpigotController contract to claim revenue tokens. Note that 90% of tokens (450_000_000_000_000) are transferred to the treasury (which is the borrower), the 10% (50_000_000_000_000) of tokens are transferred to the SpigotController.claimRevenue()

1. As the borrower, call claimAndRepay() in the SpigotedLoan contract to trade escrowed revenue tokens and pay off part of the debt. As an input, trade exchange data provide 1_000_000_000_000 of revenue tokens that should be exchanged for credit tokens.
2. Observe the SpigotedLoan balances. Note that the contract has 1,000,000,000,000 credit tokens and 49,000,000,000,000 revenue tokens. Also, 49,000,000,000,000 revenue tokens are stored in unusedTokens collection.6. As the borrower, call claimAndRepay() in the SpigotedLoan contract to trade escrowed revenue tokens and pay off part of the debt. As an input, trade exchange data provide 1_000_000_000_000 of revenue tokens that should be exchanged for credit tokens. claimAndRepay()7. Observe the SpigotedLoan balances. Note that the contract has 1,000,000,000,000 credit tokens and 49,000,000,000,000 revenue tokens. Also, 49,000,000,000,000 revenue tokens are stored in unusedTokens collection.unusedTokens

1. As the borrower, call depositAndClose() to pay off the debt.
2. As the borrower, call sweep() function with revenue token address as an input to receive 49,000,000,000,000 revenue tokens from SpigotedLoan contract.8. As the borrower, call depositAndClose() to pay off the debt. depositAndClose()9. As the borrower, call sweep() function with revenue token address as an input to receive 49,000,000,000,000 revenue tokens from SpigotedLoan contract.sweep()

\

SOLVED: The Debt DAO team solved this issue in commit 760c5bfb9c352fb681f8351253ff9776b176e357: the claimAndRepay() function now allows you to trade unused revenue tokens to pay off debt.SOLVED:Debt DAO team760c5bfb9c352fb681f8351253ff9776b176e357claimAndRepay()

\

In the SpigotedLoan contract, the borrower can add spigots with revenue contracts that will support repayment of the debt. The revenue contract earns revenue tokens that later can be exchanged for the credit tokens using the claimAndRepay() and claimAndTrade() functions. These functions call the _claimAndTrade() internal function responsible for claiming escrowed tokens from the SpigotController contract and trading the claimed tokens. It is possible to add a spigot with revenue contracts that earns revenue in ether. However, the assessment revealed that claiming and trading ether from a spigot is not possible in the _claimAndTrade() function. This function assumes that the revenue token is an ERC20 token in every case. As a result, the escrowed ether remains locked until the debt is paid off by other means, and the borrower can transfer it from the SpigotController contract.SpigotedLoan``claimAndRepay()``claimAndTrade()``_claimAndTrade()``SpigotController``_claimAndTrade()``SpigotController

SpigotedLoan.sol

function _claimAndTrade(
        address claimToken,
        address targetToken,
        bytes calldata zeroExTradeData
    ) internal returns (uint256 tokensBought) {
        uint256 existingClaimTokens = IERC20(claimToken).balanceOf(
            address(this)
        );
        uint256 existingTargetTokens = IERC20(targetToken).balanceOf(
            address(this)
        );

        uint256 tokensClaimed = spigot.claimEscrow(claimToken);

        if (claimToken == address(0)) {
            // if claiming/trading eth send as msg.value to dex
            (bool success, ) = swapTarget.call{value: tokensClaimed}(
                zeroExTradeData
            );
            require(success, "SpigotCnsm: trade failed");
        } else {
            IERC20(claimToken).approve(
                swapTarget,
                existingClaimTokens + tokensClaimed
            );
            (bool success, ) = swapTarget.call(zeroExTradeData);
            require(success, "SpigotCnsm: trade failed");
        }

        uint256 targetTokens = IERC20(targetToken).balanceOf(address(this));

        // ideally we could use oracle to calculate # of tokens to receive
        // but claimToken might not have oracle. targetToken must have oracle

        // underflow revert ensures we have more tokens than we started with
        tokensBought = targetTokens - existingTargetTokens;

        emit TradeSpigotRevenue(
            claimToken,
            tokensClaimed,
            targetToken,
            tokensBought
        );

        // update unused if we didnt sell all claimed tokens in trade
        // also underflow revert protection here
        unusedTokens[claimToken] +=
            IERC20(claimToken).balanceOf(address(this)) -
            existingClaimTokens;
    }

1. All necessary contracts are deployed and initialized: CreditToken, RevenueToken, SimpleOracle, LoanLib, SpigotedLoan, SimpleRevenueContract.
2. As the borrower and lender1, add credit position for 10_000_000_000_000_000 tokens.
3. As the borrower, borrow() all deposits.
4. As the borrower and arbiter, add a new spigot with addSpigot() function and RevenueContract as input. Set the revenue token to zero address (0x0). Note that zero address represents ether in the SpigotController contract.
5. As the borrower, transfer the ownership of the RevenueContract contract to the SpigotController.
6. Transfer 5_000_000_000_000_000_000 ether to the RevenueContract contract to simulate ether gain.
7. As the borrower, call claimRevenue() in SpigotController contract to claim ether.
8. As the borrower, attempt to call claimAndTrade() or claimAndRepay() in the SpigotedLoan contract to trade escrowed ether and pay off part of the debt.
9. Observe that the above transaction reverts. Note that ether remains in SpigotController.1. All necessary contracts are deployed and initialized: CreditToken, RevenueToken, SimpleOracle, LoanLib, SpigotedLoan, SimpleRevenueContract.
10. As the borrower and lender1, add credit position for 10_000_000_000_000_000 tokens.
11. As the borrower, borrow() all deposits. borrow()2. As the borrower and arbiter, add a new spigot with addSpigot() function and RevenueContract as input. Set the revenue token to zero address (0x0). Note that zero address represents ether in the SpigotController contract. addSpigot()``RevenueContract3. As the borrower, transfer the ownership of the RevenueContract contract to the SpigotController. RevenueContract4. Transfer 5_000_000_000_000_000_000 ether to the RevenueContract contract to simulate ether gain. RevenueContract5. As the borrower, call claimRevenue() in SpigotController contract to claim ether. claimRevenue()6. As the borrower, attempt to call claimAndTrade() or claimAndRepay() in the SpigotedLoan contract to trade escrowed ether and pay off part of the debt. claimAndTrade()``claimAndRepay()7. Observe that the above transaction reverts. Note that ether remains in SpigotController.

\

SOLVED: The Debt DAO team solved this issue in commit cd711d024ac7df475329d9dab4f88756d194d133: the solution now supports spigot earning in ether.SOLVED:Debt DAO teamcd711d024ac7df475329d9dab4f88756d194d133

\

In the SpigotedLoan contract, the borrower can add spigots with revenue contracts that will support repayment of the debt. The revenue contract earns revenue tokens that later can be exchanged to the credit tokens using the claimAndRepay() and claimAndTrade() functions. These functions call the _claimAndTrade() internal function responsible for claiming escrowed tokens from the SpigotController contract and trading the claimed tokens. Also, the claimAndTrade() function updates the unusedTokens collection with target tokens (credit tokens) that were traded. It is possible to add a new spigot with the revenue contract that earns in credit tokens. Then, the assumption is that in the _claimAndTrade() function, the trade of the tokens is done with a ratio of 1-to-1, or the decentralized exchange contract simply returns a false value (instead of revert). The project team confirmed this scenario. As a result, the unusedTokens collection is updated twice for credit tokens, first time in the _claimAndTrade() function and second time in the claimAndTrade() function.SpigotedLoan``revenue tokens``credit tokens``claimAndRepay()``claimAndTrade()``_claimAndTrade()``SpigotController``claimAndTrade()``unusedTokens``revenue contract``credit tokens``_claimAndTrade()``false``unusedTokens``credit tokens``_claimAndTrade()``claimAndTrade()

SpigotedLoan.sol

    function claimAndTrade(address claimToken, bytes calldata zeroExTradeData)
        external
        whileBorrowing
        returns (uint256 tokensBought)
    {
        require(msg.sender == borrower || msg.sender == arbiter);

        address targetToken = credits[ids[0]].token;
        tokensBought = _claimAndTrade(
            claimToken,
            targetToken,
            zeroExTradeData
        );

        // add bought tokens to unused balance
        unusedTokens[targetToken] += tokensBought;
    }

function _claimAndTrade(
        address claimToken,
        address targetToken,
        bytes calldata zeroExTradeData
    ) internal returns (uint256 tokensBought) {
        uint256 existingClaimTokens = IERC20(claimToken).balanceOf(
            address(this)
        );
        uint256 existingTargetTokens = IERC20(targetToken).balanceOf(
            address(this)
        );

        uint256 tokensClaimed = spigot.claimEscrow(claimToken);

        if (claimToken == address(0)) {
            // if claiming/trading eth send as msg.value to dex
            (bool success, ) = swapTarget.call{value: tokensClaimed}(
                zeroExTradeData
            );
            require(success, "SpigotCnsm: trade failed");
        } else {
            IERC20(claimToken).approve(
                swapTarget,
                existingClaimTokens + tokensClaimed
            );
            (bool success, ) = swapTarget.call(zeroExTradeData);
            require(success, "SpigotCnsm: trade failed");
        }

        uint256 targetTokens = IERC20(targetToken).balanceOf(address(this));

        // ideally we could use oracle to calculate # of tokens to receive
        // but claimToken might not have oracle. targetToken must have oracle

        // underflow revert ensures we have more tokens than we started with
        tokensBought = targetTokens - existingTargetTokens;

        emit TradeSpigotRevenue(
            claimToken,
            tokensClaimed,
            targetToken,
            tokensBought
        );

        // update unused if we didnt sell all claimed tokens in trade
        // also underflow revert protection here
        unusedTokens[claimToken] +=
            IERC20(claimToken).balanceOf(address(this)) -
            existingClaimTokens;
    }

\color{black} \color{white} This flaw impacts the sweep() function, which reverts as the contract's balance has fewer tokens than recorded in the unusedTokens collection. Thus, the tokens are locked in the contract.sweep()``unusedTokens

SpigotedLoan.sol

    function sweep(address token) external returns (uint256) {
        if (loanStatus == LoanLib.STATUS.REPAID) {
            return _sweep(borrower, token);
        }
        if (loanStatus == LoanLib.STATUS.INSOLVENT) {
            return _sweep(arbiter, token);
        }

        return 0;
    }

    function _sweep(address to, address token) internal returns (uint256 x) {
        x = unusedTokens[token];
        if (token == address(0)) {
            payable(to).transfer(x);
        } else {
            require(IERC20(token).transfer(to, x));
        }
        delete unusedTokens[token];
    }

1. All necessary contracts are deployed and initialized: CreditToken, RevenueToken, SimpleOracle, LoanLib, SpigotedLoan, SimpleRevenueContract. Note that SimpleRevenueContract must earn in credit tokens.
2. As borrower and lender1, add credit position for 10_000_000_000_000_000 tokens.
3. As borrower, borrow() half of the deposit.
4. As borrower and arbiter, add new spigot with addSpigot() function and RevenueContract as input. Set the ownerSplit parameter to 100.
5. As the borrower, transfer the ownership of the RevenueContract contract to the SpigotController.
6. Mint 10_000_000_000 credit tokens to the RevenueContract contract to simulate token gain.
7. As the borrower, call claimRevenue() in SpigotController contract to claim revenue tokens. Note that 100% of tokens (10_000_000_000) are transferred to the SpigotController.
8. As the borrower, call claimAndTrade() in SpigotedLoan contract to trade escrowed credit tokens. As an input, trade exchange data provide 10_000_000_000 revenue tokens (credit tokens) that should be exchanged for credit tokens.
9. Observe the SpigotedLoan balances. Note that the contract has 500,010,000,000,000 credit tokens. Also, 20,000,000,000 credit tokens are stored in unusedTokens collection.
10. As the borrower, call depositAndClose(). Observe that credit is paid off. Note that the contract has 10,000,000,000 credit tokens.
11. As the borrower, attempt to call sweep() function with credit token address as an input. Note that the function reverts the ERC20: transfer amount exceeds balance error.1. All necessary contracts are deployed and initialized: CreditToken, RevenueToken, SimpleOracle, LoanLib, SpigotedLoan, SimpleRevenueContract. Note that SimpleRevenueContract must earn in credit tokens. SimpleRevenueContract``credit tokens2. As borrower and lender1, add credit position for 10_000_000_000_000_000 tokens.
12. As borrower, borrow() half of the deposit. borrow()2. As borrower and arbiter, add new spigot with addSpigot() function and RevenueContract as input. Set the ownerSplit parameter to 100. addSpigot()``RevenueContract``ownerSplit``1003. As the borrower, transfer the ownership of the RevenueContract contract to the SpigotController. RevenueContract4. Mint 10_000_000_000 credit tokens to the RevenueContract contract to simulate token gain. credit tokens``RevenueContract5. As the borrower, call claimRevenue() in SpigotController contract to claim revenue tokens. Note that 100% of tokens (10_000_000_000) are transferred to the SpigotController. claimRevenue()6. As the borrower, call claimAndTrade() in SpigotedLoan contract to trade escrowed credit tokens. As an input, trade exchange data provide 10_000_000_000 revenue tokens (credit tokens) that should be exchanged for credit tokens. claimAndTrade()7. Observe the SpigotedLoan balances. Note that the contract has 500,010,000,000,000 credit tokens. Also, 20,000,000,000 credit tokens are stored in unusedTokens collection. unusedTokens8. As the borrower, call depositAndClose(). Observe that credit is paid off. Note that the contract has 10,000,000,000 credit tokens. depositAndClose()9. As the borrower, attempt to call sweep() function with credit token address as an input. Note that the function reverts the ERC20: transfer amount exceeds balance error.sweep()``ERC20: transfer amount exceeds balance

\

SOLVED: The Debt DAO team solved this issue in commit 760c5bfb9c352fb681f8351253ff9776b176e357: the solution now updates the unusedTokens only once.SOLVED:Debt DAO team760c5bfb9c352fb681f8351253ff9776b176e357unusedTokens

\

In the EscrowedLoan contract, the _healthcheck() internal function is supposed to check if the loan has the correct collateral ratio. Otherwise, the loan status is set to LIQUIDATABLE. The _healthcheck() function base on the _getLatestCollateralRatio() function, which returns 0 for empty collateral. Therefore, calling the healthcheck() function at the beginning of the loan's life cycle will set the EscrowedLoan contract's status to LIQUIDATABLE.EscrowedLoan``_healthcheck()``LIQUIDATABLE``_healthcheck()``_getLatestCollateralRatio()``healthcheck()``EscrowedLoan``LIQUIDATABLE

When contract has LIQUIDATABLE status, several spigot's functions can be called without authorisation: releaseSpigot(), updateOwnerSplit(), sweep(). As a result, the loan can get malformed state: LIQUIDATABLE``releaseSpigot()``updateOwnerSplit()``sweep()- the spigot's owner can be set to the arbiter, and it will not be usable by EscrowedLoan (SpigotedLoan), - the spigot's split can be set to 100%, - the unused tokens from EscrowedLoan (SpigotedLoan) can be transferred to the arbiter if only some tokens were stored in the contract.- the spigot's owner can be set to the arbiter, and it will not be usable by EscrowedLoan (SpigotedLoan), - the spigot's split can be set to 100%, - the unused tokens from EscrowedLoan (SpigotedLoan) can be transferred to the arbiter if only some tokens were stored in the contract.

The borrower can set the contract's status back to ACTIVE by adding collateral and repeatedly calling the healthcheck() function.ACTIVE``healthcheck()

EscrowedLoan.sol

  function _healthcheck() virtual internal returns(LoanLib.STATUS) {
    if(escrow.getCollateralRatio() < escrow.minimumCollateralRatio()) {
      return LoanLib.STATUS.LIQUIDATABLE;
    }

    return LoanLib.STATUS.ACTIVE;
  }

Escrow.sol

    /**
     * @notice calculates the cratio
     * @dev callable by anyone
     * @return - the calculated cratio
     */
    function getCollateralRatio() external returns (uint256) {
        return _getLatestCollateralRatio();
    }

Escrow.sol

    /**
     * @notice updates the cratio according to the collateral value vs loan value
     * @dev calls accrue interest on the loan contract to update the latest interest payable
     * @return the updated collateral ratio in 18 decimals
     */
    function _getLatestCollateralRatio() internal returns (uint256) {
        ILoan(loan).accrueInterest();
        uint256 debtValue = ILoan(loan).getOutstandingDebt();
        uint256 collateralValue = _getCollateralValue();
        if (collateralValue == 0) return 0;
        if (debtValue == 0) return MAX_INT;

        return _percent(collateralValue, debtValue, 18);
    }

1. All necessary contracts are deployed and initialized: CreditToken, RevenueToken, SimpleOracle, LoanLib, EscrowedLoan.
2. As borrower and lender, add credit position.
3. As borrower and arbiter, add new spigot with addSpigot() function and RevenueContract as input. Set the ownerSplit parameter to 10.
4. As the borrower, transfer the ownership of the RevenueContract contract to the SpigotController.
5. As the attacker, call thehealthcheck() function. Note that the loan's status is set to LIQUIDATABLE.
6. As the arbiter, enable the RevenueToken token as collateral.
7. As the borrower, add 200_000_000_000_000 of RevenueToken tokens as collateral.
8. As the borrower, attempt to borrow all deposit. Observe that transaction reverts due to the (Loan: no op) error.1. All necessary contracts are deployed and initialized: CreditToken, RevenueToken, SimpleOracle, LoanLib, EscrowedLoan.
9. As borrower and lender, add credit position.
10. As borrower and arbiter, add new spigot with addSpigot() function and RevenueContract as input. Set the ownerSplit parameter to 10. addSpigot()``RevenueContract``ownerSplit``104. As the borrower, transfer the ownership of the RevenueContract contract to the SpigotController. RevenueContract5. As the attacker, call thehealthcheck() function. Note that the loan's status is set to LIQUIDATABLE. healthcheck()``LIQUIDATABLE6. As the arbiter, enable the RevenueToken token as collateral. RevenueToken7. As the borrower, add 200_000_000_000_000 of RevenueToken tokens as collateral. RevenueToken8. As the borrower, attempt to borrow all deposit. Observe that transaction reverts due to the (Loan: no op) error.borrow``(Loan: no op)

1. As the attacker, call updateOwnerSplit() function. Observe that owner split is now set to 100.
2. As the attacker, call releaseSpigot() function. Observe that spigot's owner's address has been changed.
3. As the borrower, call thehealthcheck() function. Note that the loan's status is set to ACTIVE.
4. As the borrower, borrow all deposits. Observe that transaction finishes successfully.9. As the attacker, call updateOwnerSplit() function. Observe that owner split is now set to 100. updateOwnerSplit()``10010. As the attacker, call releaseSpigot() function. Observe that spigot's owner's address has been changed. releaseSpigot()11. As the borrower, call thehealthcheck() function. Note that the loan's status is set to ACTIVE. healthcheck()``ACTIVE12. As the borrower, borrow all deposits. Observe that transaction finishes successfully.borrow

\

RISK ACCEPTED: The Debt DAO accepted the risk of this finding. RISK ACCEPTED:Debt DAO

\

The SecuredLoan can get the LIQUIDATABLE status in two cases: when the loan's deadline has passed or when the collateral ratio is below the threshold. When the loan has the LIQUIDATABLE status, the arbiter can take control over the spigots and collateral and use the released funds to repay the debt for the lender. The assessment revealed that in some cases, the arbiter could not liquidate the collateral when the loan's deadline has passed, and the loan gets the LIQUIDATABLE status. The Escrow's liquidate() function has an assertion that checks if the collateral ratio value is below the threshold. The collateral ratio decreases while the loan's interest increases. Thus, it may take time to get the ratio low enough. Additionally, the borrower can still release part of the collateral within this time.SecuredLoan``LIQUIDATABLE``LIQUIDATABLE``LIQUIDATABLE``liquidate()

SecuredLoan.sol

  // Liquidation
  /**
   * @notice - Forcefully take collateral from borrower and repay debt for lender
   * @dev - only called by neutral arbiter party/contract
   * @dev - `loanStatus` must be LIQUIDATABLE
   * @dev - callable by `arbiter`
   * @param positionId -the debt position to pay down debt on
   * @param amount - amount of `targetToken` expected to be sold off in  _liquidate
   * @param targetToken - token in escrow that will be sold of to repay position
   */

  function liquidate(
    bytes32 positionId,
    uint256 amount,
    address targetToken
  )
    external
    returns(uint256)
  {
    require(msg.sender == arbiter);

    _updateLoanStatus(_healthcheck());

    require(loanStatus == LoanLib.STATUS.LIQUIDATABLE, "Loan: not liquidatable");

    // send tokens to arbiter for OTC sales
    return _liquidate(positionId, amount, targetToken, msg.sender);
  }

EscrowedLoan.sol

  function _liquidate(
    bytes32 positionId,
    uint256 amount,
    address targetToken,
    address to
  )
    virtual internal
    returns(uint256)
  { 
    require(escrow.liquidate(amount, targetToken, to));

    emit Liquidate(positionId, amount, targetToken);

    return amount;
  }

Escrow.sol

/**
     * @notice liquidates borrowers collateral by token and amount
     * @dev requires that the cratio is at or below the liquidation threshold
     * @dev callable by `loan`
     * @param amount - the amount of tokens to liquidate
     * @param token - the address of the token to draw funds from
     * @param to - the address to receive the funds
     * @return - true if successful
     */
    function liquidate(
        uint256 amount,
        address token,
        address to
    ) external returns (bool) {
        require(amount > 0, "Escrow: amount is 0");
        require(
            msg.sender == loan,
            "Escrow: msg.sender must be the loan contract"
        );
        require(
            minimumCollateralRatio > _getLatestCollateralRatio(),
            "Escrow: not eligible for liquidation"
        );
        require(
            deposited[token].amount >= amount,
            "Escrow: insufficient balance"
        );

        deposited[token].amount -= amount;
        require(IERC20(token).transfer(to, amount));

        emit Liquidate(token, amount);

        return true;
    }

1. All necessary contracts are deployed and initialized: CreditToken, RevenueToken, SimpleOracle, LoanLib, EscrowedLoan. Set the ttl parameter to 1 day. Set the minimumCollateralRatio parameter to 10%.
2. As the borrower and lender1, add credit position for 1_000_000_000_000_000 tokens withdrawn rate set to 10000 and facility rate set to 100.
3. As the arbiter, enable the RevenueToken token as collateral.
4. As the borrower, add 150_000_000_000_000 of RevenueToken tokens as collateral.
5. As the borrower, borrow all deposits. Note that the collateral ratio value is 15%``.1. All necessary contracts are deployed and initialized: CreditToken, RevenueToken, SimpleOracle, LoanLib, EscrowedLoan. Set thettlparameter to1 day. Set theminimumCollateralRatioparameter to10%.ttl1 dayminimumCollateralRatio10%`2. As the borrower and lender1, add credit position for 1_000_000_000_000_000 tokens withdrawn rate set to `10000` and facility rate set to `100`. `100001003. As the arbiter, enable theRevenueTokentoken as collateral.RevenueToken4. As the borrower, add 150_000_000_000_000 ofRevenueTokentokens as collateral.RevenueToken5. As the borrower,borrowall deposits. Note that the collateral ratio value is15%`.borrow`

1. Forward blockchain time for 1 day and 1 second.
2. As the arbiter, call thehealthcheck() function. Note that the loan's status is set to LIQUIDATABLE.
3. As the arbiter, attempt to call liquidate() function. Observe that the transaction reverts with the Escrow: not eligible for liquidation error.
4. As the borrower, call releaseCollateral() function. Observe that transaction finishes successfully. Note that the collateral ratio value is `14,9%``.6. Forward blockchain time for 1 day and 1 second.
5. As the arbiter, call thehealthcheck() function. Note that the loan's status is set to LIQUIDATABLE. healthcheck()``LIQUIDATABLE8. As the arbiter, attempt to call liquidate() function. Observe that the transaction reverts with the Escrow: not eligible for liquidation error. liquidate()``Escrow: not eligible for liquidation9. As the borrower, call releaseCollateral() function. Observe that transaction finishes successfully. Note that the collateral ratio value is 14,9%``.releaseCollateral()`

1. Forward blockchain time for 182 days. Note that the collateral ratio value is below 10%.
2. As the arbiter, call liquidate() function. Observe that the transaction finishes successfully.10. Forward blockchain time for 182 days. Note that the collateral ratio value is below 10%. 182 days``10%11. As the arbiter, call liquidate() function. Observe that the transaction finishes successfully.liquidate()

\

SOLVED: The Debt DAO team solved this issue in commit c882e7f2891efa4a053f28c0c5d4a439f694a397: the solution now allows the function liquidate() to be called when the loan status is set to LIQUIDATABLE regardless of the value of the collateral ratio.SOLVED:Debt DAO teamc882e7f2891efa4a053f28c0c5d4a439f694a397liquidate()``LIQUIDATABLE

\

In the LineOfCredit contract, a borrower and lender can add credit. The algorithm assumes that unused funds accrue interest based on the facility rate (frate parameter). There is no requirement that the borrower must borrow the deposit upon closing the credit. The close(id) function does not call _accrueInterest() internal function. Hence, closing credit without accruing interest from unused funds is possible.LineOfCredit``frate``close(id)``_accrueInterest()

Scenario 1 - without accrueInterest function calling``Scenario 1 - without accrueInterest function calling

1. All necessary contracts are deployed and initialized: RevenueToken, SimpleOracle, LoanLib, LineOfCredit.
2. As borrower and lender1, add credit position for 10_000_000_000_000_000 tokens with facility rate set to 1.
3. Forward blockchain time for 10 days.
4. As the borrower, close the credit. Observe that credit is closed without accruing interest from unused funds.1. All necessary contracts are deployed and initialized: RevenueToken, SimpleOracle, LoanLib, LineOfCredit.
5. As borrower and lender1, add credit position for 10_000_000_000_000_000 tokens with facility rate set to 1.
6. Forward blockchain time for 10 days.
7. As the borrower, close the credit. Observe that credit is closed without accruing interest from unused funds.close

Scenario 2 - with accrueInterest function calling``Scenario 2 - with accrueInterest function calling

1. All necessary contracts are deployed and initialized: RevenueToken, SimpleOracle, LoanLib, LineOfCredit.
2. As borrower and lender1, add credit position for 10_000_000_000_000_000 tokens with facility rate set to 1.
3. Forward blockchain time for 10 days.
4. Call accrueInterest() function. Note that the interestAccrued() parameter value in first credit record is updated.
5. As the borrower, attempt to close() the credit. Observe that the transaction reverted with Loan: close failed. credit owed error.1. All necessary contracts are deployed and initialized: RevenueToken, SimpleOracle, LoanLib, LineOfCredit.
6. As borrower and lender1, add credit position for 10_000_000_000_000_000 tokens with facility rate set to 1.
7. Forward blockchain time for 10 days.
8. Call accrueInterest() function. Note that the interestAccrued() parameter value in first credit record is updated. accrueInterest()``interestAccrued()``credit5. As the borrower, attempt to close() the credit. Observe that the transaction reverted with Loan: close failed. credit owed error.close()``Loan: close failed. credit owed

\

SOLVED: The Debt DAO team solved this issue in commit 1908ded5d604984fd1a6c52af360c9135582f175: the solution now forces the borrower to pay accrued interest on unused funds by calling the close() function.SOLVED:Debt DAO team1908ded5d604984fd1a6c52af360c9135582f175close()

\

In the LineOfCredit contract, the borrower has two possibilities to pay off the debt: by calling depositAndRepay() and then close() functions, or by calling a single depositAndClose() function. The close() function combined with depositAndRepay() should allow the borrower to close the debit. The close(id) function does not call _accrueInterest() internal function.LineOfCredit``depositAndRepay()``close()``depositAndClose()``close()``depositAndRepay()``close(id)``_accrueInterest()

The assessment revealed that the lender can front-run close() function called by the borrower with the accrueInterest() function. As a result, the close() function will revert, and a small amount of facility interest will be added to the debt. Eventually, the borrower could escape from the situation by using the depositAndClose() function.close()``accrueInterest()``close()``depositAndClose()

1. All necessary contracts are deployed and initialized: RevenueToken, SimpleOracle, LoanLib, LineOfCredit.
2. As borrower and lender, add credit position for 10_000_000_000_000_000 tokens with facility rate set to 1.
3. As borrower, borrow() all deposits.
4. As the borrower, pay off the debt and interest using the depositAndRepay() function.
5. As borrower, attempt to close() the credit, but firstly call accrueInterest() function as a lender to simulate front-running.
6. Observe that close() function reverts with Loan: close failed. credit owed error. Note that a small amount of facility rate is accrued.1. All necessary contracts are deployed and initialized: RevenueToken, SimpleOracle, LoanLib, LineOfCredit.
7. As borrower and lender, add credit position for 10_000_000_000_000_000 tokens with facility rate set to 1.
8. As borrower, borrow() all deposits. borrow()4. As the borrower, pay off the debt and interest using the depositAndRepay() function. depositAndRepay()5. As borrower, attempt to close() the credit, but firstly call accrueInterest() function as a lender to simulate front-running. close()``accrueInterest()6. Observe that close() function reverts with Loan: close failed. credit owed error. Note that a small amount of facility rate is accrued.close()``Loan: close failed. credit owed

\

SOLVED: The Debt DAO team solved this issue in commit 1908ded5d604984fd1a6c52af360c9135582f175: the solution now forces the borrower to pay accrued interest on unused funds by calling the close() function.SOLVED:Debt DAO team1908ded5d604984fd1a6c52af360c9135582f175close()

\

In the SpigotedLoan contract, the borrower can add spigots with revenue contracts that will support repayment of the debt. The revenue contract earns revenue tokens that later can be exchanged for the credit tokens using the claimAndRepay() and claimAndTrade() functions. These functions call the _claimAndTrade() internal function responsible for claiming escrowed tokens from the SpigotController contract and trading the claimed tokens. Also, the claimAndTrade() function updates the unusedTokens collection with target tokens (credit tokens) that were traded.SpigotedLoan``claimAndRepay()``claimAndTrade()``_claimAndTrade()``SpigotController``claimAndTrade()``unusedTokens

SpigotedLoan.sol

    function claimAndTrade(address claimToken, bytes calldata zeroExTradeData)
        external
        whileBorrowing
        returns (uint256 tokensBought)
    {
        require(msg.sender == borrower || msg.sender == arbiter);

        address targetToken = credits[ids[0]].token;
        tokensBought = _claimAndTrade(
            claimToken,
            targetToken,
            zeroExTradeData
        );

        // add bought tokens to unused balance
        unusedTokens[targetToken] += tokensBought;
    }

In the SpigotController contract, the _claimRevenue() internal function is responsible for calculating the claimed token value. However, it reverts when no tokens can be claimed (line 140). The assessment revealed that it is not possible to use traded credit tokens to pay off the debt until new revenue is claimed in the SpigotController contract. The SpigotController contract gains tokens from the revenue contract, which is a third-party component; thus, the point of time of new revenue arrival is uncertain. The borrower can also abuse this vulnerability to trade revenue tokens but not use them to pay off the debt, making the spigot mechanism ineffective. SpigotController``_claimRevenue()``SpigotController``SpigotController

Spigot.sol

function _claimRevenue(address revenueContract, bytes calldata data, address token)
        internal
        returns (uint256 claimed)
    {
        uint256 existingBalance = _getBalance(token);
        if(settings[revenueContract].claimFunction == bytes4(0)) {
            // push payments
            // claimed = total balance - already accounted for balance
            claimed = existingBalance - escrowed[token];
        } else {
            // pull payments
            require(bytes4(data) == settings[revenueContract].claimFunction, "Spigot: Invalid claim function");
            (bool claimSuccess, bytes memory claimData) = revenueContract.call(data);
            require(claimSuccess, "Spigot: Revenue claim failed");
            // claimed = total balance - existing balance
            claimed = _getBalance(token) - existingBalance;
        }

        require(claimed > 0, "Spigot: No revenue to claim");
        if(claimed > MAX_REVENUE) claimed = MAX_REVENUE;

        return claimed;
    }

1. All necessary contracts are deployed and initialized: CreditToken, RevenueToken, SimpleOracle, LoanLib, SpigotedLoan, SimpleRevenueContract.
2. As borrower and lender1, add credit position for 10_000_000_000_000_000 tokens.
3. As borrower, borrow() all deposits.
4. As borrower and arbiter, add new spigot with addSpigot() function and RevenueContract as input. Set the ownerSplit parameter to 10.
5. As the borrower, transfer the ownership of the RevenueContract contract to the SpigotController.
6. Mint 500_000_000_000_000 revenue tokens to the RevenueContract contract to simulate token gain.
7. As the borrower, call claimRevenue() in SpigotController contract to claim revenue tokens. Note that 90% of tokens (450_000_000_000_000) are transferred to the treasury (which is the borrower), the 10% (50_000_000_000_000) of tokens are transferred to the SpigotController.1. All necessary contracts are deployed and initialized: CreditToken, RevenueToken, SimpleOracle, LoanLib, SpigotedLoan, SimpleRevenueContract.
8. As borrower and lender1, add credit position for 10_000_000_000_000_000 tokens.
9. As borrower, borrow() all deposits. borrow()2. As borrower and arbiter, add new spigot with addSpigot() function and RevenueContract as input. Set the ownerSplit parameter to 10. addSpigot()``RevenueContract``ownerSplit``103. As the borrower, transfer the ownership of the RevenueContract contract to the SpigotController. RevenueContract4. Mint 500_000_000_000_000 revenue tokens to the RevenueContract contract to simulate token gain. RevenueContract5. As the borrower, call claimRevenue() in SpigotController contract to claim revenue tokens. Note that 90% of tokens (450_000_000_000_000) are transferred to the treasury (which is the borrower), the 10% (50_000_000_000_000) of tokens are transferred to the SpigotController.claimRevenue()

1. As the borrower, call claimAndTrade() in the SpigotedLoan contract to trade escrowed revenue tokens. As an input, trade exchange data provide 50_000_000_000_000 revenue tokens that should be exchanged for credit tokens.6. As the borrower, call claimAndTrade() in the SpigotedLoan contract to trade escrowed revenue tokens. As an input, trade exchange data provide 50_000_000_000_000 revenue tokens that should be exchanged for credit tokens.claimAndTrade()

1. Observe the SpigotedLoan balances. Note that the contract has 50,000,000,000,000 credit tokens. Also 50,000,000,000,000 credit tokens are stored in unusedTokens collection.

2. As the borrower, attempt to call claimAndTrade() or claimAndRepay() functions. Observe that these functions revert with the error Spigot: No escrow to claim.7. Observe the SpigotedLoan balances. Note that the contract has 50,000,000,000,000 credit tokens. Also 50,000,000,000,000 credit tokens are stored in unusedTokens collection.

unusedTokens8. As the borrower, attempt to call claimAndTrade() or claimAndRepay() functions. Observe that these functions revert with the error Spigot: No escrow to claim.claimAndTrade()``claimAndRepay()``Spigot: No escrow to claim

1. Mint 1000 revenue tokens to the RevenueContract contract to simulate token gain.
2. As the borrower, call claimRevenue() in the SpigotController contract to claim revenue tokens.
3. As the borrower, call claimAndRepay(). Observe that all unused tokens were used to pay off part of the debt.9. Mint 1000 revenue tokens to the RevenueContract contract to simulate token gain. RevenueContract10. As the borrower, call claimRevenue() in the SpigotController contract to claim revenue tokens. claimRevenue()11. As the borrower, call claimAndRepay(). Observe that all unused tokens were used to pay off part of the debt.claimAndRepay()

\

SOLVED: The Debt DAO team solved this issue in commit 1908ded5d604984fd1a6c52af360c9135582f175: the solution now allows all traded credit tokens to be used via the useAndRepay() function.SOLVED:Debt DAO team1908ded5d604984fd1a6c52af360c9135582f175useAndRepay()

\

In the SpigotController, the claimRevenue() function allows claiming revenue from the revenue contract, split it between treasury and escrow, and send part of it to the treasury. By default, the treasury is the borrower. The ownerSplit parameter is the initial split agreed upon and set up by the borrower and the arbiter. When the credit deadline has passed, and it has defaulted, the healthcheck() function from the LineOfCredit contract must be called explicitly to set loan status to LIQUIDATABLE. Afterward, the arbiter can call the updateOwnerSplit() function from the SpigotedLoan contract to update the ownerSplit parameter, so 100% of revenue is escrowed, and none is sent to the treasury.SpigotController``claimRevenue()``ownerSplit``healthcheck()``LineOfCredit``LIQUIDATABLE``updateOwnerSplit()``SpigotedLoan``ownerSplit

Spigot.sol

    /**

     * @notice - Claim push/pull payments through Spigots.
                 Calls predefined function in contract settings to claim revenue.
                 Automatically sends portion to treasury and escrows Owner's share.
     * @dev - callable by anyone
     * @param revenueContract Contract with registered settings to claim revenue from
     * @param data  Transaction data, including function signature, to properly claim revenue on revenueContract
     * @return claimed -  The amount of tokens claimed from revenueContract and split in payments to `owner` and `treasury`
    */
    function claimRevenue(address revenueContract, bytes calldata data)
        external nonReentrant
        returns (uint256 claimed)
    {
        address token = settings[revenueContract].token;
        claimed = _claimRevenue(revenueContract, data, token);

        // split revenue stream according to settings
        uint256 escrowedAmount = claimed * settings[revenueContract].ownerSplit / 100;
        // update escrowed balance
        escrowed[token] = escrowed[token] + escrowedAmount;

        // send non-escrowed tokens to Treasury if non-zero
        if(claimed > escrowedAmount) {
            require(_sendOutTokenOrETH(token, treasury, claimed - escrowedAmount));
        }

        emit ClaimRevenue(token, claimed, escrowedAmount, revenueContract);

        return claimed;
    }

SpigotedLoan.sol

    /**
     * @notice changes the revenue split between borrower treasury and lan repayment based on loan health
     * @dev    - callable `arbiter` + `borrower`
     * @param revenueContract - spigot to update
     */
    function updateOwnerSplit(address revenueContract) external returns (bool) {
        (, uint8 split, , bytes4 transferFunc) = spigot.getSetting(
            revenueContract
        );

        require(transferFunc != bytes4(0), "SpgtLoan: no spigot");

        if (
            loanStatus == LoanLib.STATUS.ACTIVE && split != defaultRevenueSplit
        ) {
            // if loan is healthy set split to default take rate
            spigot.updateOwnerSplit(revenueContract, defaultRevenueSplit);
        } else if (
            loanStatus == LoanLib.STATUS.LIQUIDATABLE && split != MAX_SPLIT
        ) {
            // if loan is in distress take all revenue to repay loan
            spigot.updateOwnerSplit(revenueContract, MAX_SPLIT);
        }

        return true;
    }

\color{black} \color{white} The assessment revealed that the loan's LIQUIDATABLE status is not propagated to the spigots. Furthermore, claimRevenue() function has no authorization implemented. As a result, the borrower can front-run the updateOwnerSplit() function with the claimRevenue() to obtain one more revenue share from spigot. LIQUIDATABLE``claimRevenue()``updateOwnerSplit()``claimRevenue()

1. All necessary contracts are deployed and initialized: CreditToken, RevenueToken, SimpleOracle, LoanLib, SpigotedLoan, SimpleRevenueContract. Set the ttl parameter to 1 day.
2. As borrower and lender1, add credit position for 10_000_000_000_000_000 tokens.
3. As the borrower borrow() half of the deposit - 5_000_000_000_000_000.
4. As borrower and arbiter, add a new spigot with the addSpigot() function and Set the ownerSplit parameter to 10.
5. As the borrower, transfer the ownership of the RevenueContract contract to the SpigotController.
6. Mint 500_000_000_000_000 revenue tokens to the RevenueContract contract to simulate token gain.
7. Forward blockchain time for 1 day and 1 second.
8. As the borrower, attempt to borrow() the remaining deposit. Observe that the transaction reverted with the Loan: can't borrow error.1. All necessary contracts are deployed and initialized: CreditToken, RevenueToken, SimpleOracle, LoanLib, SpigotedLoan, SimpleRevenueContract. Set the ttl parameter to 1 day. ttl2. As borrower and lender1, add credit position for 10_000_000_000_000_000 tokens.
9. As the borrower borrow() half of the deposit - 5_000_000_000_000_000. borrow()2. As borrower and arbiter, add a new spigot with the addSpigot() function and Set the ownerSplit parameter to 10. addSpigot()``ownerSplit``103. As the borrower, transfer the ownership of the RevenueContract contract to the SpigotController. RevenueContract4. Mint 500_000_000_000_000 revenue tokens to the RevenueContract contract to simulate token gain. RevenueContract5. Forward blockchain time for 1 day and 1 second.
10. As the borrower, attempt to borrow() the remaining deposit. Observe that the transaction reverted with the Loan: can't borrow error.borrow()``Loan: can't borrow

1. As the arbiter, call the healthcheck() function. Note that the loan's status changed from ACTIVE to LIQUIDATABLE.
2. As the borrower, call claimRevenue() to simulate front-run of the updateOwnerSplit() function, so 90% of revenue will go to the borrower and 10% go to the SpigotController contract.7. As the arbiter, call the healthcheck() function. Note that the loan's status changed from ACTIVE to LIQUIDATABLE. healthcheck()``ACTIVE``LIQUIDATABLE8. As the borrower, call claimRevenue() to simulate front-run of the updateOwnerSplit() function, so 90% of revenue will go to the borrower and 10% go to the SpigotController contract.claimRevenue()``updateOwnerSplit()

\

RISK ACCEPTED: The Debt DAO accepted the risk of this finding. RISK ACCEPTED:Debt DAO

\

In the Escrow contract, the minimumCollateralRatio parameter defines the minimum threshold of collateral ratio that allows the borrower to borrow the deposit (using the _healthcheck() function). Basing on project's documentation this parameter is expected to have 18 decimals, e.g. 1 ether = 100%. However, the contract does not implement any input validation, thus prone to human errors. A user could set too small a value, e.g., 100, which would make the mechanism ineffective. No function to update the minimumCollateralRatio parameter is available in the contract.Escrow``minimumCollateralRatio``_healthcheck()project's documentationminimumCollateralRatio

Escrow.sol

    // the minimum value of the collateral in relation to the outstanding debt e.g. 10% of outstanding debt
    uint256 public minimumCollateralRatio;

Escrow.sol

    constructor(
        uint256 _minimumCollateralRatio,
        address _oracle,
        address _loan,
        address _borrower
    ) public {
        minimumCollateralRatio = _minimumCollateralRatio;
        oracle = _oracle;
        loan = _loan;
        borrower = _borrower;
    }

EscrowedLoan.sol

  function _healthcheck() virtual internal returns(LoanLib.STATUS) {
    if(escrow.getCollateralRatio() < escrow.minimumCollateralRatio()) {
      return LoanLib.STATUS.LIQUIDATABLE;
    }

    return LoanLib.STATUS.ACTIVE;
  }

1. All necessary contracts are deployed and initialized: CreditToken, RevenueToken, SimpleOracle, LoanLib, EscrowedLoan. For the minimumCollateralRatio parameter in the EscrowedLoan contract, provide a small value, e.g. 100.
2. Observe that the EscrowedLoan contract deployed successfully.
3. As any account, call the minimumCollateralRatio() function. Observe the result, note that this confirms the contract's constructor appceted too low value for the minimumCollateralRatio parameter.1. All necessary contracts are deployed and initialized: CreditToken, RevenueToken, SimpleOracle, LoanLib, EscrowedLoan. For the minimumCollateralRatio parameter in the EscrowedLoan contract, provide a small value, e.g. 100. minimumCollateralRatio``EscrowedLoan2. Observe that the EscrowedLoan contract deployed successfully. EscrowedLoan3. As any account, call the minimumCollateralRatio() function. Observe the result, note that this confirms the contract's constructor appceted too low value for the minimumCollateralRatio parameter.minimumCollateralRatio()``minimumCollateralRatio

\

SOLVED: The Debt DAO team solved this issue in commit f07592950e62c9e797ab4e30d17f02a74d4165c6: the _minimumCollateralRatio parameter now uses the uint32 type. The logic is updated, so the parameters use only 2 decimal points, e.g., 10000 = 100%. Such an approach minimizes human error only by setting a collateral of 0.01%. SOLVED:Debt DAO teamf07592950e62c9e797ab4e30d17f02a74d4165c6_minimumCollateralRatio

\

In the Spigot contract, the removeSpigot() function allows the contract's owner to transfer tokens held by the spigot, and transfer the ownership of the revenueContract to the operator (the borrower) and remove the revenueContract's data from spigot's settings collection. The amount of tokens to transfer is based on the escrowed collection. The Spigot allows multiple revenueContract contracts, and such contracts can provide revenue in the same revenue tokens. In such circumstances, the first call to the removeSpigot() function will transfer all escrowed tokens from every related revenueContract, but escrowed[token] will not be reset. Any subsequent call will revert, as a function would attempt to transfer escrowed tokens with an empty contract's balance. As a result, transferring the ownership of the remaining revenueContract will not be possible. However, the contract's owner could escape from this situation by transferring the tokens back to the contract, which is cumbersome. Spigot``removeSpigot()``revenueContract``escrowed``Spigot``revenueContract``removeSpigot()``revenueContract``escrowed[token]``revenueContract

Spigot.sol

    function removeSpigot(address revenueContract) external returns (bool) {
        require(msg.sender == owner);

        address token = settings[revenueContract].token;
        uint256 claimable = escrowed[token];
        if(claimable > 0) {
            require(_sendOutTokenOrETH(token, owner, claimable));
            emit ClaimEscrow(token, claimable, owner);
        }

        (bool success, bytes memory callData) = revenueContract.call(
            abi.encodeWithSelector(
                settings[revenueContract].transferOwnerFunction,
                operator    // assume function only takes one param that is new owner address
            )
        );
        require(success);

        delete settings[revenueContract];
        emit RemoveSpigot(revenueContract, token);

        return true;
    }

1. All necessary contracts are deployed and initialized: CreditToken, RevenueToken, SimpleOracle, LoanLib, SpigotedLoan.
2. Deploy the first SimpleRevenueContract with RevenueToken.
3. Deploy the second SimpleRevenueContract with RevenueToken.
4. As borrower and lender, add credit position for 1_000_000_000_000_000 tokens.
5. As borrower and arbiter, add the first spigot with addSpigot() function and the first SimpleRevenueContract as input.
6. As borrower and arbiter, add a second spigot with addSpigot() function and the second SimpleRevenueContract as input.
7. As the borrower, transfer the ownership of the first SimpleRevenueContract contract to the SpigotController.
8. As the borrower, transfer the ownership of the second SimpleRevenueContract contract to the SpigotController.
9. Mint 10_000_000_000_000 revenue tokens to the first SimpleRevenueContract contract to simulate token gain.
10. Mint 10_000_000_000_000 revenue tokens to the second SimpleRevenueContract contract to simulate token gain.
11. As the borrower, borrow() all deposits.
12. As borrower, claim revenue for the first SimpleRevenueContract contract in SpigotController.
13. As borrower, claim revenue for the second SimpleRevenueContract contract in SpigotController.
14. As the borrower, pay off the debt.
15. As a borrower, release the spigot in the SpigotedLoan controller.
16. As borrower, call removeSpigot() function with the first SimpleRevenueContract contract as an input.
17. As borrower, attempt to call removeSpigot() function with the second SimpleRevenueContract contract as an input. Observe that the transaction reverts with the ERC20: transfer amount exceeds balance error. Note that the contract's ownership still belongs to the SpigotController.1. All necessary contracts are deployed and initialized: CreditToken, RevenueToken, SimpleOracle, LoanLib, SpigotedLoan.
18. Deploy the first SimpleRevenueContract with RevenueToken. SimpleRevenueContract3. Deploy the second SimpleRevenueContract with RevenueToken. SimpleRevenueContract4. As borrower and lender, add credit position for 1_000_000_000_000_000 tokens.
19. As borrower and arbiter, add the first spigot with addSpigot() function and the first SimpleRevenueContract as input. addSpigot()``SimpleRevenueContract6. As borrower and arbiter, add a second spigot with addSpigot() function and the second SimpleRevenueContract as input. addSpigot()``SimpleRevenueContract7. As the borrower, transfer the ownership of the first SimpleRevenueContract contract to the SpigotController. SimpleRevenueContract8. As the borrower, transfer the ownership of the second SimpleRevenueContract contract to the SpigotController. SimpleRevenueContract9. Mint 10_000_000_000_000 revenue tokens to the first SimpleRevenueContract contract to simulate token gain. SimpleRevenueContract10. Mint 10_000_000_000_000 revenue tokens to the second SimpleRevenueContract contract to simulate token gain. SimpleRevenueContract11. As the borrower, borrow() all deposits. borrow()12. As borrower, claim revenue for the first SimpleRevenueContract contract in SpigotController. SimpleRevenueContract13. As borrower, claim revenue for the second SimpleRevenueContract contract in SpigotController. SimpleRevenueContract14. As the borrower, pay off the debt.
20. As a borrower, release the spigot in the SpigotedLoan controller. SpigotedLoan16. As borrower, call removeSpigot() function with the first SimpleRevenueContract contract as an input. removeSpigot()``SimpleRevenueContract17. As borrower, attempt to call removeSpigot() function with the second SimpleRevenueContract contract as an input. Observe that the transaction reverts with the ERC20: transfer amount exceeds balance error. Note that the contract's ownership still belongs to the SpigotController.removeSpigot()``SimpleRevenueContract``ERC20: transfer amount exceeds balance

\

SOLVED: The Debt DAO team solved this issue in commit 1908ded5d604984fd1a6c52af360c9135582f175: the spigot now does not transfer escrowed tokens in the removeSpigot() function. Escrowed tokens can be claimed with the claimEscrow() function.SOLVED:Debt DAO team1908ded5d604984fd1a6c52af360c9135582f175removeSpigot()``claimEscrow()

\

In the SpigotedLoan the solution assumes that the borrower adds a spigot with the revenue contract and transfers the ownership of the revenue contract to the SpigotController. SpigotedLoan``SpigotController

In the SpigotedLoan contract, the arbiter can call the updateWhitelist function to allow or disallow execution of the particular function for the operator (and the operator is the borrower by default) in the context of the revenue contract. The assessment revealed that a malicious arbiter could whitelist the ownership transfer function for the operator. Thus, the operator can transfer the ownership from SpigotController back to the borrower using the _operate() function. This function disallows to execute claimFunction(); however, it does not disallow to execute transferOwnerFunction.SpigotedLoan``updateWhitelist``SpigotController``_operate()``claimFunction()``transferOwnerFunction

Spigot.sol

    /**
     * @notice - Checks that operation is whitelisted by Spigot Owner and calls revenue contract with supplied data
     * @param revenueContract - smart contracts to call
     * @param data - tx data, including function signature, to call contracts with
     */
    function _operate(address revenueContract, bytes calldata data) internal nonReentrant returns (bool) {
        // extract function signature from tx data and check whitelist
        require(whitelistedFunctions[bytes4(data)], "Spigot: Unauthorized action");
        // cant claim revenue via operate() because that fucks up accounting logic. Owner shouldn't whitelist it anyway but just in case
        require(settings[revenueContract].claimFunction != bytes4(data), "Spigot: Unauthorized action");


        (bool success, bytes memory opData) = revenueContract.call(data);
        require(success, "Spigot: Operation failed");

        return true;
    }

1. All necessary contracts are deployed and initialized: CreditToken, RevenueToken, SimpleOracle, LoanLib, SpigotedLoan, SimpleRevenueContract.
2. As borrower and arbiter, add a new spigot with the addSpigot() function and RevenueContract as input.
3. As the borrower, transfer the ownership of the RevenueContract contract to the SpigotController.
4. As arbiter, whitelist transferOwnership() function from RevenueContract contract using updateWhitelist() function and the transferOwnership() selector as an input.
5. As the borrower, transfer the ownership again using the operate() function. Note that the encoded transferOwnership() function selector and borrower's address must be provided as input.1. All necessary contracts are deployed and initialized: CreditToken, RevenueToken, SimpleOracle, LoanLib, SpigotedLoan, SimpleRevenueContract.
6. As borrower and arbiter, add a new spigot with the addSpigot() function and RevenueContract as input. addSpigot()3. As the borrower, transfer the ownership of the RevenueContract contract to the SpigotController.
7. As arbiter, whitelist transferOwnership() function from RevenueContract contract using updateWhitelist() function and the transferOwnership() selector as an input. transferOwnership()``updateWhitelist()``transferOwnership()4. As the borrower, transfer the ownership again using the operate() function. Note that the encoded transferOwnership() function selector and borrower's address must be provided as input.operate()``transferOwnership()

\

RISK ACCEPTED: The Debt DAO team accepted the risk of this finding.RISK ACCEPTEDDebt DAO team

\

In the SpigotController contract, the contract's owner can call the _updateWhitelist function to allow or disallow execution of the particular function for the operator in the context of the revenue contract. Upon function execution, the UpdateWhitelistFunction event is emitted with true value, despite the actual value present in the allowed parameter. As a result, the contract may emit false and misleading information when some function is disallowed.SpigotController``_updateWhitelist``UpdateWhitelistFunction``true``allowed

Spigot.sol

    /**

     * @notice - Allows Owner to whitelist function methods across all revenue contracts for Operator to call.
     * @param func - smart contract function signature to whitelist
     * @param allowed - true/false whether to allow this function to be called by Operator
     */
    function _updateWhitelist(bytes4 func, bool allowed) internal returns (bool) {
        whitelistedFunctions[func] = allowed;
        emit UpdateWhitelistFunction(func, true);
        return true;
    }

\

SOLVED: The Debt DAO team solved this issue in commit f61ecfc20f8b2550fa9b98f3821571cbaa154295: the UpdateWhitelistFunction event is now emitted with a variable, not a fixed one.SOLVED:Debt DAO teamf61ecfc20f8b2550fa9b98f3821571cbaa154295UpdateWhitelistFunction

\

The borrower can postpone borrowing the deposit (borrow() function), which would start drawing interest accruing as far as the credit would be required. When the lender attempt to withdraw() the deposit, the borrower could front-run it with the borrow() function, and immediately call the repay() function to pay off the debt, so no drawn interest would be applied, and all deposit still would be available. The borrower could repeat that until the credit deadline has passed, or the deposit would be required. During this time, the facility interest would still be accrued.borrow()``withdraw()``borrow()``repay()

\

RISK ACCEPTED: The Debt DAO team accepted the risk of this finding. RISK ACCEPTED:Debt DAO team

\

In the Spigot contract, the removeSpigot() allows the contract's owner to transfer tokens held by the spigot, and transfer the ownership of the revenueContract to the operator (the borrower) and remove the revenueContract's data from the spigot's settings collection.Spigot``removeSpigot()``revenueContract

The amount of tokens to transfer is based on the escrowed collection. However, the contract's balance and the escrowed collection may contain different values. The contract's balance may be affected by push payments or by the MAX_REVENUE check in the _claimRevenue() function. The contract's balance may have a higher value than recorded in the escrowed collection. Thus, spigot removal may end up with an amount of tokens locked in the contract. Adding a new spigot may be required to unlock the tokens, which is cumbersome. escrowed``escrowed``MAX_REVENUE``_claimRevenue()``escrowed

Spigot.sol

    function removeSpigot(address revenueContract) external returns (bool) {
        require(msg.sender == owner);

        address token = settings[revenueContract].token;
        uint256 claimable = escrowed[token];
        if(claimable > 0) {
            require(_sendOutTokenOrETH(token, owner, claimable));
            emit ClaimEscrow(token, claimable, owner);
        }

        (bool success, bytes memory callData) = revenueContract.call(
            abi.encodeWithSelector(
                settings[revenueContract].transferOwnerFunction,
                operator    // assume function only takes one param that is new owner address
            )
        );
        require(success);

        delete settings[revenueContract];
        emit RemoveSpigot(revenueContract, token);

        return true;
    }

\

SOLVED: The Debt DAO team solved this issue in commit f61ecfc20f8b2550fa9b98f3821571cbaa154295: the removeSpigot() function is now protected with whileNoUnclaimedRevenue modifier.SOLVED:Debt DAO teamf61ecfc20f8b2550fa9b98f3821571cbaa154295removeSpigot()``whileNoUnclaimedRevenue

\

In the LineOfCredit contract, the increaseCredit() allows borrowers and lenders to increase their credit. This function also allows transferring specified principal to the borrower immediately. However, this function does not call the _sortIntoQ() function, which updates the credit position in the repaid queue. As a result, the credit position with increased credit and the transferred principal may not be updated in the queue and left behind other credit positions with the repaid principal. In some rare scenarios, to update the queue, the borrower would be forced to close other credit positions or to borrow from such a position (if the deposit is available).LineOfCredit``increaseCredit()``principal``_sortIntoQ()

LineOfCredit.sol

function increaseCredit(
        bytes32 id,
        address lender,
        uint256 amount,
        uint256 principal
    )
      external
      override
      whileActive
      mutualConsent(lender, borrower)
      returns (bool)
    {
        _accrueInterest(id);
        require(principal <= amount, 'LoC: amount must be over princpal');
        Credit memory credit = credits[id];
        require(lender == credit.lender, 'LoC: only lender can increase');

        require(IERC20(credit.token).transferFrom(
          credit.lender,
          address(this),
          amount
        ), "Loan: no tokens to lend");

        credit.deposit += amount;

        int256 price = oracle.getLatestAnswer(credit.token);

        emit IncreaseCredit(
          id,
          amount,
          LoanLib.calculateValue( price, amount, credit.decimals)
        );

        if(principal > 0) {  
            require(
              IERC20(credit.token).transfer(borrower, principal),
              "Loan: no liquidity"
            );

            uint256 value = LoanLib.calculateValue(price, principal, credit.decimals);
            credit.principal += principal;
            principalUsd += value;
            emit Borrow(id, principal, value);
        }

        credits[id] = credit;

        return true;
    }

\

RISK ACCEPTED: The Debt DAO team accepted the risk of this finding. RISK ACCEPTED:Debt DAO team

\

In all the loops, the counter variable is incremented using i++. It is known that, in loops, using ++i costs less gas per iteration than i++.i++``++i``i++

Code Location

LineOfCredit.sol LineOfCredit.sol- Line 94: for (uint256 i = 0; i < len; i++) - Line 131: for (uint256 i = 0; i < len; i++) - Line 159: for (uint256 i = 0; i < len; i++) - Line 686: for (uint256 i = 0; i < len; i++)- Line 94: for (uint256 i = 0; i < len; i++) for (uint256 i = 0; i < len; i++)- Line 131: for (uint256 i = 0; i < len; i++) for (uint256 i = 0; i < len; i++)- Line 159: for (uint256 i = 0; i < len; i++) for (uint256 i = 0; i < len; i++)- Line 686: for (uint256 i = 0; i < len; i++)``for (uint256 i = 0; i < len; i++)

Escrow.sol Escrow.sol- Line 88: for (uint256 i = 0; i < length; i++)- Line 88: for (uint256 i = 0; i < length; i++)``for (uint256 i = 0; i < length; i++)

Spigot.sol Spigot.sol- Line 206: for(uint256 i = 0; i < data.length; i++)- Line 206: for(uint256 i = 0; i < data.length; i++)``for(uint256 i = 0; i < data.length; i++)

LoanLib.sol LoanLib.sol- Line 122: for(uint i = 0; i < positions.length; i++) - Line 151: for(uint i = 1; i < len; i++)- Line 122: for(uint i = 0; i < positions.length; i++) for(uint i = 0; i < positions.length; i++)- Line 151: for(uint i = 1; i < len; i++)``for(uint i = 1; i < len; i++)

For example, based in the following test contract:

Test.sol

//SPDX-License-Identifier: MIT
pragma solidity 0.8.9;

contract test {
    function postiincrement(uint256 iterations) public {
        for (uint256 i = 0; i < iterations; i++) {
        }
    }
    function preiincrement(uint256 iterations) public {
        for (uint256 i = 0; i < iterations; ++i) {
        }
    }
}

We can see the difference in the gas costs:

\

SOLVED: The Debt DAO team solved this issue in commit 227d484486cb71c638d9fbe3ee4fbbe8f935c7cf: all instances were fixed.SOLVED:Debt DAO team227d484486cb71c638d9fbe3ee4fbbe8f935c7cf

\

As i is an uint256, it is already initialized to 0. uint256 i = 0 reassigns the 0 to i which wastes gas.i``uint256``uint256 i = 0``i

Code Location

LineOfCredit.sol LineOfCredit.sol- Line 94: for (uint256 i = 0; i < len; i++) - Line 131: for (uint256 i = 0; i < len; i++) - Line 159: for (uint256 i = 0; i < len; i++) - Line 684: uint256 _i = 0; // index that p should be moved to - Line 686: for (uint256 i = 0; i < len; i++)- Line 94: for (uint256 i = 0; i < len; i++) for (uint256 i = 0; i < len; i++)- Line 131: for (uint256 i = 0; i < len; i++) for (uint256 i = 0; i < len; i++)- Line 159: for (uint256 i = 0; i < len; i++) for (uint256 i = 0; i < len; i++)- Line 684: uint256 _i = 0; // index that p should be moved to uint256 _i = 0; // index that p should be moved to- Line 686: for (uint256 i = 0; i < len; i++)``for (uint256 i = 0; i < len; i++)

Escrow.sol Escrow.sol- Line 83: uint256 collateralValue = 0; - Line 88: for (uint256 i = 0; i < length; i++)- Line 83: uint256 collateralValue = 0; uint256 collateralValue = 0;- Line 88: for (uint256 i = 0; i < length; i++)``for (uint256 i = 0; i < length; i++)

Spigot.sol Spigot.sol- Line 206: for(uint256 i = 0; i < data.length; i++)- Line 206: for(uint256 i = 0; i < data.length; i++)``for(uint256 i = 0; i < data.length; i++)

LoanLib.sol LoanLib.sol- Line 119: uint256 count = 0; - Line 122: for(uint i = 0; i < positions.length; i++) - Line 151: for(uint i = 1; i < len; i++)- Line 119: uint256 count = 0; uint256 count = 0;- Line 122: for(uint i = 0; i < positions.length; i++) for(uint i = 0; i < positions.length; i++)- Line 151: for(uint i = 1; i < len; i++)``for(uint i = 1; i < len; i++)

\

SOLVED: The Debt DAO team solved this issue in commit 227d484486cb71c638d9fbe3ee4fbbe8f935c7cf: all instances were fixed.SOLVED:Debt DAO team227d484486cb71c638d9fbe3ee4fbbe8f935c7cf

\

Several instances of assertions without messages were identified. The lack of message in require assertion might be unfavorable for end users.require

Code Location

LineOfCredit.sol LineOfCredit.sol- Line 69: require(ids.length > 0 && credits[ids[0]].principal > 0); - Line 199: require(interestRate.setRate(id, drate, frate)); - Line 228: require(interestRate.setRate(id, drate, frate)); - Line 344: require(amount <= credits[id].principal + credits[id].interestAccrued) - Line 401: require(_sortIntoQ(id)); - Line 491: require(_close(id));- Line 69: require(ids.length > 0 && credits[ids[0]].principal > 0); require(ids.length > 0 && credits[ids[0]].principal > 0);- Line 199: require(interestRate.setRate(id, drate, frate)); require(interestRate.setRate(id, drate, frate));- Line 228: require(interestRate.setRate(id, drate, frate)); require(interestRate.setRate(id, drate, frate));- Line 344: require(amount <= credits[id].principal + credits[id].interestAccrued) require(amount <= credits[id].principal + credits[id].interestAccrued)- Line 401: require(_sortIntoQ(id)); require(_sortIntoQ(id));- Line 491: require(_close(id));``require(_close(id));

SecuredLoan.sol SecuredLoan.sol- Line 53: require(msg.sender == arbiter);- Line 53: require(msg.sender == arbiter);``require(msg.sender == arbiter);

SpigotedLoan.sol SpigotedLoan.sol- Line 97: require(msg.sender == borrower || msg.sender == arbiter); - Line 142: require(msg.sender == borrower || msg.sender == arbiter); - Line 229: require(msg.sender == arbiter);- Line 97: require(msg.sender == borrower || msg.sender == arbiter); require(msg.sender == borrower || msg.sender == arbiter);- Line 142: require(msg.sender == borrower || msg.sender == arbiter); require(msg.sender == borrower || msg.sender == arbiter);- Line 229: require(msg.sender == arbiter);``require(msg.sender == arbiter);

Escrow.sol Escrow.sol- Line 144: require(msg.sender == ILoan(loan).arbiter());- Line 144: require(msg.sender == ILoan(loan).arbiter());``require(msg.sender == ILoan(loan).arbiter());

Spigot.sol Spigot.sol- Line 154: require(msg.sender == owner); - Line 193: require(msg.sender == operator); - Line 205: require(msg.sender == operator); - Line 243: require(msg.sender == operator); - Line 256: require(revenueContract != address(this)); - Line 277: require(msg.sender == owner); - Line 292: require(success); - Line 277: require(msg.sender == owner); - Line 315: require(msg.sender == owner); - Line 330: require(msg.sender == operator); - Line 345: require(msg.sender == treasury || msg.sender == operator); - Line 330: require(msg.sender == operator); - Line 362: require(msg.sender == owner);- Line 154: require(msg.sender == owner); require(msg.sender == owner);- Line 193: require(msg.sender == operator); require(msg.sender == operator);- Line 205: require(msg.sender == operator); require(msg.sender == operator);- Line 243: require(msg.sender == operator); require(msg.sender == operator);- Line 256: require(revenueContract != address(this)); require(revenueContract != address(this));- Line 277: require(msg.sender == owner); require(msg.sender == owner);- Line 292: require(success); require(success);- Line 277: require(msg.sender == owner); require(msg.sender == owner);- Line 315: require(msg.sender == owner); require(msg.sender == owner);- Line 330: require(msg.sender == operator); require(msg.sender == operator);- Line 345: require(msg.sender == treasury || msg.sender == operator); require(msg.sender == treasury || msg.sender == operator);- Line 330: require(msg.sender == operator); require(msg.sender == operator);- Line 362: require(msg.sender == owner);``require(msg.sender == owner);

EscrowedLoan.sol EscrowedLoan.sol- Line 55: require(escrow.liquidate(amount, targetToken, to));- Line 55: require(escrow.liquidate(amount, targetToken, to));``require(escrow.liquidate(amount, targetToken, to));

\

PARTIALLY SOLVED: The Debt DAO team partially solved this informational finding.PARTIALLY SOLVED:Debt DAO team

\

The defaultRevenueSplit parameter lacks input validation in the SpigotedLoan contract.defaultRevenueSplit``SpigotedLoan

Code Location

SpigotedLoan.sol

constructor(
        address oracle_,
        address arbiter_,
        address borrower_,
        address swapTarget_,
        uint256 ttl_,
        uint8 defaultRevenueSplit_
    ) LineOfCredit(oracle_, arbiter_, borrower_, ttl_) {
        spigot = new SpigotController(address(this), borrower, borrower);

        defaultRevenueSplit = defaultRevenueSplit_;

        swapTarget = swapTarget_;
    }

\

SOLVED: The Debt DAO team solved this issue in commit 760c5bfb9c352fb681f8351253ff9776b176e357: the defaultRevenueSplit parameter now has input validation.SOLVED:Debt DAO team760c5bfb9c352fb681f8351253ff9776b176e357defaultRevenueSplit

\

Within the LoanLib contract, part of the code seems unused and redundant: calculateValue() function, DEBT_TOKEN constant, and some values from STATUS enum: UNINITIALIZED, INITIALIZED, UNDERCOLLATERALIZED, DELINQUENT, LIQUIDATING, OVERDRAWN, DEFAULT, ARBITRATION, INSOLVENT. Note that INSOLVENT is being used, but it is never set.LoanLib``calculateValue()``DEBT_TOKEN``STATUS``INSOLVENT

Within the EscrowedLoan contract, the _liquidate() function has positionId input parameter. This variable is used only to emit the Liquidate event from the IEscrowedLoan interface. Apart from that, no processing related to the credit with positionId is being done. Also, the liquidate() function from Escrow contract emits another Liquidate event from IEscrow interface. The positionId parameter and emission of the Liquidate event from the IEscrowedLoan interface seem to be redundant.EscrowedLoan``_liquidate()``positionId``Liquidate``IEscrowedLoan``positionId``liquidate()``Escrow``Liquidate``IEscrow``positionId``Liquidate``IEscrowedLoan

Code Location

LoanLib.sol

address constant DEBT_TOKEN = address(0xdebf);

enum STATUS {
        // ¿hoo dis
        // Loan has been deployed but terms and conditions are still being signed off by parties
        UNINITIALIZED,
        INITIALIZED,

        // ITS ALLLIIIIVVEEE
        // Loan is operational and actively monitoring status
        ACTIVE,
        UNDERCOLLATERALIZED,
        LIQUIDATABLE, // [#X
        DELINQUENT,

        // Loan is in distress and paused
        LIQUIDATING,
        OVERDRAWN,
        DEFAULT,
        ARBITRATION,

        // Lön izz ded
        // Loan is no longer active, successfully repaid or insolvent
        REPAID,
        INSOLVENT
    }

LoanLib.sol

    function calculateValue(
      int price,
      uint256 amount,
      uint8 decimals
    )
      internal
      returns(uint256)
    {
      return _calculateValue(price, amount, decimals);
    }

IEscrowedLoan.sol

event Liquidate(bytes32 indexed positionId, uint256 indexed amount, address indexed token);

EscrowedLoan.sol

  function _liquidate(
    bytes32 positionId,
    uint256 amount,
    address targetToken,
    address to
  )
    virtual internal
    returns(uint256)
  { 
    require(escrow.liquidate(amount, targetToken, to));

    emit Liquidate(positionId, amount, targetToken);

    return amount;
  }

\

SOLVED: The Debt DAO team solved this issue in commit 227d484486cb71c638d9fbe3ee4fbbe8f935c7cf: all instances were fixed. In addition, the _liquidate() function will use the positionId input parameter in a future release.SOLVED:Debt DAO team227d484486cb71c638d9fbe3ee4fbbe8f935c7cf_liquidate()``positionId

\

The SpigotController contract inherits ReentrancyGuard. The functions _operate(), claimRevenue(), and claimEscrow() are protected by the nonReentrant modifier. The functions claimRevenue() and claimEscrow() uses _sendOutTokenOrETH() function to transfer the ether. However, the removeSpigot() also uses _sendOutTokenOrETH() function, but it is not protected by the nonReentrant modifier, also it modifies the contract's state after transferring the ether.SpigotController``ReentrancyGuard``_operate()``claimRevenue()``claimEscrow()``nonReentrant``claimRevenue()``claimEscrow()``_sendOutTokenOrETH()``removeSpigot()``_sendOutTokenOrETH()``nonReentrant

Spigot.sol

function removeSpigot(address revenueContract) external returns (bool) {
        require(msg.sender == owner);

        address token = settings[revenueContract].token;
        uint256 claimable = escrowed[token];
        if(claimable > 0) {
            require(_sendOutTokenOrETH(token, owner, claimable));
            emit ClaimEscrow(token, claimable, owner);
        }

        (bool success, bytes memory callData) = revenueContract.call(
            abi.encodeWithSelector(
                settings[revenueContract].transferOwnerFunction,
                operator    // assume function only takes one param that is new owner address
            )
        );
        require(success);

        delete settings[revenueContract];
        emit RemoveSpigot(revenueContract, token);

        return true;
    }

\color{black} \color{white} In the SpigotedLoan the sweep() function transfers ether, however, it is not protected by the nonReentrant modifier, also it modifies the contract's state after transferring the ether.SpigotedLoan``sweep()``nonReentrant

SpigotedLoan.sol

   * @notice - sends unused tokens to borrower if repaid or arbiter if liquidatable
             -  doesnt send tokens out if loan is unpaid but healthy
   * @dev    - callable by anyone 
   * @param token - token to take out
  */
    function sweep(address token) external returns (uint256) {
        if (loanStatus == LoanLib.STATUS.REPAID) {
            return _sweep(borrower, token);
        }
        if (loanStatus == LoanLib.STATUS.INSOLVENT) {
            return _sweep(arbiter, token);
        }

        return 0;
    }

    function _sweep(address to, address token) internal returns (uint256 x) {
        x = unusedTokens[token];
        if (token == address(0)) {
            payable(to).transfer(x);
        } else {
            require(IERC20(token).transfer(to, x));
        }
        delete unusedTokens[token];
    }

\

SOLVED: The Debt DAO team solved this issue in commit 4c0fd8888c6d1daf64cbdf7db018119fc9b18730: the claimAndRepay(), the claimAndTrade() and the sweep() functions now have the nonReentrant modifier. The removeSpigot() functionality no longer transfers tokens. SOLVED:Debt DAO team4c0fd8888c6d1daf64cbdf7db018119fc9b18730claimAndRepay()``claimAndTrade()``sweep()``nonReentrant``removeSpigot()

\