re-al-Foundation / rwa-contracts

0 stars 0 forks source link

[VEV-01M] Potentially Outdated Data Point #66

Closed chasebrownn closed 5 months ago

chasebrownn commented 5 months ago

VEV-01M: Potentially Outdated Data Point

Type Severity Location
Logical Fault VotingEscrowVesting.sol:L127

Description:

The VotingEscrowVesting::deposit function will snapshot the locked amount of the token ID during the deposit to the vestingSchedules entry, however, the balance associated with the tokenId may change after it has been deposited to the VotingEscrowVesting contract via the RWAVotingEscrow::depositFor function.

Impact:

The severity of this exhibit cannot be quantified as usage of the vestingSchedules entry is unknown.

Example:

/**
 * @dev Deposits a VotingEscrow token to start its vesting schedule. The function records the vesting schedule,
 * removes the token's voting power, and transfers the token to this contract for vesting.
 *
 * @param tokenId The identifier of the VotingEscrow token to be deposited.
 * The vesting schedule is based on the remaining vesting duration and locked amount of the token.
 */
function deposit(uint256 tokenId) external nonReentrant {
    uint256 duration = votingEscrow.getRemainingVestingDuration(tokenId);
    uint256 startTime = block.timestamp;
    uint256 endTime = startTime + duration;
    uint256 amount = votingEscrow.getLockedAmount(tokenId);

    _addTokenToDepositorEnumeration(msg.sender, tokenId);
    // Store the vesting schedule for the token in `vestingSchedules`
    vestingSchedules[tokenId] = VestingSchedule(startTime, endTime, amount);
    // set new vesting duration to 0 on nft contract -> vesting contract should not have voting power
    votingEscrow.updateVestingDuration(tokenId, 0);
    // transfer NFT from depositor to this contract
    votingEscrow.transferFrom(msg.sender, address(this), tokenId);
}

Recommendation:

We advise either the data point to not be stored locally, or deposits to be prevented at the RWAVotingEscrow level either of which we consider an adequate resolution to this exhibit.

chasebrownn commented 5 months ago

Resolved