Users' funds could be stolen or locked by malicious or rouge owners
Users' funds could be stolen or locked by malicious or rouge owners.\\nIn the contest's README, the following was mentioned.\\nQ: Is the admin/owner of the protocol/contracts TRUSTED or RESTRICTED?\\nrestricted. the owner should not be able to steal funds.\\nIt was understood that the owner is not "trusted" and should not be able to steal funds. Thus, it is fair to assume that the sponsor is keen to know if there are vulnerabilities that could allow the owner to steal funds or, to a lesser extent, lock the user's funds.\\nMany control measures are implemented within the protocol to prevent the owner from stealing or locking the user's funds.\\nHowever, based on the review of the codebase, there are still some "loopholes" that the owner can exploit to steal funds or indirectly cause losses to the users. Following is a list of methods/tricks to do so.\\nMethod 1 - Use the vulnerable `withdrawNative` function\\nOnce the user's order is fulfilled, the swapped ETH/WETH will be sent to the contract awaiting the user's claim. However, the owner can call the `withdrawNative` function, which will forward all the Native ETH and Wrapped ETH in the contract to the owner's address due to another bug ("Lack of segregation between users' assets and collected fees resulting in loss of funds for the users") that I highlighted in another of my report.\\nMethod 2 - Add a malicious custom price feed\\n```\\nFile: LimitOrderRegistry.sol\\n    function setFastGasFeed(address feed) external onlyOwner {\\n        fastGasFeed = feed;\\n    }\\n```\\n\\nThe owner can create a malicious price feed contract and configure the `LimitOrderRegistry` to use it by calling the `setFastGasFeed` function.\\n```\\nFile: LimitOrderRegistry.sol\\n    function performUpkeep(bytes calldata performData) external {\\n        (UniswapV3Pool pool, bool walkDirection, uint256 deadline) = abi.decode(\\n            performData,\\n            (UniswapV3Pool, bool, uint256)\\n        );\\n\\n        if (address(poolToData[pool].token0) == address(0)) revert LimitOrderRegistry__PoolNotSetup(address(pool));\\n\\n        PoolData storage data = poolToData[pool];\\n\\n        // Estimate gas cost.\\n        uint256 estimatedFee = uint256(upkeepGasLimit * getGasPrice());\\n```\\n\\nWhen fulfilling an order, the `getGasPrice()` function will fetch the gas price from the malicious price feed that will report an extremely high price (e.g., 100000 ETH), causing the `estimatedFee` to be extremely high. When users attempt to claim the order, they will be forced to pay an outrageous fee, which the users cannot afford to do so. Thus, the users have to forfeit their orders, and they will lose their swapped tokens.
Consider implementing the following measures to reduce the risk of malicious/rouge owners from stealing or locking the user's funds.\\nTo mitigate the issue caused by the vulnerable `withdrawNative` function. Refer to my recommendation in my report titled "Lack of segregation between users' assets and collected fees resulting in loss of funds for the users".\\nTo mitigate the issue of the owner adding a malicious custom price feed, consider performing some sanity checks against the value returned from the price feed. For instance, it should not be larger than the `MAX_GAS_PRICE` constant. If it is larger than `MAX_GAS_PRICE` constant, fallback to the user-defined gas feed, which is constrained to be less than `MAX_GAS_PRICE`.
Users' funds could be stolen or locked by malicious or rouge owners.
```\\nFile: LimitOrderRegistry.sol\\n    function setFastGasFeed(address feed) external onlyOwner {\\n        fastGasFeed = feed;\\n    }\\n```\\n