Haven1 - audit hSwap

In the contract `UniswapV3Factory` the role owner has authority over the following functions: - function `setOwner()` - function `setRouter` - function `enableFeeAmount()` Any compromise to the owner account may allow a hacker to take advantage of this authority and modify default fee settings for swap pools or modify the whitelisted router address list. ----------------------------------------------------------------------- In the contract `UniswapV3Pool` the role `FactoryOwner` has authority over the following functions: - function `setFeeProtocol()` - function `collectProtocol()` Any compromise to the factory owner account may allow a hacker to take advantage of this authority and modify protocol fee or collect protocol fee. ----------------------------------------------------------------------- In the contract `H1NativeApplication` the role `association` has authority over the following functions: - function `_authorizeFeeContractAddressUpdate()` Any compromise to the `association` account may allow a hacker to take advantage of this authority and modify fee contract addresses through the function `updateFeeContractAddress`. ----------------------------------------------------------------------- In the contract `H1NativeApplication` the role `association` has authority over the following functions: - function `updateSwapRouter()` - function `updateQuoter()` - function `addBaseToken()` - function `removeBaseToken()` Any compromise to the `association` account may allow a hacker to take advantage of this authority and change Router/Quoter address and modify predefined basetoken lists when performing the on-chain swap routing.

A reentrancy attack can occur when the contract creates a function that makes an external call to another untrusted contract before resolving any effects. The contract pays fees to `_feeContract` via a low-level call `(call{value: amount})` before finalizing state updates. Since the `_feeContract` can be modified by the `association` account, if the attacker can control the `_feeContract` address, a malicious fallback function or reentry call could invoke the original contract logic again. This allows the attacker to trigger functions and manipulate state that would otherwise only run once. ```solidity=100 modifier applicationFee(bool payableFunction, bool refundRemainingBalance) { _updateFee(); uint256 fee = _feeContract().getFeeForContract( address(this), msg.sender, msg.sig ); if (msg.value < fee || (address(this).balance < fee)) { revert H1NativeBase__InsufficientFunds(address(this).balance, fee); } H1NativeBaseStorage storage $ = _getH1NativeStorage(); if (payableFunction) $.msgValueAfterFee = (msg.value - fee); _payFee(fee); _; if (refundRemainingBalance && address(this).balance > 0) { _safeTransfer(msg.sender, address(this).balance); } delete $.msgValueAfterFee; } ```

The `positions()` function is intended to expose details about a specific position. While the function successfully returns most of the details, it omits the `collectBlockNumber` field that is available in the Position struct. ```solidity=185 function positions( uint256 tokenId ) external view override returns ( uint96 nonce, address operator, address token0, address token1, uint24 fee, int24 tickLower, int24 tickUpper, uint128 liquidity, uint256 feeGrowthInside0LastX128, uint256 feeGrowthInside1LastX128, uint128 tokensOwed0, uint128 tokensOwed1 ) ```

`safeApprove()` has been [Deprecated](https://github.com/OpenZeppelin/openzeppelin-contracts/blob/bfff03c0d2a59bcd8e2ead1da9aed9edf0080d05/contracts/token/ERC20/utils/SafeERC20.sol#L38-L45) in favor of `safeIncreaseAllowance()` and `safeDecreaseAllowance()`. If only setting the initial allowance to the value that means infinite, `safeIncreaseAllowance()` can be used instead. The function may currently work, but if a bug is found in this version of OpenZeppelin, and the version that you're forced to upgrade to no longer has this function, you'll encounter unnecessary delays in porting and testing replacement contracts.

The `safeTransfer` function does not check for the existence of the ERC20 token contract , `TransferHelper.sol` performs a transfer with a low-level call without confirming the contract's existence ```solidity function safeTransfer( address token, address to, uint256 value ) internal { (bool success, bytes memory data) = token.call(abi.encodeWithSelector(IERC20Minimal.transfer.selector, to, value)); require(success && (data.length == 0 || abi.decode(data, (bool))), 'TF'); } ``` The low-level functions call, delegatecall and staticcall return true as their first return value if the account called is non-existent, as part of the design of the EVM. Account existence must be checked prior to calling if needed.

If an event is not indexed in a smart contract, it means that the event's parameters are not tagged with the `indexed` keyword. This has implications for how the event data can be searched and filtered when looking through blockchain logs. Without indexing, the event will still emit the data as part of the transaction log, but users won't be able to query for these events using the parameters. They'll have to retrieve the entire set of logs and manually sift through them to find events with the specific data. This can be less efficient and more time-consuming, especially on a blockchain with a high volume of transactions and events.

The `H1NativeBase` contract is serving as the underlying entity to interact with third-party `_feeContract` address to get the application fee amount. The scope of the audit treats third-party entities as black boxes and assumes their functional correctness. However, in the real world, third parties can be compromised and external call to a malicious contract may lead to lost or stolen assets. ```solidity=100 modifier applicationFee(bool payableFunction, bool refundRemainingBalance) { _updateFee(); uint256 fee = _feeContract().getFeeForContract( address(this), msg.sender, msg.sig ); ```