[RWV-01M] Discrepancy of Merge/Split Authorizations

[chasebrownn]

RWV-01M: Discrepancy of Merge/Split Authorizations

Type	Severity	Location
Standard Conformity		RWAVotingEscrow.sol:L338, L339, L347-L348, L350, L373, L409, L411

Description:

The present RWAVotingEscrow::merge and RWAVotingEscrow::split systems allow a different caller than the owner of the token IDs involved to execute them, however, their end result differs in terms of authorizations.

In the case of the RWAVotingEscrow::merge function, the caller would retain authorization over the intoTokenId and thus of the merged amounts. On the other hand, in the case of the RWAVotingEscrow::split function the owner will retain ownership (and thus authorization) of the newly minted token IDs while the caller may not necessarily be able to operate on them (f.e. if they were only authorized for the input tokenId).

Impact:

A severity of minor has been assigned as the code is discrepant in its present implementation.

Example:

// fetch owner of tokenId being split
address owner = _requireOwned(tokenId);
// verify ownership of token
_checkAuthorized(owner, _msgSender(), tokenId);
// create array to store all newly minted tokenIds
tokenIds = new uint256[](len);
tokenIds[0] = tokenId;
// find total shares from shares array
uint256 totalShares;
for (uint256 i = len; i != 0;) {
    unchecked {
        --i;
    }
    totalShares += shares[i];
}

// fetch voting escrow data storage
VotingEscrowStorage storage $ = _getVotingEscrowStorage();
// fetch remaining duration of tokenId
uint256 remainingVestingDuration = $._remainingVestingDuration[tokenId];
// fetch lockedBalance of tokenId
uint256 lockedBalance = $._lockedBalance[tokenId];
// store lockedBalance in another var (will be used later)
uint256 remainingBalance = lockedBalance;
// find timestamp of current block
uint48 mintingTimestamp = clock();
// start iterating through shares array
for (uint256 i = 1; i < len;) {
    // grab current share
    uint256 share = shares[i];
    // locked balance for this NFT is percentage of shares * total locked balance
    uint256 _lockedBalance = share * lockedBalance / totalShares;
    // fetch new tokenId to mint
    uint256 newTokenId = _incrementAndGetTokenId();
    // store new tokenId in tokenIds array
    tokenIds[i] = newTokenId;
    // store timeststamp for new token
    $._mintingTimestamp[newTokenId] = mintingTimestamp;
    // mint new token
    _mint(owner, newTokenId);
    // update lock info for new token
    _updateLock(newTokenId, _lockedBalance, remainingVestingDuration);
    // subtract locked balance from total balance
    unchecked {
        remainingBalance -= _lockedBalance;
        ++i;
    }
}

Recommendation:

We advise this discrepancy to be dealt with, either by disallowing RWAVotingEscrow::split invocations via authorizations or by ensuring that the msg.sender is authorized for the newly minted token IDs. We consider any of the aforementioned solutions as acceptable alleviations for this exhibit.

[chasebrownn]

Resolved