Vesting.sol#vest() Anyone can make other users' vestings revokable

[code423n4]

Handle

WatchPug

Vulnerability details

vest() can be called by anyone with an arbitrary _beneficiary address to add a Timelock (vesting) to the _beneficiary.

At L83-88, it changes the global storage of revokable settings for the _beneficiary.

This allows anyone to change the revokable settings for other users. Non-revocable vestings can later be changed into revokable, and then be revoked, causing the user to lose funds.

https://github.com/code-423n4/2021-11-bootfinance/blob/7c457b2b5ba6b2c887dafdf7428fd577e405d652/vesting/contracts/Vesting.sol#L73-L98

function vest(address _beneficiary, uint256 _amount, uint256 _isRevocable) external payable whenNotPaused {
    require(_beneficiary != address(0), "Invalid address");
    require( _amount > 0, "amount must be positive");
    // require(totalVestedAmount.add(_amount) <= maxVestingAmount, 'maxVestingAmount is already vested');
    require(_isRevocable == 0 || _isRevocable == 1, "revocable must be 0 or 1");
    uint256 _unlockTimestamp = block.timestamp.add(unixYear);

    Timelock memory newVesting = Timelock(_amount, _unlockTimestamp);
    timelocks[_beneficiary].push(newVesting);

    if(_isRevocable == 0){
        benRevocable[_beneficiary] = [false,false];
    }
    else if(_isRevocable == 1){
        benRevocable[_beneficiary] = [true,false];
    }

    totalVestedAmount = totalVestedAmount.add(_amount);
    benTotal[_beneficiary] = benTotal[_beneficiary].add(_amount);

    // transfer to SC using delegate transfer
    // NOTE: the tokens has to be approved first by the caller to the SC using `approve()` method.
    vestingToken.transferFrom(msg.sender, address(this), _amount);

    emit TokenVested(_beneficiary, _amount, _unlockTimestamp, block.timestamp);
}

Recommendation

Consider making the revokable settings set per Timelock instead of per address.

[chickenpie347]

Addressed in #132.