sherlock-audit / 2024-09-predict-fun-judging

5 stars 4 forks source link

phoenixv110 - Any number of 0 amount loans can be created on the proposal that is fully filled and not expired #134

Open sherlock-admin3 opened 1 month ago

sherlock-admin3 commented 1 month ago

phoenixv110

Medium

Any number of 0 amount loans can be created on the proposal that is fully filled and not expired

Summary

The missing check in loan creation flow enables user to create 0 amount loans on any proposal that is fulfilled 100% but is still valid. These 0 amount loans can be repay(), call(),auction() & seize() without any cost. As the interested acquired on them is also 0.

Root Cause

In method _assertFulfillAmountNotTooLow() if the proposal is 100% filled the loanAmount = fulfilledAmount. So the checkfulfillAmount != loanAmount - fulfilledAmount will be false for fulfillAmount = 0.

function _assertFulfillAmountNotTooLow(
    uint256 fulfillAmount,
    uint256 fulfilledAmount,
    uint256 loanAmount
) private pure {
    if (fulfillAmount != loanAmount - fulfilledAmount) {
        if (fulfillAmount < loanAmount / 10) {
            revert FulfillAmountTooLow();
        }
    }
}

https://github.com/sherlock-audit/2024-09-predict-fun/blob/main/predict-dot-loan/contracts/PredictDotLoan.sol#L1269C1-L1279C6

Internal pre-conditions

  1. The proposal should be 100% fulfilled.
  2. The proposal should be an active proposal.

External pre-conditions

No response

Attack Path

No response

Impact

On Blast the gas fee is very low. Which can lead to user creating any number of such 0 amount loans with minimal cost.

PoC

Add the following test in PredictDotLoan.acceptBorrowRequest.t.sol

function test_acceptBorrowRequest_With_0_Amount() public {
    testFuzz_acceptBorrowRequest(0);

    IPredictDotLoan.Proposal memory proposal = _generateBorrowRequest(IPredictDotLoan.QuestionType.Binary);
    proposal.salt = 1;
    proposal.from = borrower2;
    proposal.signature = _signProposal(proposal, borrower2PrivateKey);

    mockERC20.mint(lender, proposal.loanAmount);
    _mintCTF(borrower2);

    vm.prank(lender);
    mockERC20.approve(address(predictDotLoan), proposal.loanAmount);

    vm.prank(borrower2);
    mockCTF.setApprovalForAll(address(predictDotLoan), true);

    expectEmitCheckAll();
    emit ProposalAccepted(
        2,
        predictDotLoan.hashProposal(proposal),
        proposal.from,
        lender,
        _getPositionId(true),
        COLLATERAL_AMOUNT,
        LOAN_AMOUNT,
        0
    );

    vm.prank(lender);
    predictDotLoan.acceptBorrowRequest(proposal, proposal.loanAmount);

    predictDotLoan.acceptBorrowRequest(proposal, 0);
}

Mitigation

Add a fulfillAmount = 0 amount check in _assertFulfillAmountNotTooLow() check will fix the issue.

function _assertFulfillAmountNotTooLow(
    uint256 fulfillAmount,
    uint256 fulfilledAmount,
    uint256 loanAmount
) private pure {
@> if (fulfillAmount == 0 || (fulfillAmount != loanAmount - fulfilledAmount)) {
        if (fulfillAmount < loanAmount / 10) {
            revert FulfillAmountTooLow();
        }
    }
}
sherlock-admin2 commented 1 month ago

The protocol team fixed this issue in the following PRs/commits: https://github.com/PredictDotFun/predict-dot-loan/pull/35