Lucene search

K
code423n4Code4renaCODE423N4:2022-11-DEBTDAO-FINDINGS-ISSUES-471
HistoryNov 10, 2022 - 12:00 a.m.

Arbitrary call order to handle mutual consent can lead to unrecoverable native ETH

2022-11-1000:00:00
Code4rena
github.com
5
lineofcredit
mutualconsent
vulnerability
eth
lender
borrower
contract
github
code
security
credit
deposit
function
execution
issue
proof of concept
modifier
impact
line of credit
increase
decrease

Lines of code
<https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/LineOfCredit.sol#L234&gt;
<https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/LineOfCredit.sol#L270&gt;

Vulnerability details

Creating new credits and increasing the credit deposit requires both parties, the lender and the borrower, to agree. This is implemented by having both call the same function with the same call data.

However, as it’s possible to use native ETH as a credit token, this can lead to a situation where the lender is the first to call the function and sends native ETH with it. The function is not executed at this time, as the borrower has not yet called the function. The already transferred native ETH is now locked and unrecoverable.

Impact

If the lender is the first to call the LineOfCredit.addCredit or LineOfCredit.increaseCredit functions, native ETH transferred with the call will be locked as the function is only executed after the second and final signer has called the function.

Proof of Concept

utils/MutualConsent.sol#L31-L36

The MutualConsent.mutualConsent modifier only allows the function to be called in case both signers have called the function with the same call data.

modifier mutualConsent(address _signerOne, address _signerTwo) {
  if(_mutualConsent(_signerOne, _signerTwo))  {
    // Run whatever code needed 2/2 consent
    _;
  }
}

modules/credit/LineOfCredit.sol#L234

The LineOfCredit.addCredit function uses the mutualConsent modifier to add a new credit. Native ETH can be sent to the function as the deposit. However, if the lender is the first caller of the function and sends native ETH, the function will not be executed due to the missing second call of the borrower. ETH funds will be unrecoverable.

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

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

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

    return id;
}

modules/credit/LineOfCredit.sol#L270

Similarly, the LineOfCredit.increaseCredit function uses the .LineOfCreditmutualConsentById modifier (which internally uses the MutualConsent.mutualConsent modifier). The same issue with native-sent ETH applies. If the lender is the first caller of the function and sends native ETH, the function will not be executed due to the missing second call of the borrower. ETH funds will be unrecoverable.

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

    credit.deposit += amount;

    credits[id] = credit;

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

    emit IncreaseCredit(id, amount);

    return true;
}

Tools Used

Manual review

Recommended mitigation steps

Consider enforcing the function call order to ensure that the party providing the native ETH funds is the second and last caller of the function.


The text was updated successfully, but these errors were encountered:

All reactions