VTVL contest - wuwe1's results

Lines of code

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

Vulnerability details

Proof of Concept

function withdrawOtherToken(IERC20 _otherTokenAddress) external onlyAdmin {
    require(_otherTokenAddress != tokenAddress, "INVALID_TOKEN"); // tokenAddress address is already sure to be nonzero due to constructor
    uint256 bal = _otherTokenAddress.balanceOf(address(this));
    require(bal > 0, "INSUFFICIENT_BALANCE");
    _otherTokenAddress.safeTransfer(_msgSender(), bal);
}

tokenAddress can be a different address but point to the same storage. In this case, admin can by pass the check and drain underlying token.

https://blog.openzeppelin.com/compound-tusd-integration-issue-retrospective/

Recommended Mitigation Steps

add check on token balance

diff --git a/contracts/VTVLVesting.sol b/contracts/VTVLVesting.sol
index c564c6f..2b9c606 100644
--- a/contracts/VTVLVesting.sol
+++ b/contracts/VTVLVesting.sol
@@ -445,8 +445,10 @@ contract VTVLVesting is Context, AccessProtected {
      */
     function withdrawOtherToken(IERC20 _otherTokenAddress) external onlyAdmin {
         require(_otherTokenAddress != tokenAddress, "INVALID_TOKEN"); // tokenAddress address is already sure to be nonzero due to constructor
+        uint256 balBefore = IERC20(tokenAddress).balanceOf(address(this));
         uint256 bal = _otherTokenAddress.balanceOf(address(this));
         require(bal > 0, "INSUFFICIENT_BALANCE");
         _otherTokenAddress.safeTransfer(_msgSender(), bal);
+        require(IERC20(tokenAddress).balanceOf(address(this)) == balBefore);
     }
 }