re-al-Foundation / rwa-contracts

0 stars 0 forks source link

[CCM-01M] Incorrect Utilization of Max Pay-Out #27

Closed chasebrownn closed 6 months ago

chasebrownn commented 6 months ago

CCM-01M: Incorrect Utilization of Max Pay-Out

Type Severity Location
Logical Fault CrossChainMigrator.sol:L210, L284

Description:

The code of CrossChainMigrator::migrateNFT and CrossChainMigrator::migrateNFTBatch functions will re-calculate the multiplier due for the lock and compare it to its active one.

If the calculated multiplier is different than the one associated with the lock, it means that an IPassiveIncomeNFT::claim occurred that was penalized based on the IPassiveIncomeNFT implementation. The code of CrossChainMigrator, however, will assign the amount of tokens to bridge as equal to the maxPayout which is incorrect.

The IPassiveIncomeNFT contract will decrease the multiplier proportionately based on the percentage of tokens that were claimed as penalized, however, the newMaxPayout the code will calculate utilizes the full lockedAmount of the lock and does not account for the funds that have been claimed.

Impact:

Penalized NFTs that have had their multiplier diminished will result in a significant portion of tokens being minted erroneously as the amount of funds claimed are not accounted for in the NFT.

Example:

uint256 amountTokens;
if (multiplier != piCalculator.determineMultiplier(BOOST_START, BOOST_END, startTime, uint8((endTime - startTime) / 30 days))) {
    // if early claim, just mint them remaining in `maxPayout`.
    amountTokens = maxPayout;
}
else {
    // otherwise, just calculate amount to mint/lock as normal.
    amountTokens = lockedAmount + ((lockedAmount * (multiplier - 1e18)) / 1e18) - claimed;
}

Recommendation:

We advise the code to ensure the claimed tokens are properly subtracted from the maxPayout, preventing a bridge operation from minting more tokens than expected for the NFT.

chasebrownn commented 6 months ago

So when there exists an early claim, the user is penalized and the multiplier is lowered thus lowering the amount that the user can claim next. The PassiveIncomeNFT contract drops the multiplier in response to the early claim so there's no need to subtract the amount claimed. This is an intentional method for calculating amountTokens given the NFT experienced an early claim.