VTVL contest - 0xhunter's results

Lines of code

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

Vulnerability details

the purpose of withdrawOtherToken function is to rescue user's funds that was sent accidentally to the contract address . // Withdraw a token which isn't controlled by the vesting contract // Note that the token to be withdrawn can't be the one at "tokenAddress the function uses require(_otherTokenAddress != tokenAddress, "INVALID_TOKEN"); to revert the call if _otherTokenAddress has been set to contract's tokenAddress . because its not safe and logical for a decentralized smart contract to contain a function that be able to drain user's funds. however the requirement check has been deployed incorrectly . missing require check for double entry point tokens. some erc20 tokens have multiple entry points , eg SNX . those entry points belong to the proxy contact and the main one it self , however they share same storage and same user's balance but its possible to transfer and withdraw funds by calling any of them . in this case if tokenAddress of the contract had multiple entry points , owner has the ability to drain user's funds . not using a double entry point token as tokenAddress is not a solution for this vulnerability . For example, Tether USD (USDT) could enable a secondary entry point in the future when upgrading their contract .

recommendation : to fix the issue the requirement check has to be reworked like that:

uint 258 TokenBalanceBefore =  tokenAddress.balanceOf(address(this));
// do the rescue here
uint 258 TokenBalanceafter =  tokenAddress.balanceOf(address(this));
require(stakingTokenBalanceafter  == stakingTokenBalanceBefore );
// its going to revert even if _otherTokenAddress  was set to tokenAddress itself