Vesting revoke will disallow receiver from receiving already unlocked tokens

[code423n4]

Lines of code

https://github.com/code-423n4/2022-09-vtvl/blob/f68b7f3e61dad0d873b5b5a1e8126b839afeab5f/contracts/VTVLVesting.sol#L432

Vulnerability details

Impact

Unfair loss of tokens for the receiver.

Proof of Concept

Vesting is used by employers to align incentives in startups and prevent employees from leaving the company if they want to get the vested tokens. This is why a revoke function was included.

From the documentation : "However, due to the nature of vesting, e.g. employee token vesting, our vesting contract is deliberately designed to allow admin revocation in the circumstances of early employment termination (before the end of vesting period specified)."

The current implementation of revoke is flawed since it disallows the receiver from getting tokens that were already unlocked. Admin revokation in case of early termination should be possible but the employee should be able to withdraw his tokens that were already released. If the employee didn't call the withdraw() function before termination he won't be able to get these (already released) tokens.

The revoke feature should only revoke future tokens that weren't still released. This is how vestings work, if you leave early you only get what was already released but not 0.

Recommended Mitigation Steps

In the revoke function make a withdraw on behalf of the receiver before revoking. In that way, he will get his released tokens and be revoked from future releases.

[indijanc]

Duplicate of #475

[0xean]

closing as dupe