Dria

Swan
NFTHardhat
21,000 USDC
View results
Submission Details
Severity: medium
Valid

Lack the check of fee amount in `withdrawPlatformFees`

Vulnerability Description

The vulnerability detected in the provided smart contract function withdrawPlatformFees lies in the sweeping of the total balance of feeToken held by the contract to the owner's address without verifying whether the current balance includes funds that are supposed to be locked or held due to other functional requirements of the system.

Since feeToken.balanceOf(address(this)) returns the entire token balance of the contract at the invoking time, if there are tokens in the contract that are meant for other purposes (like user deposits, staking, or operating reserves that should not be moved), the withdrawPlatformFees function doesn't differentiate and will mistakenly transfer all tokens to the owner. This can potentially result in misuse of funds mistakenly considered as "fees."

https://github.com/Cyfrin/2024-10-swan-dria/blob/c8686b199daadcef3161980022e12b66a5304f8e/contracts/llm/LLMOracleCoordinator.sol#L375

Step-by-Step Exploit

  1. Background Setup: Assume the contract holds multiple token balances:

    • User deposits meant for a different function like staking or future withdrawals.

    • Fee tokens collected from platform transactions that are rightfully the fees to be withdrawn.

  2. Invocation of withdrawPlatformFees:

    • When withdrawPlatformFees is triggered by the owner, it fetches the total balance of tokens (including those that are not fees) by calling feeToken.balanceOf(address(this)).

    • It then issues a transfer command to send all these tokens to owner().

  3. Result of Exploit:

    • Not only are the intended fee tokens transferred, but all user deposits and other tokens crucial for different operations are also incorrectly moved to the owner.

    • Hence, the contract fails to distinguish between different types of tokens, leading to an inappropriate handling of contract-held assets.

Correction Suggestion

To correct the flaw in withdrawPlatformFees, a mechanism should be introduced to differentiate between fee tokens and other tokens stored within the contract:

uint256 feesAvailableForWithdrawal = calculateFeesAvailable(); // Implement this
feeToken.transfer(owner(), feesAvailableForWithdrawal);

In the suggested correction:

  • We introduce a new function calculateFeesAvailable() which specifically calculates the correct amount of fee tokens that are available for the transfer. This function must account for, and exclude, any tokens vested/locked/reserved for other purposes.

  • This would ensure that only the correct number of fee tokens is withdrawn to the owner, securing the integrity of other tokens stored within the same contract.

Updates

Lead Judging Commences

inallhonesty Lead Judge 12 months ago
Submission Judgement Published
Validated
Assigned finding tags:

`withdrawPlatformFees` withdraws the entire balance

Support

FAQs

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