Debt DAO contest - Nyx's results

Lines of code

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

Vulnerability details

Impact

When a user wants to use addCredit() function with ETH, user can mistakenly send more ETH than amount. If that happens, User ETH can be frozen inside the contract.

Proof of Concept

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

Test :

function test_canSendMoreETHThanAmount() public {
        assertEq(address(line).balance, 0, "Line balance should be 0");
        assertEq(
            lender.balance,
            mintAmount,
            "lender should have initial mint balance"
        );
        console.log(lender.balance / 1e18);
        hoax(borrower);
        line.addCredit(dRate, fRate, 1 ether, Denominations.ETH, lender);

        vm.startPrank(lender);
        line.addCredit{value: 2 ether}(
            dRate,
            fRate,
            1 ether,
            Denominations.ETH,
            lender
        );

        vm.stopPrank();
        console.log("Lender balance after addCredit", lender.balance / 1e18);
        console.log(
            "line balance after addCredit",
            address(line).balance / 1e18
        );
    }

Tools Used

Manuel review

Recommended Mitigation Steps

msg.value needs to be equal to amount.

Lines of code

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

Vulnerability details

Impact

transfer() uses a fixed amount of gas, which can result in revert.

transfer() uses a fixed amount of gas, which was used to prevent reentrancy. However this limit your protocol to interact with others contracts that need more than that to process the transaction.

Proof of Concept

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

Tools Used

Manuel review

Recommended Mitigation Steps

Consider using .call() instead with the checks-effects-interactions pattern implemented correctly. Careful consideration needs to be made to prevent reentrancy.

1 - UNUSED/EMPTY RECEIVE()/FALLBACK() FUNCTION

If the intention is for the Ether to be used, the function should call another function, otherwise it should revert.

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/SpigotedLine.sol#L272

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/spigot/Spigot.sol#L227