Debt DAO contest - codexploder's results

Lines of code

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

Vulnerability details

Impact

The receiveTokenOrETH function does not return excess ETH if passed accidentally by the user by calling the addCredit function at LineOfCredit.sol#L237. This can lead to loss for user

Proof of Concept

1. User calls receiveTokenOrETH function via addCredit function and passes msg.value>amount
2. The function keeps the excess ETH and only the amount passed in argument is added as Credit

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

Recommended Mitigation Steps

Return any excess ETH which is passed to receiveTokenOrETH function (in this case return msg.value-amount, if msg.value>amount)

Lines of code

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

Vulnerability details

Impact

In receiveTokenOrETH function, if token takes fee on transfer then amount received by contract will be lesser than what is being accounted in

Proof of Concept

1. Observe the receiveTokenOrETH function

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

2. In this case if token is a ERC20 token taking fees on transfer then contract will receive amount-fees

3. Now this function is utilized by addCredit function

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

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

        ...
    }

4. In case of fee on transfer token amount received is lesser but still contract will create credit for full amount

Recommended Mitigation Steps

Calculate the balance before and after transfer and then subtract them to get exact amount of token transferred to contract

Lines of code

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

Vulnerability details

Impact

The transfer function is bounded by 2300 gas units so it might fail if recipient requires more gas unit. A better approach is to make use of call function instead

Proof of Concept

1. Observe the sendOutTokenOrETH function

function sendOutTokenOrETH(
      address token,
      address receiver,
      uint256 amount
    )
      external
      returns (bool)
    {
        ...
        } else { // ETH
            payable(receiver).transfer(amount);
        }
        return true;
    }

2. Observe that ETH is paid to receiver using transfer instead of using call

Recommended Mitigation Steps

Use call instead of transfer function