[RWV-04M] Inexistent Prevention of Self-Merging

RWV-04M: Inexistent Prevention of Self-Merging

Type	Severity	Location
Logical Fault		RWAVotingEscrow.sol:L334-L352

Description:

The RWAVotingEscrow::merge function will result in loss of funds and corrupt the total voting power entry in the contract if the input tokenId and intoTokenId match. Specifically, the function will calculate the new combinedLockBalance as double the original, the remainingVestingDuration the same as the original, and will ultimately burn the token with the inflated entries thereby causing the tokens associated with it to become irredeemable.

Impact:

Merging of a token with itself will result in loss of funds as well as an inflated _lockedBalance and potentially non-zero _remainingVestingDuration which would result in this exhibit being medium in severity.

After re-evaluation, we identified that it is possible for the contract's $._totalVotingPowerCheckpoints entry to be manipulated in this way and thus cause the governance system to become inaccessible thereby rendering this to be upgraded to major.

Example:

function merge(uint256 tokenId, uint256 intoTokenId) external {
    address owner = _requireOwned(tokenId);
    address targetOwner = _requireOwned(intoTokenId);

    _checkAuthorized(owner, _msgSender(), tokenId);
    _checkAuthorized(targetOwner, _msgSender(), intoTokenId);

    VotingEscrowStorage storage $ = _getVotingEscrowStorage();

    uint256 combinedLockBalance = $._lockedBalance[tokenId] + $._lockedBalance[intoTokenId];
    uint256 remainingVestingDuration =
        Math.max($._remainingVestingDuration[tokenId], $._remainingVestingDuration[intoTokenId]);

    _updateLock(tokenId, 0, 0);
    _updateLock(intoTokenId, combinedLockBalance, remainingVestingDuration);

    _burn(tokenId);
    emit lockMerged(tokenId, intoTokenId);
}

Recommendation:

We advise the code to prevent the same tokenId from being merged with itself by ensuring that the tokenId is not equal to the intoTokenId.

re-al-Foundation / rwa-contracts