Assert indicates unnecessary check or missing constraint/logic

[code423n4]

Handle

0xRajeev

Vulnerability details

Impact

updateLastRentalTime() function “tracks when the user last rented so they cannot rent and immediately withdraw thus bypassing minimum rental duration.”

This function currently always returns true and so there is no need to assert its return value, as done in newRental(), unless it was meant to return false in some scenarios which indicates missing constraint/logic. It is not clear what that might be.

Impact: Given that the minimum rental duration is one of the two key protection mechanisms, any missing logic/constraint here could affect the project significantly.

Proof of Concept

https://github.com/code-423n4/2021-06-realitycards/blob/86a816abb058cc0ed9b6f5c4a8ad146f22b8034c/contracts/RCMarket.sol#L731

https://github.com/code-423n4/2021-06-realitycards/blob/86a816abb058cc0ed9b6f5c4a8ad146f22b8034c/contracts/RCTreasury.sol#L484-L498

Tools Used

Manual Analysis

Recommended Mitigation Steps

Validate constraint/logic to see if function should return false in any scenario. Remove assert at call site if otherwise.

[Splidge]

Duplicate of #55

[dmvt]

I do not see how this is a duplicate of #55.

[Splidge]

Sorry, I think it must have been #53 I wanted to mark it a duplicate of.
Although there is also some overlap with #83 as the assert wasn't used correctly.