Users will wait untille the last day to cancel reservation to avoid cancellation penalty

[c4-bot-3]

Lines of code

https://github.com/code-423n4/2024-10-coded-estate/blob/main/contracts/codedestate/src/execute.rs#L1082

Vulnerability details

Description

This protocol gives the right to homeowners to apply a penalty for cancellation after the rent gets approved
The logic is here in execute.rs#cancelreservationafterapprovalforshortterm() function

        let diff_days = (check_in_time_timestamp - current_time)/86400;
        for (_i, item) in cancellation.iter().enumerate() {
            if item.deadline < diff_days {
                refundable_amount =  Uint128::new((amount.u128() * u128::from(item.percentage)) / 100);
                break;
            }
        }

so the cancellation penalty depends on how many days are left before your rent starts.
It calculates the diff_days by subtracting the check-in time from the current time and dividing by 86400
How ever in case the subtracting gives a number less than 86400 the diff_days will be zero

and this check here if item.deadline < diff_days { will never be true
So these two lines will set the token.rentals[].cancelled to true and keep the token.rentals[].deposit_amount same as before because the refundable_amount is zero

            token.rentals[position as usize].cancelled = true;
            token.rentals[position as usize].deposit_amount = amount - refundable_amount;

Impact

users will wait until the last day to cancel the reservation and withdraw all the initial deposit even with the existing of cancellation penalty.

Tools Used

Manual Review

Recommended Mitigation Steps

      for (_i, item) in cancellation.iter().enumerate() {
 -           if item.deadline < diff_days {
+            if item.deadline <= diff_days {
                refundable_amount =  Uint128::new((amount.u128() * u128::from(item.percentage)) / 100);
                break;
            }
        }

Assessed type

Other

[c4-judge]

OpenCoreCH marked the issue as primary issue

[blockchainstar12]

Withdrawable amount is "refundable_amount", NOT "deposit_amount"

and once "cancelled" flag is true, users cannot withdraw any funds!

[OpenCoreCH]

The usage of < or <= in this comparison just seems to be a design decision that alters the meaning of the deadline field in a CancellationItem slightly (inclusive or exclusive).

[c4-judge]

OpenCoreCH marked the issue as unsatisfactory:
Invalid