During the analysis of the FeeManager.sol smart contract, a potential security vulnerability was identified in the transfer function. The vulnerability arises from the use of a parameterized from value in the safeTransferFrom function call, which could allow for the unauthorized transfer of tokens from any address.
Vulnerability Detail
The transfer function is designed to handle the transfer of assets, including Ethereum (ETH) and ERC20 tokens. For ERC20 tokens, it uses the safeTransferFrom function, which requires three parameters: the address from which the tokens are transferred (from), the address to which the tokens are transferred (to), and the amount of tokens to transfer (amount). The original implementation of this function uses a parameterized from value, which poses a security risk. An attacker could potentially manipulate the from parameter to transfer tokens from any address, not just the one that currently owns them. This behavior is contrary to the expected functionality of the safeTransferFrom function, which should only allow transfers from the address that is currently executing the transaction (i.e., msg.sender).
Impact
The impact of this vulnerability is significant. It could lead to the theft or loss of tokens, as it allows for unauthorized transfers. This could result in financial losses for token holders and undermine trust in the smart contract and the broader ecosystem. Additionally, it could be exploited to drain funds from wallets or contracts, leading to a potential loss of assets.
To mitigate this vulnerability, it is recommended to modify the _transfer function to use msg.sender as the from parameter in the safeTransferFrom function call. This ensures that tokens can only be transferred from the address that is currently executing the transaction, aligning with the expected behavior of the safeTransferFrom function and preventing unauthorized transfers.
Here is the recommended modification to the _transfer function:
Enc3yptedDegen
high
Arbitrary transferFrom vulnerability
Summary
During the analysis of the FeeManager.sol smart contract, a potential security vulnerability was identified in the transfer function. The vulnerability arises from the use of a parameterized from value in the safeTransferFrom function call, which could allow for the unauthorized transfer of tokens from any address.
Vulnerability Detail
The transfer function is designed to handle the transfer of assets, including Ethereum (ETH) and ERC20 tokens. For ERC20 tokens, it uses the safeTransferFrom function, which requires three parameters: the address from which the tokens are transferred (from), the address to which the tokens are transferred (to), and the amount of tokens to transfer (amount). The original implementation of this function uses a parameterized from value, which poses a security risk. An attacker could potentially manipulate the from parameter to transfer tokens from any address, not just the one that currently owns them. This behavior is contrary to the expected functionality of the safeTransferFrom function, which should only allow transfers from the address that is currently executing the transaction (i.e., msg.sender).
Impact
The impact of this vulnerability is significant. It could lead to the theft or loss of tokens, as it allows for unauthorized transfers. This could result in financial losses for token holders and undermine trust in the smart contract and the broader ecosystem. Additionally, it could be exploited to drain funds from wallets or contracts, leading to a potential loss of assets.
Code Snippet
https://github.com/sherlock-audit/2024-04-titles/blob/main/wallflower-contract-v2/src/fees/FeeManager.sol#L465
Tool used
Manual Review
Recommendation
To mitigate this vulnerability, it is recommended to modify the _transfer function to use msg.sender as the from parameter in the safeTransferFrom function call. This ensures that tokens can only be transferred from the address that is currently executing the transaction, aligning with the expected behavior of the safeTransferFrom function and preventing unauthorized transfers.
Here is the recommended modification to the _transfer function: