RentManager Manual Review Findings

RMR-01M: Incorrect Maintenance of Claimed Amount Total

Type	Severity	Location
Logical Fault		RentManager.sol:L421, L428

Description:

The RentManager::_claimRentForToken function will incorrectly maintain the claimedAmountTotal of a particular RentInfo entry as it will not increment the total by the unclaimedAmount acquired.

Impact:

While the value will be improperly maintained, it does not appear to be utilized in any sensitive logic of the system thus rendering the vulnerability to be of "minor" severity.

Example:

Copy
417if (rent.unclaimedAmount > 0) {
418    if (rent.unclaimedAmount < claimableRent) {
419        unchecked {
420            rent.claimedAmount += claimableRent - rent.unclaimedAmount;
421            rent.claimedAmountTotal += claimableRent - rent.unclaimedAmount;
422            rent.unclaimedAmount = 0;
423        }
424    } else {
425        // this should never happen
426        unchecked {
427            rent.unclaimedAmount -= claimableRent;
428            rent.claimedAmountTotal += claimableRent;
429        }
430    }
431} else {

Recommendation:

We advise the code to increment the claimedAmountTotal by the claimableRent rather than the claimableRent minus the rent.unclaimedAmount as the unclaimedAmount that is immediately vested should increment the total regardless.

Alleviation (2ad448279d9e8e4b6edd94bcd2eb22129b6f7357):

The code was updated per our recommendation, no longer subtracting the unclaimedAmount on each iteration to provide a transparent claimedAmountTotal that more accurately matches its expected description.

RMR-02M: Inexistent Deletion of Deposit Time

Type	Severity	Location
Logical Fault		RentManager.sol:L295-L299

Description:

The RentManager::pauseAndClaimBackDistribution function will not properly reset the RentInfo entry of the contract as the depositTime will remain non-zero after its execution.

Impact:

The RentManager::pauseAndClaimBackDistribution function will not properly reset the depositTime to its default value despite its specification which can lead to issues when resuming the rent for a token via the RentManager::deposit function and setting skipBackpayment to false.

Example:

Copy
278function pauseAndClaimBackDistribution(uint256 tokenId) external returns (uint256 amount) {
279    require(msg.sender == depositor, "Only the rent depositor can call this function");
280    RentInfo storage rent = rentInfo[tokenId];
281    // claim for owner of tnft
282    _claimRentForToken(IERC721(TNFT_ADDRESS).ownerOf(tokenId), tokenId);
283    // take the rest of the rent and reset
284    amount = rent.depositAmount - rent.claimedAmount;
285    emit DistributionPaused(
286        rent.rentToken,
287        tokenId,
288        amount,
289        rent.depositAmount,
290        block.timestamp - rent.depositTime,
291        rent.depositTime
292    );
293    IERC20(rent.rentToken).safeTransfer(depositor, amount);
294    // reset
295    rent.claimedAmount = 0;
296    rent.depositAmount = 0;
297    rent.distributionRunning = false;
298    rent.endTime = 0;
299    rent.rentToken = address(0);
300}

Recommendation:

We advise the depositTime value to be set to a value of 0 as well, ensuring that the RentInfo structure is correctly reset.

Alleviation (2ad448279d9e8e4b6edd94bcd2eb22129b6f7357):

The depositTime entry is correctly zeroed out in the updated code, rendering this exhibit fully alleviated.