Debt DAO contest - d3e4's results

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

General Information

Platform: Code4rena

Start Date: 03/11/2022

Pot Size: $115,500 USDC

Total HM: 17

Participants: 120

Period: 7 days

Judge: LSDan

Total Solo HM: 1

Id: 174

League: ETH

Debt DAO

Findings Distribution

Researcher Performance

Rank: 111/120

Findings: 2

Award: $13.42

🌟 Selected for report: 0

🚀 Solo Findings: 0

Awards

8.0811 USDC - $8.08

Labels

bug
2 (Med Risk)
satisfactory
duplicate-39

External Links

Lines of code

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/LineLib.sol#L71

Vulnerability details

Impact

LineLib.receiveTokenOrETH() can receive ETH and accounts for an amount to receive but it is possible to send more ETH than amount, meaning that any extra ETH sent will be lost to the contract.

Proof of Concept

LineLib.receiveTokenOrETH() is as follows.

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
            if(msg.value < amount) { revert TransferFailed(); }
        }
        return true;
    }

In the case ETH is to be received it checks if(msg.value < amount), in which case it reverts. We want msg.value == amount, but if msg.value > amount, then amount will be accounted for and msg.value - amount will be lost.

Tools Used

Code inspection

Change if(msg.value < amount) { revert TransferFailed(); } into if(msg.value != amount) { revert TransferFailed(); }.

#0 - c4-judge

2022-11-17T16:43:46Z

dmvt marked the issue as duplicate of #25

#1 - c4-judge

2022-12-06T15:05:24Z

dmvt marked the issue as satisfactory

#2 - C4-Staff

2022-12-20T06:44:54Z

liveactionllama marked the issue as duplicate of #39

Awards

5.3388 USDC - $5.34

Labels

bug
2 (Med Risk)
satisfactory
duplicate-369

External Links

Lines of code

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/LineLib.sol#L48

Vulnerability details

Impact

LineLib.sendOutTokenOrETH() may revert when its receiver is a contract, preventing it from receiving ETH, and thus from much of the functionality of the protocol.

Proof of Concept

LineLib.sendOutTokenOrETH() is as follows.

/**
* @notice - Send ETH or ERC20 token from this contract to an external contract
* @param token - address of token to send out. Denominations.ETH for raw ETH
* @param receiver - address to send tokens to
* @param amount - amount of tokens to send
*/
function sendOutTokenOrETH(
    address token,
    address receiver,
    uint256 amount
)
    external
    returns (bool)
{
    if(token == address(0)) { revert TransferFailed(); }
    
    // both branches revert if call failed
    if(token!= Denominations.ETH) { // ERC20
        IERC20(token).safeTransfer(receiver, amount);
    } else { // ETH
        payable(receiver).transfer(amount);
    }
    return true;
}

In the case ETH is to be sent it calls payable(receiver).transfer(amount). transfer() forwards only 2300 gas which may cause a receiver contract to revert if it consumes more than this upon reception, or if gas costs change in the future. See https://consensys.net/diligence/blog/2019/09/stop-using-soliditys-transfer-now/. Note the comment Send ETH or ERC20 token from this contract to an external contract, so it is indeed the intention to send ETH to a contract.

Tools Used

Code inspection

Use call() instead, checking the return value and protecting against reentrancy.

#0 - c4-judge

2022-11-17T16:44:21Z

dmvt marked the issue as duplicate of #14

#1 - c4-judge

2022-12-06T14:39:13Z

dmvt marked the issue as satisfactory

#2 - C4-Staff

2022-12-20T05:56:43Z

liveactionllama marked the issue as duplicate of #369

AuditHub

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

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter