unifarm - Bridge

getBackTokens() can be exploited by malicious admin to drain the liquidity from any liquidity pool.

tokenBridgeRegistryUpgradeable and bridgeUpgradeable can be updated anytime by the owner using updateTokenBridgeRegistryAddress() and updateBridgeUpgradeableAddress(). If an LP has fees accrued say 100 USD in the contract, but the BridgeUpgradeable address is updated using updateBridgeUpgradeableAddress() function, then the LP will never be able to claim this fees- leading to a Denial of Service. In the BridgeUtilsUpgradeable contract, there is a possibility of mismatch between the feePoolUpgradeable and the most current FeePoolUpgradeable contract in usage as well as bridgeUpgradeable and the most current BridgeUpgradeable in usage, which is same as the issue stated in BridgeUpgradeable contracts section above in the report.

There is insufficient test coverage provided for the codebase. For some situations it is possible that router protocol should revert or do expected things which must be checked while in testing on mainnet.For example, It is possible that routerSend() returns true but the transaction fails to go on the destination chain. If it passes and the dest chain tx could not be executed due to low gas price and limit, then replay transaction needs to be done.

getUserConfirmedRewards() shows zero rewards even when there are non-zero rewards claimable by liquidity providers. This is because the fee calculation and distribution depends on addPassedEpochs() that gets updated only when claimFeeShare, addLiquidity, etc. is called. This means that Liquidity Providers don't have a foolproof way to check what their current rewards are using getUserConfirmedRewards() or even getUserUnconfirmedRewards() unless someone calls claimFeeShare or other state changing functions which call addPassedEpochs() internally.

BridgeUpgradeableUtils contract inherits BridgeStorage contract which is not required to be used anywhere.

safeWithdrawLiquidity is a crucial function which does not emit any event.

Missing events for critical functions like- addTokenMetadata(), _deploySetuToken(), updateFeeConfig(), updateEpochLength(), disableBridgeToken(), enableBridgeToken() and updateNoOfDepositors()

The function createCrossChainTransferMapping() of the BridgeUpgradeable contract which is critical for cross chain transfers, relies highly Router protocol. There is a possibility that the _transferIndex parameter is set acccordingly to pass the require check every time in createCrossChainTransferMapping(). Also there can be a possibility of transactions getting manipulated or denied of getting executed crosschain if Router protocol is centralized or gets compromised.

In the BridgeUtilsUpgradeable contract, there is a possibility of mismatch between the feePoolUpgradeable and the most current FeePoolUpgradeable contract in usage as well as bridgeUpgradeable and the most current BridgeUpgradeable in usage. This is because the owner of the contract can forget to change the addresses of the bridge and the feepool contract after they have been upgraded. A malicious admin can also take an advantage of this to hide the actual contract with some malicious code.

It is possible that if the epochLength is set as too small, epochs will get added faster in the Epoch array of the epochs mapping of the BridgeUpgradeable contract. If this happens, this could to out of gas errors when claimFeeShare() and withdrawAdminTokenFees() because the for loop would be looping over a large array length.