Foxify - New Deal

The `addKeepers()` function is used to add addresses from the `keepers_` array to the `_keepers` address set. If an address already exists in the set, the `add()` function returns false instead of reverting the transaction. Consequently, the `KeepersAdded` event logs may contain existing addresses, causing inconsistencies between event logs and the actual state of the `_keepers` set. Likewise, If an address does not exist in the set, the `remove()` function returns false instead of reverting the transaction. Consequently, the `KeepersRemoved` event logs may contain those addresses. Other functions mentioned in the "Locations" section suffer from the same issue. ```solidity=141 function addKeepers(address[] memory keepers_) external onlyOwner returns (bool) { for (uint256 i = 0; i < keepers_.length; i++) { address keeper_ = keepers_[i]; require(keeper_ != address(0), "CoreConfiguration: Keeper is zero address"); _keepers.add(keeper_); } emit KeepersAdded(keepers_); return true; } ``` ```solidity=160 function removeKeepers(address[] memory keepers_) external onlyOwner returns (bool) { for (uint256 i = 0; i < keepers_.length; i++) { address keeper_ = keepers_[i]; _keepers.remove(keeper_); } emit KeepersRemoved(keepers_); return true; } ```

When transferring deflationary ERC20 tokens, the input amount may not be equal to the received amount due to the charged transaction fee. For example, if a user sends 100 deflationary tokens (with a 10% transaction fee), only 90 tokens actually arrived to the contract. However, a failure to discount such fees may allow the same user to withdraw 100 tokens from the contract, which causes the contract to lose 10 tokens in such a transaction. Reference: https://thoreum-finance.medium.com/what-exploit-happened-today-for-gocerberus-and-garuda-also-for-lokum-ybear-piggy-caramelswap-3943ee23a39f ```solidity=99 _immutableConfiguration.stable.transferFrom(msg.sender, address(this), amount); ``` - Transferring tokens by `amount`. ```solidity=98 counters_.totalStableAmount += amount; ``` - The `amount` appears to be used for bookkeeping purposes without compensating the potential transfer fees. --- ```solidity=224 if (amount > 0) _immutableConfiguration.stable.transferFrom(msg.sender, address(this), amount); ``` - Transferring tokens by `amount`. ```solidity=223 counters_.totalStableAmount += amount; ``` - The `amount` appears to be used for bookkeeping purposes without compensating the potential transfer fees. --- ```solidity=258 _immutableConfiguration.stable.transferFrom(msg.sender, address(this), amount); ``` - Transferring tokens by `amount`. ```solidity=257 counters_.totalStableAmount += amount; ``` - The `amount` appears to be used for bookkeeping purposes without compensating the potential transfer fees.

The `Core` contract is serving as the underlying entity to interact with the following out-of-scope contracts: - IERC20Stable stable - IFoxifyAffiliation affiliation - IOracleConnector oracle_ - ISwapperConnector swapperConnector The scope of the audit treats out-of-scope entities as black boxes and assumes their functional correctness. However, in the real world, they can be compromised and this may lead to lost or stolen assets. For instance, the project heavily relies on the price feed from the oracle contracts. If the price data from the oracles is manipulated, it could severely impact the tokenomics of the project. Furthermore, within the `autoResolve()` function, the `autoResolveFee` is the return value of `swapperConnector.getAmountIn()`. If this value is large enough, the winner of a position might not receive any rewards.

The `autoResolve()` function in the current codebase, designed to resolve a position when `block.timestamp` surpasses the position's `endTime`, has potential vulnerabilities which could be exploited in certain scenarios. Firstly, the function assigns the current price as the `endPrice` of the position, which might not accurately represent the price at `endTime`. Since the function can only be triggered by a "keeper", if all of the keepers' accounts are compromised, the hacker can delay the invocation of `autoResolve()` to manipulate the `endPrice`. Secondly, if all of the keepers' accounts are compromised, the hacker can even extend the delay beyond the `endTime + CANCELATION_PERIOD`, which leads to the burning of the position when `autoResolve()` is finally called. In this scenario, if the attacker is the creator of the position and is set to lose, they could evade giving the `amountCreator` tokens to the winner. Conversely, if the attacker is the position holder, they can retrieve the `amountAccepter` tokens.