028.md

Kind Aqua Ostrich

High

Unchecked transfer operations in the _seize function will leave the lender without the collateral in PredictDotLoan.sol

Summary

Unchecked transfer operations in the _seize function in PredictDotLoan.sol will leave the lender without the collateral.

Root Cause

In the _seize function, the collateral is transferred to the lender when the loan is defaulted. However, the safeTransferFrom call to transfer collateral does not check for successful execution. If the transfer fails, the loan status is still updated to Defaulted, potentially leaving the lender without the collateral. https://github.com/sherlock-audit/2024-09-predict-fun/blob/main/predict-dot-loan/contracts/PredictDotLoan.sol#L877-L883 An attacker (borrower) could intentionally cause the transfer of collateral to fail (for example, by pausing the CTF token contract or blocking the token transfer). Even though the transfer fails, the loan’s status would still be set to Defaulted, and the borrower could avoid losing their collateral while the lender would receive no compensation.

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

1. The borrower takes out a loan.
2. The lender calls the _seize function to claim the collateral when the borrower defaults.
3. The token transfer fails due to a manipulated or paused token contract.
4. Despite the failure of the token transfer, the loan status is updated to Defaulted, causing the lender to lose the collateral and the borrower retains their assets.

In PoC below: Malicious token contract (MaliciousToken):

1. The MaliciousToken contract is a mock implementation of an ERC1155 token. It has a blockTransfers flag that can be set to block any token transfers by causing the safeTransferFrom function to revert.
2. When blockTransfers is set to true, any attempt to transfer tokens will fail with a "Transfers are blocked!" error.

Seize exploit contract (SeizeExploit):

1. This contract interacts with the vulnerable loan contract.
2. The exploitSeize function blocks token transfers by setting blockTransfers to true in the MaliciousToken contract.
3. It then calls the _seize function on the vulnerable loan contract, which will attempt to transfer the collateral but fail due to the blocked transfer.
4. Despite the failure, the loan's status will be updated to Defaulted, allowing the borrower to retain their collateral without paying off the loan.

Impact

The lender loses the collateral while the borrower still retains control of the assets, resulting in a significant financial loss for the lender. The protocol's integrity is compromised as loan statuses do not reflect the actual transfer of collateral, leading to systemic financial risks.

PoC

// Mock token contract that will reject transfers
contract MaliciousToken is ERC1155 {
    bool public blockTransfers = false;

    function setBlockTransfers(bool _block) external {
        blockTransfers = _block;
    }

    // Override transfer function to fail when blockTransfers is true
    function safeTransferFrom(
        address from,
        address to,
        uint256 id,
        uint256 amount,
        bytes memory data
    ) public override {
        require(!blockTransfers, "Transfers are blocked!");
        super.safeTransferFrom(from, to, id, amount, data);
    }
}

// PoC contract to exploit the vulnerability
contract SeizeExploit {
    LoanContract loanContract;   // Assume this is the vulnerable contract
    MaliciousToken maliciousToken; // The token used for collateral

    constructor(address _loanContract, address _maliciousToken) {
        loanContract = LoanContract(_loanContract);
        maliciousToken = MaliciousToken(_maliciousToken);
    }

    function exploitSeize(uint256 loanId) external {
        // Step 1: Block the transfer of collateral
        maliciousToken.setBlockTransfers(true);

        // Step 2: Call the seize function to default the loan
        loanContract.seize(loanId);  // This will fail to transfer but still mark the loan as defaulted

        // Step 3: The loan is marked as Defaulted, but the collateral is not transferred
    }
}

Mitigation

The transfer should be wrapped in a try/catch block or its return value should be checked to ensure the state is only updated after a successful transfer. Here’s an example mitigation:

function _seize(uint256 loanId, Loan storage loan) private {
    loan.status = LoanStatus.Defaulted;

    try CTF.safeTransferFrom(address(this), msg.sender, loan.positionId, loan.collateralAmount, "") {
        emit LoanDefaulted(loanId);
    } catch {
        revert("Collateral transfer failed, loan not defaulted");
    }
}

With this mitigation, the loan status will only be marked as Defaulted if the collateral transfer succeeds.