RealFevr - bridge

Function `depositERC20()` counts a deposit by a `DEFAULT_ADMIN_ROLE` account as part of the `dailyDeposits`, even though a `DEFAULT_ADMIN_ROLE` account can bypass the check that the `token.max24hDeposits` is not exceeded. This is different from the behavior of minting and burning, where the `DEFAULT_ADMIN_ROLE` account's mints and burns are not included in the daily tracking (unless the `DEFAULT_ADMIN_ROLE` account also has the `BRIDGE` role). As a result, if a `DEFAULT_ADMIN_ROLE` account deposits an amount causing the `token.max24hDeposits` to be exceeded, other users are prevented from depositing for that 24 hour period. Function `withdrawERC20()` checks that `currentWithdrawalAmount + amount` does not exceed `token.max24hWithdraws`, but `dailyWithdraws[tokenAddress][currentDay]` is never updated within the function.

Please specify whether an account given the `BRIDGE` role is expected to be an externally-owned account or a contract. If it is a contract, are actions triggered by user interaction? The current assumption is that an account given this role is a contract that is part of the protocol, but which is out-of-scope for this audit report. There is an out-of-scope dependency on such a contract, since it is assumed that the contract given `BRIDGE` privilege for each contract performs proper validation on the provided input for each function that can be called. Specifically, there is an assumption that the contract given `BRIDGE` privilege performs proper validation on the keys used to permit withdrawing and minting in each bridge. The checks performed on the `uniqueKey` string input within each in-scope bridge contract are not enough to ensure that the string corresponds to the other input information. Moreover, since the minting and withdrawing functions of each bridge use the same input types, there is no prevention of the re-use of a `uniqueKey` across these functions. If a contract given `BRIDGE` privilege does not perform the proper validation on the input before it is provided, then the lack of validation could result in loss of funds through possible replay attacks, invalid user-provided input, or other means. If the account given `BRIDGE` privilege is an externally owned account, then the use of the `uniqueKey` string while lacking proper validation renders the `uniqueKey` ineffective as a mechanic. Namely, the externally owned account calling the minting and withdrawal functions can input any string not previously used in the function as a means to recover any desired tokens from the bridge. There is no validation performed to ensure the uniqueKey provided in some way corresponds to the mint or withdrawal execution, and there is no way to validate that the call corresponds to other actions performed within the bridge.

There are no upper boundaries for the deposit and withdraw token fees, or for the ETH deposit fees, of either bridge contract. It is possible to set the total fee rate up to any arbitrary amount.

Access control roles `OPERATOR` and `BRIDGE` should be initialized inline as constants. Constants are tracked by upgradeable contracts because their values are explicitly recorded in the bytecode.

### ERC20BridgeImpl.sol - In function `depositERC20()`, - a low-level call to `feeReceiver` is performed before updates to state and event emissions - Token transfers and burns are performed before event emission - In function `withdrawERC20()`, token transfers are performed before event emission - In function `mintToken()`, token minting is performed before event emission ### ERC721BridgeImpl.sol - In function `depositSingleERC721()`, - a low-level call to `feeReceiver` is performed before updates to state and event emissions - ERC20 and ERC721 token transfers are performed before updates to state and event emission - In function `depositMultipleERC721()` a low-level call to `feeReceiver` is performed before updates to state and event emission - In function `withdrawSingleERC721()`, ERC721 token transfers are performed before event emission - In function `takeFees()`, ERC20 token transfer is performed before event emission - In function `mintERC721()`, ERC721 token minting and other external calls are performed before event emission

Function `withdrawERC20()` always uses the fee associated with `block.chainid`, so this fee can be expressed as its own `uint` type state variable for the whole contract, rather than using a withdrawal fee corresponding to the chain id and token withdrawn.

When transferring standard ERC20 deflationary tokens, the input amount may not be equal to the received amount due to the charged transaction fee. As a result, an inconsistency in the amount will occur and the transaction may fail due to the validation checks. For example, if a user sends 100 deflationary tokens (with a 10% transaction fee) to the target contract, only 90 tokens actually arrive to the contract.

In `ERC721BridgeImpl.sol`, the reentrancy guard `nonReentrant` is applied to function `depositMultipleERC721()`. Applying the `nonReentrant` modifier to only one of the user-facing functions limits its effectiveness and use as reentrancy protection. This only prevents reentrancy that starts and iterates through function `depositMultipleERC721()` and does not prevent cross-function reentrancy that may start in other functions and reenter through `depositMultipleERC721()`, or start in `depositMultipleERC721()` and reenter through other functions. The following functions remain open to potential cross-function reentrancy combinations: - `depositSingleERC721()` - `withdrawSingleERC721()` - `mintERC721()` Adding a `nonReentrant` modifier to the functions above will increase the effectiveness of the reentrancy guard. --- In `ERC20BridgeImpl.sol`, the reentrancy guard `nonReentrant` is applied to functions `depositERC20()` and `withdrawERC20()`. Applying the `nonReentrant` modifier to only some of the user-facing functions limits its effectiveness and use as reentrancy protection. This only prevents reentrancy that starts and iterates through combinations of functions `depositERC20()` and `withdrawERC20()` and does not prevent cross-function reentrancy that may start in other functions and reenter through these functions, or start from `depositERC20()` or `withdrawERC20()` and reenter through other functions. The following functions remain open to potential cross-function reentrancy combinations: - `mintToken()`

### base_erc20.sol - Function `airdropD()` is missing a check ensuring `usr` and `amounts` are the same length. ### base_erc721.sol - Function `setMarketplaceDistributions()` is missing validation on its input. Without proper documentation of what these values are used for, reasonable validation cannot be recommended. ### ERC721BridgeImpl.sol - Function `depositMultipleERC721()` is missing a check ensuring `nftAddress` and `tokenIds` are the same length. - Function `withdrawMultipleERC721()` is missing a check ensuring `tokenIds` and `uniqueKeys` are the same length. ### ERC20BridgeImpl.sol - Function `depositERC20()` is missing a check ensuring `amount` is nonzero. - Function `withdrawERC20()` is missing a check ensuring `amount` is nonzero.

Functions that update state variables and are privileged should emit relevant events as notifications. Additionally: - event `NFTDetailsSet` emitted in `setNFTDetails()` of `ERC721BridgeImpl` does not detail the update to `nft.feeWithdrawAmount`. - event `ERC20DetailsSet` emitted in `setERC20Details()` of `ERC20BridgeImpl` does not detail the update to `token.burnOnDeposit`.