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

5 stars 4 forks source link

nikhilx0111 - a malicious user can grief the protocol #302

Open sherlock-admin4 opened 1 month ago

sherlock-admin4 commented 1 month ago

nikhilx0111

High

a malicious user can grief the protocol

Summary

when the loan amount is smaller than 50 the protocol wont be able to charge any fee due to precison loss

uint256 private constant MAXIMUM_PROTOCOL_FEE_BASIS_POINTS = 200;

     uint256 protocolFee = (exchangeOrder.takerAmount * protocolFeeBasisPoints) / 10_000;

the calculation will look something like this 49×200=9800 9800/10,000=0.98 this rounds down to 0 now a malicious user can create many loanoffers with different proposal ids with a amount smaller than 50 to bypass paying any fee to the protocol after creating multiple proposals the malicious borrower can match the proposal with his own borrowrequest (with a different address) now after accepting his own loan request the malicious user will repay the loan and transfer the collateral back to himself after repaying his loan the repay will set his loan to loan.status = LoanStatus.Repaid; it will upate the status to repaid but wont reset the active status (a loan can have multiple status) now after repaying all his loan and transferring back his collateral the malicious user will call the function call

function call(uint256 loanId) external nonReentrant {
    Loan storage loan = loans[loanId];

    _assertAuthorizedCaller(loan.lender);
    _assertLoanStatus(loan.status, LoanStatus.Active);

    if (loan.startTime + loan.minimumDuration > block.timestamp) {
        revert LoanNotMatured();
    }

    if (_isQuestionPriceAvailable(loan.questionType, positionQuestion[loan.positionId])) {
        _seize(loanId, loan);
    } else {
        loan.status = LoanStatus.Called;
        loan.callTime = block.timestamp;

        emit LoanCalled(loanId);

which checks if the loan being called has matured or active which will be true now after calling the auction the malicious user can sell all his empty loans to a innocent lender

function auction(uint256 loanId) external nonReentrant whenNotPaused {
    Loan storage loan = loans[loanId];

    _assertLoanStatus(loan.status, LoanStatus.Called);

    _assertLenderIsNotBorrower(msg.sender, loan.borrower);

    _assertNewLenderIsNotTheSameAsOldLender(msg.sender, loan.lender);

    uint256 callTime = loan.callTime;
    uint256 timeElapsed = block.timestamp - callTime;

    _assertAuctionIsActive(timeElapsed);

    // If the question is resolved in the middle of the auction, the lender can wait for the auction to be over
    // and seize the collateral
    _assertQuestionPriceUnavailable(loan.questionType, positionQuestion[loan.positionId]);

    uint256 interestRatePerSecond = _auctionCurrentInterestRatePerSecond(timeElapsed);

    loan.status = LoanStatus.Auctioned;

    uint256 _nextLoanId = nextLoanId;
    uint256 debt = _calculateDebt(loan.loanAmount, loan.interestRatePerSecond, callTime - loan.startTime);
    uint256 protocolFee = (debt * protocolFeeBasisPoints) / 10_000;

    Loan storage newLoan = loans[_nextLoanId];
    newLoan.borrower = loan.borrower;
    newLoan.lender = msg.sender;
    newLoan.positionId = loan.positionId;
    newLoan.collateralAmount = loan.collateralAmount;
    newLoan.loanAmount = debt + protocolFee;
    newLoan.interestRatePerSecond = interestRatePerSecond;
    newLoan.startTime = block.timestamp;
    newLoan.status = LoanStatus.Active;
    newLoan.questionType = loan.questionType;

    _transferLoanAmountAndProtocolFeeWithoutDeductingFromLoanAmount(msg.sender, loan.lender, debt, protocolFee);

    unchecked {
        ++nextLoanId;
    }

    emit LoanTransferred(loanId, debt, protocolFee, _nextLoanId, msg.sender, interestRatePerSecond);

as you can see in the auction there is no check if the loan for which the auction is called is repaid or not the malicious user will succesfully execute the attack by avoiding paying any fee and selling his empty loans to a lender

Root Cause

https://github.com/sherlock-audit/2024-09-predict-fun/blob/41e70f9eed3f00dd29aba4038544150f5b35dccb/predict-dot-loan/contracts/PredictDotLoan.sol#L52

https://github.com/sherlock-audit/2024-09-predict-fun/blob/41e70f9eed3f00dd29aba4038544150f5b35dccb/predict-dot-loan/contracts/PredictDotLoan.sol#L234

https://github.com/sherlock-audit/2024-09-predict-fun/blob/41e70f9eed3f00dd29aba4038544150f5b35dccb/predict-dot-loan/contracts/PredictDotLoan.sol#L454-L473

https://github.com/sherlock-audit/2024-09-predict-fun/blob/41e70f9eed3f00dd29aba4038544150f5b35dccb/predict-dot-loan/contracts/PredictDotLoan.sol#L534-L550

https://github.com/sherlock-audit/2024-09-predict-fun/blob/41e70f9eed3f00dd29aba4038544150f5b35dccb/predict-dot-loan/contracts/PredictDotLoan.sol#L564-L604

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

loss of fee for protocol and loss of funds for lenders who will buy the empty loan

PoC

No response

Mitigation

round up while calculating the protocol fee to avoid precision loss reset the loan status after the loan is repaid implement a check in auction and call require(loan.status != LoanStatus.Repaid, "Loan has already been repaid");