Potential Contract Drain Due to Lack of User Investment Tracking

Summary

The contract currently doesn't keep track of how much each user has invested. This could lead to problems when users want to withdraw their funds. For example, a bad actor might try to take more money than they actually put in, potentially causing issues for the protocol. Simply just relies on the total amount claimable in the contract

Vulnerability Detail

The main issue is the absence of a system to link each user's address with the amount they deposited. Without this, there's a risk that users may try to claim more tokens than they originally invested, which could harm the fairness of the protocol.

Impact

This vulnerability could result in inaccurate calculations during withdrawals and create a situation where a user might claim more tokens than they actually put in. This could undermine the trust and reliability of the protocol.

Code Snippet

https://github.com/sherlock-audit/2024-01-rio-vesting-escrow/blob/2d14c2b84b69c53a45c81aa4f907af9617f9a94f/rio-vesting-escrow/src/VestingEscrow.sol#L136C5-L144C6

function claim(address beneficiary, uint256 amount) external onlyRecipient returns (uint256) {
        uint256 claimable = Math.min(unclaimed(), amount);
        totalClaimed += claimable;

        token().safeTransfer(beneficiary, claimable);
        emit Claim(beneficiary, claimable);

        return claimable;
    }

/// @notice Claim tokens which have vested.
/// @param beneficiary Address to transfer claimed tokens to.
/// @param amount Amount of tokens to claim.
function claim(address beneficiary, uint256 amount) external onlyRecipient returns (uint256) {
    uint256 claimable = Math.min(unclaimed(), amount);

    // Check if the user has enough invested to claim the requested amount
    require(userInvestments[beneficiary] >= claimable, "Insufficient invested amount");

    totalClaimed += claimable;

    // Update the user's investment
    userInvestments[beneficiary] -= claimable;

    token().safeTransfer(beneficiary, claimable);
    emit Claim(beneficiary, claimable);

    return claimable;
}

Tool used

Manual Review

Recommendation

To fix this issue, implement a system (using the userInvestments mapping) to link each user's address with the amount they've invested. Make sure to update the contract logic to use this mapping for withdrawal calculations to ensure accurate and secure tracking of user contributions.

// Mapping to track user investments (to be added)
mapping(address => uint256) public userInvestments;
...

sherlock-audit / 2024-01-rio-vesting-escrow-judging

phenom - Potential Contract Drain Due to Lack of User Investment Tracking #85