hats-finance / Fenix--0x9d7765a7ebd5b6322a30797a44a5428531970d3d

0 stars 1 forks source link

incorrect token recovery logic in erc20Recover function #50

Open hats-bug-reporter[bot] opened 2 months ago

hats-bug-reporter[bot] commented 2 months ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0x1295c20753753cc2f9aea1f2ff349d2e2962209d0b0835b8c7f3b05a082d2d30 Severity: medium

Description: Description\ in this function, the admin can recover any ERC20 token sent to the contract, except for the managed token (FENIX). the problem is in the if condition that checks if the token being recovered is the managed token or an allowed token. However, the error IncorrectRecoverToken() is raised after the tokens have already been sent, meaning the tokens are transferred before the error is triggered.

this can potentially result in unintended token transfers if an admin accidentally includes a token that should not be recoverable. Therefore, it's important to validate and carefully review the permissions and conditions for recovering ERC20 tokens to prevent unauthorized token transfers.

  1. Proof of Concept (PoC)
function erc20Recover(address token_, address recipient_) external {
_checkBuybackSwapPermissions();

if (token_ == address(fenix) || IRouterV2PathProvider(routerV2PathProvider).isAllowedTokenInInputRoutes(token_)) {
revert IncorrectRecoverToken();
}

uint256 amount = IERC20(token_).balanceOf(address(this));
IERC20(token_).safeTransfer(recipient_, amount);
emit Erc20Recover(msg.sender, recipient_, token_, amount);
}

CompoundVeFNXManagedNFTStrategyUpgradeable.sol

0xmahdirostami commented 1 month ago

The ERC20 recovery method works well and the if condition is in the right position