TangibleTNFT / baskets-foundry

baskets-foundry
2 stars 0 forks source link

[BTE-13M] Incorrect Calculation of Rental Income #20

Closed chasebrownn closed 9 months ago

chasebrownn commented 9 months ago

BTE-13M: Incorrect Calculation of Rental Income

Type Severity Location
Logical Fault Basket.sol:L559

Description:

The Basket::rebase function will incorrectly calculate rental income as it will not claim it when a rebase operation occurs.

Should the rentFee be sufficiently small that no claims need to be performed, multiple Basket::rebase operations can be performed to artificially inflate the evaluation of the Basket leading to significant attacks against depositors.

Impact:

The current rebasing system is incorrect as it will track the same pending-claim rent multiple times.

Example:

/**
 * @notice This function allows for the Basket token to "rebase" and will update the multiplier based
 * on the amount of rent accrued by the basket tokens.
 */
function rebase() public {
    uint256 previousRentalIncome = totalRentValue;
    uint256 totalRentalIncome = _getRentBal();

    uint256 collectedRent = totalRentalIncome - previousRentalIncome;

    // Take 10% off collectedRent and send to revenue contract
    uint256 rentDistribution = (collectedRent * rentFee) / 100_00;
    collectedRent -= rentDistribution;

    uint256 rebaseIndexDelta = (collectedRent * decimalsDiff()) * 1e18 / getTotalValueOfBasket();

    uint256 rebaseIndex = rebaseIndex();

    rebaseIndex += rebaseIndexDelta;
    totalRentValue = totalRentalIncome;

    _transferRent(_getRevenueDistributor(), rentDistribution);
    _setRebaseIndex(rebaseIndex);

    emit RebaseExecuted(msg.sender, totalRentValue, rebaseIndex);
}

Recommendation:

We advise the code to either properly claim pending rent, or to track the "calculated" pending rent by the Basket::_getRentBal function invocation within Basket::rebase operations, ensuring that only newly accumulated rent is tracked in rebase operations.

chasebrownn commented 9 months ago

@omniscia-core rent does not need to be claimed. When we rebase we take the difference between the last rental income total calculation and the current. No matter if we claim, it's still accounted for. the previous rental income amount is stored in totalRentValue and is compared with the new balance returned by _getRentBal to calculate rebase

chasebrownn commented 9 months ago

Nullified