Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

QA Report #15

Open
c4-bot-9 opened this issue Oct 10, 2024 · 5 comments
Open

QA Report #15

c4-bot-9 opened this issue Oct 10, 2024 · 5 comments
Labels
2nd place bug Something isn't working edited-by-warden grade-a Q-03 QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")

Comments

@c4-bot-9
Copy link
Contributor

See the markdown file with the details of this report here.

@c4-bot-9 c4-bot-9 added bug Something isn't working QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax labels Oct 10, 2024
c4-bot-4 added a commit that referenced this issue Oct 10, 2024
c4-bot-8 added a commit that referenced this issue Oct 10, 2024
c4-bot-9 added a commit that referenced this issue Oct 11, 2024
@blockchainstar12 blockchainstar12 added the sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") label Oct 14, 2024
@c4-judge c4-judge added the selected for report This submission will be included/highlighted in the audit report label Oct 16, 2024
@c4-judge
Copy link
Contributor

OpenCoreCH marked the issue as selected for report

@c4-judge
Copy link
Contributor

OpenCoreCH marked the issue as grade-a

@c4-judge
Copy link
Contributor

OpenCoreCH marked the issue as not selected for report

@c4-judge c4-judge removed the selected for report This submission will be included/highlighted in the audit report label Oct 16, 2024
@nnez
Copy link

nnez commented Oct 17, 2024

Hi @OpenCoreCH
I have two issues regarding my Low/QA findings:

1.

L-07 Lanlord should only allow to call withdrawtolandlord for approved rental should be a duplicate of #37 because it describes the same exact scenario

However, I think the root cause here is that the function allows withdrawal of unapproved long-term rental. The token owner should be able to reject active long-term reservations as this could be used to free up availability in case of violation on rental agreement.

The token owner should have more authority than the renter in long-term rental because they can't demand a full payment for the total rental period (it doesn't make sense to do so, you don't pay 12 months in advance for renting an apartment for a year). To illustrate my point, if the renter were to miss their payment for three months, they might face an eviction in the real world, Similarly, they must be evicted on the smart contract as well.

2.

L-02 Token owner might frontrun reservation transaction and change cancellation policy in short-term rental should be a duplicate of #35, as it describes the same root cause: a malicious cancellation policy.

The specific nature of the malicious cancellation policy is irrelevant. It could be a large-size vector or a cancellation fee of 100% or more. It would still prevent renter from cancelling their reservation.

I submitted this issue as a Low severity because

  1. Token owner can't change their cancellation policy while there are rentals (active or non-active) in their token, therefore, this can only be done on the first rental on the property.
  2. This scenario requires a degree of user error, as users should be able to view the cancellation policy before making a reservation. Hence, malicious renters would need to frontrun user transactions to change the cancellation policy.

@thebrittfactor
Copy link

For awarding purposes, C4 staff have marked as 2nd place.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2nd place bug Something isn't working edited-by-warden grade-a Q-03 QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
Projects
None yet
Development

No branches or pull requests

7 participants