Skip to content

Commit

Permalink
Ch_301 data for issue #30
Browse files Browse the repository at this point in the history
  • Loading branch information
c4-bot-8 committed Oct 11, 2024
1 parent b492af9 commit c127b0a
Showing 1 changed file with 139 additions and 0 deletions.
139 changes: 139 additions & 0 deletions data/Ch_301-Q.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
# Issue summary

| Issue Number | Title |
|--------------|-------------------------------------------------------------------------------------------------------------|
| L-1 | The current logic can't handle CW20 tokens|
| L-2 | Malicious owners can set the fee to 100% |
| L-3 | `auto_approve` is not used in long-term rent |
| L-4 | `minter` is not used in this contract delete it |
| L-5 | Use `to_json_binary` and `from_json_binary` |
| L-6 | `cosmwasm-std` 1.4.0v is vulnerable |
| L-7 | The first buyer could get front-runed after `autoApprove` get updated |
| L-8 | `available_period` is not used |
| L-9 | The logic doesn't return the excided funds to the users |
| L-10 | Risk to run out-of-gas |
| L-11 | Risk of 100% cancellation penalty for users |
| L-12 | `check_can_edit_long()` and `check_can_edit_short()` have the same logic |
| L-12 | DoS attack |
| L-12 | Use a daily or monthly basis |
| L-12 | The function `execute.rs#depositforlongtermrental()` doesn't check if the deposit amount is enough for the reserved period. |


# [L-1] The current logic can't handle CW20 tokens
Travelers can't make reservations with CW20 (but the readME says: **ERC20 used by the protocol Any (all possible ERC20s))**

https://github.com/code-423n4/2024-10-coded-estate/blob/main/README.md#general-questions

# [L-2] Malicious owners can set the fee to 100%
Malicious owners can set the fee to 100% by triggering `execute.rs#set_fee_value()`, this will leave homeowners with zero revenue

https://github.com/code-423n4/2024-10-coded-estate/tree/main/contracts/codedestate/src#L318-L323

# [L-3] `auto_approve` is not used in long-term rent
the `execute.rs#setlistforlongtermrental()` function lets NFT owner set the `auto_approve`, but it is not used in the logic of long-term rent

https://github.com/code-423n4/2024-10-coded-estate/tree/main/contracts/codedestate/src#L1288


# [L-4] `minter` is not used in this contract delete it
the struct I`nstantiateMsg` has a `pub minter: String,`
this minter is no longer used In this cw721 contract.

Also all the `query.rs#minter()`

https://github.com/code-423n4/2024-10-coded-estate/blob/97efb35fd3734676f33598e6dff70119e41c7032/contracts/codedestate/src/query.rs#L418-L424

# [L-5] Use `to_json_binary` and `from_json_binary`

`to_binary` and `from_binary` are deprecated so replace with: `to_json_binary` and `from_json_binary`
Check [THIS](https://github.com/public-awesome/cw-nfts/issues/141) for more details

https://github.com/code-423n4/2024-10-coded-estate/blob/main/packages/cw721/src/receiver.rs#L26

# [L-6] `cosmwasm-std` 1.4.0v is vulnerable

using a vulnerable version of `cosmwasm-std`
check [HERE](https://github.com/CosmWasm/advisories/blob/main/CWAs/CWA-2024-002.md) for more details
```rust
File: Cargo.lock

157: [[package]]
158: name = "cosmwasm-std"
159: version = "1.4.0"
```
https://github.com/code-423n4/2024-10-coded-estate/blob/main/Cargo.lock#L158-L159

# [L-7] The first buyer could get front-runed after `autoApprove` get updated

If NFT is not `autoApprove`, in case the user calls the `execute.rs#setbidtobuy()` function then the owner updates the `autoApprove to true.
any other user could call the `execute.rs#setbidtobuy()` function and buy it (transfer it),
So, the first user wants to be able to buy it even if he bays first. it should be transferred to the first bid

# [L-8] `available_period` is not used

The NFT owner is able to set the `available_period: Vec<String>` but it never gets checked in this contract

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

# [L-9] The logic doesn't return the excided funds to the users

When the user calls `execute.rs#setreservationforshortterm()` to send more funds than price + fee, he will not receive it back
it will go to the protocol.
https://github.com/code-423n4/2024-10-coded-estate/blob/main/contracts/codedestate/src/execute.rs#L866

# [L-10] Risk of out-of-gas
in `execute.rs`, multiple iterations occur over the `token.rentals` vector, which may cause the transaction to fail due to an out-of- gas error.
specifically in `setreservationforshortterm()` and `setapproveforshortterm()`
Consequently, malicious users could exploit this by opening many reservations to force `setapproveforshortterm()` to fail due to gas limits.

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

# [L-11] Risk of 100% cancellation penalty for users

Malicious NFT owners could percentage of cancellations to 100% in short-term reservations.

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

# [L-12] `check_can_edit_long()` and `check_can_edit_short()` have the same logic
the NFT owner can't un-list the LongRent only or ShortRant
So, in case I have only one going short rent and I want to unlist my NFT from the LongRent.
is not possible.
Because, both `check_can_edit_long()` and `check_can_edit_short()` have the same logic,
you need to check the `rental_type`
not just the last one in `rentals: vec<Rantal>`

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

# [L-13] DoS attack
In the long-term malicious addresses can keep reserving one big period or multiple small ones. by triggering `execute.rs#setreservationforlongterm()`
the attacker will only lose the gas fee
because the logic doesn't for users to deposit funds first in order to reserve for long-term rent.

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

# [L-14] Use a daily or monthly basis
This checks the minimum stay for long-term rent in `execute.rs#setreservationforlongterm()`
```rust
if ((new_checkout_timestamp - new_checkin_timestamp)/ 86400) < token.longterm_rental.minimum_stay {
return Err(ContractError::LessThanMinimum {});
}
```
we can assume the `token.longterm_rental.minimum_stay` is a daily basis. But on the other side, we have `price_per_month` which is a monthly basis. and this could confuse NFT owners.

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

# [L-15] The function `execute.rs#depositforlongtermrental()` doesn't check if the deposit amount is enough for the reserved period.

In the long-term rental functions, the user will call `execute.rs#setreservationforlongterm()` to reserve the period first. He needs to trigger `execute.rs#depositforlongtermrental()` to deposit the necessary amount.
NFT owner will call `setapproveforlongterm()` but it doesn't check whether the rental has deposited the required funds or not.

This is not a big problem because the NFT owner still able to reject or approve the reservation.

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



0 comments on commit c127b0a

Please sign in to comment.