UniFarm security record

There is possibility of a profitable reentrancy attack in removeLiquidity().  This is because a low level call is used in transferLPFee() function which allows the caller to call any malicious code from another contract. Using this multiple rewards for same liquidity position can be harvested.For example, Let’s say the 1st liquidity position by an LP is 1000 USDC. After that a user does cross-chain transfer. Now the LP sees that he has some rewards available to claim.  Then he adds a 2nd position of say 5000 USDC tokens. Now when the user does a reentrant call via low level call in transferLpFee, he will be able to get twice the reward for the same liquidity position. This is possible because although the index of the first liquidity position will be used for the reentrant call, 2000 or more USDC tokens will need to be withdrawn for claiming more rewards which will be taken from the LP’s second liquidity position directly. The rest of the liquidity he will still be able to claim via transferOut.

The report gives no description here.

The report gives no description here.

The report gives no description here.

A malicious owner can deploy a dummy or fake setu token and overwrite the existing metadata of already added setu token using addTokenMetadata using addTokenMetadata() function. For example, let's say setuUSDC was deployed using addBridge() for USDC token, then a malicious owner can overwrite the bridgeTokenMetadata mapping for the USDC setuToken. This is possible if the owner had deployed an ERC20 token with symbol as setuUSDC and the address of this token is passed as a parameter to the addTokenMetadata() function. This attack can be coupled with A.1 to amplify this attack. In addition to this, overwriting to bridgeTokenMetadata can also be done via _deploySetuToken if a setuToken with the same ticker and symbol is deployed again by an owner.

burnTokens() function can be called by the owner to burn tokens from any account, even from the accounts that it currently does not own. Here the owner is a bridge contract, so this intended design is fine. But this can be exploited by a malicious admin if he calls renounceOwnership() and transfer ownership of the contract to himself. Also lockedTokens() function can be called by the owner to burn tokens from any account, even from the accounts that it currently does not own. Also a malicious owner can mint more setuTokens and use it to drain liquidity from the Liquidity Pools by calling the transferOut() function.

safeWithdrawLiquidity() can be used by a malicious owner to withdraw all the liquidity of any token.

updateFeeConfig() allows updating of tokenBridge's fee at any time. Also the _feeInBips parameter has no upper limit and can be exploited by a malicious admin to set very high fees without anyone noticing or taking action. Also _feeInBips parameter lacks zero value check as well which can lead to unintended issues such as no fees getting paid by users. updateEpochLength() allows to update the epochLength of bridge anytime which can lead to unintended issues and bugs.

Although the childToken is an upgradeableToken, it can never be upgraded by the owner of the Registry Contract. It can only be upgradedby the proxyAdmin. The Registry contract is the owner of the proxyAdmin, not the owner of the Registry contract.

If a user buys booster packs using buyBoosterPacks() and then add liquidity using addLiquidity(), then the user will lose his currently bought boosters due to overwriting of boosterEndEpochIndex for that particular liquidity position. For example, let's say user buys 1000 booster packs for token USDC, with _index parameter specified as 1 (for which he has not added any liquidity yet and let's say currentEpochIndex is 50). This will set boosterEndEpochIndex to 1049. Now at epoch say 60, and the user adds liquidity using addLiquidity() with _noOfBoosters as zero, the boosterEndEpochIndex will get overwritten to 60 resulting in loss of boosters bought.