Users may not withdraw their tokens.

[code423n4]

Lines of code

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

Vulnerability details

Impact

VTVLVesting's withdraw function has a logic error that impacts the distribution. According to the NATSPEC comment, the users can withdraw their tokens which are fully claimable. However, as per the function's logic users can withdraw their tokens until a limit.

Proof of Concept

withdraw function checks the vested amount by calling _baseVestedAmount _baseVestedAmount checks whether the claim is active and the withdraw request timestamp is inside allowed limits with 3 conditions. And if

            if(_referenceTs > _claim.startTimestamp) {

it checks how many tokens should be allowed to be withdrawn as below;

                uint40 currentVestingDurationSecs = _referenceTs - _claim.startTimestamp; // How long since the start
                // Next, we need to calculated the duration truncated to nearest releaseIntervalSecs
                uint40 truncatedCurrentVestingDurationSecs = (currentVestingDurationSecs / _claim.releaseIntervalSecs) * _claim.releaseIntervalSecs;
                uint40 finalVestingDurationSecs = _claim.endTimestamp - _claim.startTimestamp; // length of the interval

                // Calculate the linear vested amount - fraction_of_interval_completed * linearVestedAmount
                // Since fraction_of_interval_completed is truncatedCurrentVestingDurationSecs / finalVestingDurationSecs, the formula becomes
                // truncatedCurrentVestingDurationSecs / finalVestingDurationSecs * linearVestAmount, so we can rewrite as below to avoid 
                // rounding errors
                uint112 linearVestAmount = _claim.linearVestAmount * truncatedCurrentVestingDurationSecs / finalVestingDurationSecs;

                // Having calculated the linearVestAmount, simply add it to the vested amount
                vestAmt += linearVestAmount;

Permalink

If the user has a previous withdraw, the function returns the allowed withdrawal amount as allowance to withdraw function accordingly as below;

        return (vestAmt > _claim.amountWithdrawn) ? vestAmt : _claim.amountWithdrawn;

Permalink

However, at withdraw function, the allowance should be more than the previously withdrawn amount. (see below)

        require(allowance > usrClaim.amountWithdrawn, "NOTHING_TO_WITHDRAW");

Permalink

So the users are asked to withdraw the amounts more than the previous withdrawal. So it will not be possible to withdraw to full _linearVestAmounts amount. By this approach, a user can withdraw their tokens at once or at portions where the first withdrawal is less than the half of the _linearVestAmounts

Tools Used

Manual Review

Recommended Mitigation Steps

The withdrawal logic should be validated to _linearVestAmounts rather than intermediary allowance.

[0xean]

closing as invalid, I do not follow the logic in this report.

vestAmt would be more than was previously withdrawn and the delta between the two is the amount that is now able to be withdrawn.