Two address tokens can be withdrawn by the admin even if they are vested

[code423n4]

Lines of code

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

Vulnerability details

Impact

Two address tokens exists in the blockchain. For example, Synthetix's ProxyERC20 contract is such a token which exists in many forms (sUSD, sBTC...). Tokens as such can be vested, but the admin can withdraw them even if they are vested by providing the other address to the withdrawOtherToken function. The only check in this function is that _otherTokenAddress != tokenAddress, which is irrelevant in the case of two address tokens.

This can make the admin be able to withdraw the vested funds and break the system, because the balance of the contract can be less than the vested amount.

Proof of Concept

1. The VTVLVesting is deployed with the sUSD contract, using its main (proxy) address - 0x57Ab1ec28D129707052df4dF418D58a2D46d5f51.
2. A claim is created for Alice, vesting 1000 sUSD in linear vesting. Assuming this is the only claim currently, the balance of the contract is 1000 sUSD and the value of numTokensReservedForVesting is 1000e18.
3. The admin calls the withdrawOtherToken for 1000e18 sUSD, providing its second address - 0x57Ab1ec28D129707052df4dF418D58a2D46d5f51. The value of numTokensReservedForVesting is still 1000e18, but the balance of the contract is now 0 sUSD.
4. Alice waits for her vest to end, calls the withdraw function, but the function reverts on the call to safeTransfer() because there is insufficient balance of sUSD. Alice can't receive her funds.

Tools Used

Manual audit

Recommended Mitigation Steps

Replace the address check with a balance check - record the vesting token balance of the contract before and after the transfer and assert that they are equal.

[0xean]

downgrading to M. The fix is a good idea, but this is a pretty rare token implementation and definitely qualifies as an external factor.

[lawrencehui]

yes agreed with @0xean that this is very rare and appreciate warden's suggestion on the fix on balance checking.