Vyper Vested Claims
First Flight #34
Beginner FriendlyDeFi
100 EXP
View results
Previous Next
Submission Details
Severity: low
Invalid

Unrestricted Token Rescue: Risk of Total Fund Drainage

betrue

Summary

Function for rescuing tokens in an emergency has no restrictions, opening possibilities for malicious acts or accidents that can drain all tokens, including unclaimed vested amounts.

Vulnerability Details

In the VestedAirdrop::rescue_tokens function, the owner can move tokens out of the contract at any time and at any amount with no restrictions:

@external
def rescue_tokens(to: address, amount: uint256):
"""
@notice This function is used to rescue tokens from the contract
@param to address, the address to send the tokens to
@param amount uint256, the amount of tokens to send
@dev this is a "better safe then sorry" function, use it only in case of emergency
@dev This function can only be called by the owner
"""
self.onlyOwner()
log TokensRescued(to, amount)
_success: bool = extcall IERC20(self.token).transfer(to, amount)
assert _success, "Transfer failed"

This implementation lacks any checks and governance control for restricting its functionality.

Impact

Any amount of tokens can be moved out of the contract at any time by the owner. This could put all tokens at risk if owner acts maliciously or their private information is compromised. Requires complete trust in the owner, undermining the trustless nature of blockchain contracts.

Tools Used

Manual inspection, copilot and moccasin.

Recommended Mitigation

Alternative implementations where allowing only excess tokens to be rescued, enabling a timelock mechanism or disabling rescue after claims start would provide stronger protection while still allowing legitimate rescues of the excess tokens, e.g.:

@external
def rescue_tokens(to: address, amount: uint256):
"""
@notice This function is used to rescue tokens from the contract
@param to address, the address to send the tokens to
@param amount uint256, the amount of tokens to send
@dev this is a "better safe then sorry" function, use it only in case of emergency
@dev This function can only be called by the owner
"""
self.onlyOwner()
​
# Ensure the vesting period has ended
assert block.timestamp > self.vesting_end_time, "Cannot rescue before vesting ends"
​
# Get contract's current token balance
balance: uint256 = IERC20(self.token).balanceOf(self)
​
# Calculate excess tokens
reserved: uint256 = self.get_reserved_tokens() # implement a function to track this
excess: uint256 = 0
​
if balance > reserved:
excess = balance - reserved
​
# Only allow rescuing up to the excess amount
assert amount <= excess, "Cannot withdraw reserved tokens"
​
log TokensRescued(to, amount)
_success: bool = extcall IERC20(self.token).transfer(to, amount)
assert _success, "Transfer failed"
Updates

Appeal created

bube Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

[Invalid] Owner can call rescue_tokens and withdraw users tokens

The `owner` is trusted and the function `rescue_tokens` can be called only by the owner and only in case of emergency. This means the owner will not act maliciously and will not call the function without need. Also, issues realated to the malicious admin actions are invalid according to the CodeHawks documentation: https://support.cyfrin.io/en/articles/10059196-findings-validity

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.