code-423n4 / 2024-06-size-findings

0 stars 0 forks source link

Malicious offers can significantly impact the usability of the protocol #156

Closed c4-bot-7 closed 2 months ago

c4-bot-7 commented 2 months ago

Lines of code

https://github.com/code-423n4/2024-06-size/blob/8850e25fb088898e9cf86f9be1c401ad155bea86/src/libraries/actions/BuyCreditLimit.sol#L29-L66 https://github.com/code-423n4/2024-06-size/blob/8850e25fb088898e9cf86f9be1c401ad155bea86/src/libraries/actions/SellCreditLimit.sol#L25-L51 https://github.com/code-423n4/2024-06-size/blob/8850e25fb088898e9cf86f9be1c401ad155bea86/src/libraries/actions/SellCreditMarket.sol#L201-L202 https://github.com/code-423n4/2024-06-size/blob/8850e25fb088898e9cf86f9be1c401ad155bea86/src/libraries/actions/SellCreditLimit.sol#L19-L52

Vulnerability details

Impact

Malicious borrow & loanOffers can be created to intentionally cause a DOS, griefing unsuspecting protocol users who attempt to match with these offers and end up paying transaction costs.

A coordinated attack, with hundreds of offers featuring very favourable conditions, could flood the Order Book could significantly impact the usability of the system as many legitimate users experience DOS.

Vulnerability Details

There are no restrictions, in BuyCreditLimit() or SellCreditLimit(), on which users can create loanOffers and borrowOffers.

This means that a malicious "lender" can create a loanOffer with extremely generous rates via BuyCreditLimit(), despite having no borrowATokens deposited in the protocol. Any user who tries to match with the offer via SellCreditMarket() will experience a DOS when the execution reverts in transferFrom() as the lender does not actually have any borrowATokens to transfer to the borrower.

Similarly, a malicious "borrower" can create a borrowOffer via SellCreditLimit() with no collateralTokens backing it. In this case, the revert will occur during the execution of SellCreditMarket(), will occur when validateMinimumCreditOpening() is executed, as the malicious user has no collateralTokens to open a credit position.

    function validateMinimumCreditOpening(State storage state, uint256 credit) public view {
        if (credit < state.riskConfig.minimumCreditBorrowAToken) {
            revert Errors.CREDIT_LOWER_THAN_MINIMUM_CREDIT_OPENING(credit, state.riskConfig.minimumCreditBorrowAToken);
        }
    }

POC

This test shows how a borrower can create a borrowerOffer with no collateral deposited in the protocol which causes a DOS when a lender tries to match with it. Add the test to BuyCreditMarket.t.sol:

    function test_borrowOffer_NoBacking() public {

        _deposit(bob, weth, 100e18);
        _deposit(bob, usdc, 100e6);

        uint256 tenor = 365 days;
        int256 apr = 0.05e18;

        _sellCreditLimit(alice, apr, tenor);

        uint256 amountLoanId1 = 10e6;

        vm.startPrank(bob);
        vm.expectRevert();
        size.buyCreditMarket(
            BuyCreditMarketParams({
                borrower: alice,
                creditPositionId: RESERVED_ID,
                tenor: 365 days,
                amount: amountLoanId1,
                deadline: block.timestamp,
                minAPR: 0,
                exactAmountIn: true
            })
        );     
    }

Tools Used

Manual Review Foundry Testing

Recommendations

Only allow lenders to create a loanOffer if they have a minimum amount of borrowATokens and prevent them from withdrawing that amount if they have an open loanOffer.

Similarly, only allow borrowers to create a borrowOffer if they have a minimum amount of collateralTokens and prevent them from withdrawing that amount if they have an open borrowOffer.

Assessed type

DoS

c4-judge commented 2 months ago

hansfriese marked the issue as unsatisfactory: Invalid