withdrawToLandlord() cant send all rent earned overtime to landlord

[c4-bot-9]

Lines of code

https://github.com/code-423n4/2024-10-coded-estate/blob/97efb35fd3734676f33598e6dff70119e41c7032/contracts/codedestate/src/execute.rs#L1823-L1834

Vulnerability details

Impact

withdrawtolandlord() function does not allow token.ower or any approved/operator address to withdraw all the earned rent to the provided landlord address

Proof Of Concept

https://github.com/code-423n4/2024-10-coded-estate/blob/97efb35fd3734676f33598e6dff70119e41c7032/contracts/codedestate/src/execute.rs#L1823-L1834

           for (i, item) in token.rentals.iter().enumerate() {
            if item.address == Some(Addr::unchecked(tenant.clone()))
                && item.renting_period[0].to_string() == renting_period[0]
                && item.renting_period[1].to_string() == renting_period[1]
            {
                position = i as i32;
                if item.cancelled {
                    return Err(ContractError::NotApproved { });
                }
                if item.deposit_amount - Uint128::from(token.longterm_rental.price_per_month) < Uint128::from(amount)  { 
                    return Err(ContractError::UnavailableAmount {  });
                }

            }

The snippet above is from the iteration logic in withdrawtolandlord(). The inner if statement here does not allow the landlord to receive 100% of the rental value it has earned on the protocol.

For example if a tennant has paid 10,000 USD over the span of 10 months and price_per_month is 1000 USD, the landlord wants to receive its 10,000 USD which is the total rent earned over time, the logic there will interpret to be:

item.deposit_amount = 10,000
token.longterm_rental.price_per_month = 1000
amount = 10,000

                if  10,000 - Uint128::from(1000) < Uint128::from(10,000)  { 
                    return Err(ContractError::UnavailableAmount {  });
                }

10,000 - 1,000 = 9,000 is lower than 10,000 (the amount to be withdrawn) so this means the function will revert with unavailableAmount error. A months's rent will be stuck in the protocol and cannot be sent to the landlord.

The validation for amount to be withdrawn to landlord is incorrect.

Recommened Mitigation

change the validation to be

if  Uint128::from(amount) > item.deposit_amount { 
                    return Err(ContractError::UnavailableAmount {  });
                }

Assessed type

Context

[c4-judge]

OpenCoreCH marked the issue as primary issue

[blockchainstar12]

it is intended logic. Owners can withdraw all funds in withdrawtolandlord() function, so it's not needed to withdraw all funds at this function

[OpenCoreCH]

@blockchainstar12 I cannot follow your argumentation here, the issue is about the withdrawtolandlord() function. Should the owner be able to withdraw all funds or is it intended that they have to leave token.longterm_rental.price_per_month in the deposit?

[blockchainstar12]

withdrawtolandlord function is not designed to withdraw all funds from the contract, it is for monthly withdrawal, we have finalizelongtermrental function for full withdraw

[OpenCoreCH]

Ok that makes sense, so this indeed is not a problem because there is another function for withdrawing everything.

[c4-judge]

OpenCoreCH marked the issue as unsatisfactory:
Invalid