RewardDistributor._claim() Will Always Revert if The Recipient is a Contract

Lines of code Vulnerability details Impact The claim function is called to claim a reward for a given rewardIdentifier. The leaf node is calculated using the hashed index, account and amount values and is verified to belong to the merkle tree. After setting the reward as claimed, the tokens are...

non erc20 token might revert when calling emergencyWithdrawERC20()

Lines of code Vulnerability details Impact when there is an emergency situation, the admin can call emergencyWithdrawERC20 to save the funds, however due to mismatch between non erc20 token like USDT and openzeppelin IERC20 where the open zeppelin IERC20 is expecting a return on the transfer...

Manipulations of setFee

Lines of code Vulnerability details Impact If we consider that the fee variable is meaningfully applied, there will still be several problems with this: 1. Admin can setFee up to 100%. This is bad for users, fees should have a reasonable upper limit, e.g. 30% to prevent potential griefing. 2...

Unchecked feiAmount range

Lines of code Vulnerability details Impact BoosValue could be not safe. Proof of Concept The variable feiAmount is not used during the logic of the method canSafeBoostVault. According to the comment: The amount of Fei asset to check the Safe is authorized boost the Vault with. So this argument mu...

Missing input validation in _claim function at RewardDistributor.sol

Lines of code Vulnerability details Description All the functions present in the contract were found to have input validation except the claim functions. The function at inputs the user's value and then is processed at and is lacking all form of input validation. Impact Users can supply illegal...

Unsafe ERC20 transfers are used in BribeVault and ThecosomataETH

Lines of code Vulnerability details Impact For some ERC20 tokens no revert occurs but false is returned if transfer failed for any reason. If this isn't checked, the system will enter a wrong state with an accounted, but not executed transfer. Such effect can pile up, messing the logic altogether...

Users Can Frontrun Calls to updateRewardsMetadata() And Claim Tokens Twice

Lines of code Vulnerability details Impact The updateRewardsMetadata function is called by the BribeVault contract by the admin role. The function will take a list of distributions which are used to update the associated reward metadata. It is expected that the merkle root will be updated to...

[WP-H0] DEFAULT_ADMIN_ROLE of BribeVault can steal tokens from users' wallets

Lines of code Vulnerability details The current design/implementation allows the DEFAULTADMINROLE of BribeVault to steal funds from any address that approved this contract up to allowance: As a DEFAULTADMINROLE, the attack is simply do the following steps: 1. grantDepositorRole to self; 2...

[WP-H2] Improper control over the versions of distributions' metadata may lead to repeated claims of rewards

Lines of code Vulnerability details function updateRewardsMetadataCommon.Distribution calldata distributions external onlyRoleDEFAULTADMINROLE requiredistributions.length 0, "Invalid distributions"; IRewardDistributordistributor.updateRewardsMetadatadistributions; In the current implementation,...

ThecosomataETH.addLiquidity can be subject to sandwich attack

Lines of code Vulnerability details Impact Liquidity addition can happen at a manipulated pool state and result in receiving fewer LP shares than actual market state dictates Proof of Concept addLiquidity measures slippage based on the pool returned amount via calctokenamount: Pool returned amoun...

Missing briber check is equal to address(this), can cause imbalance between the bribe amount, and the actual balance that the contract has

Lines of code Vulnerability details Impact the depositBribeERC20 is called from https://github.com/code-423n4/2022-02-redacted-cartel/blob/main/contracts/TokemakBribe.sol, however if there is a malicious user that had depositor role, this user can call depositBribeERC20 and set the briber to the...

Slurp misses boost cap checks

Lines of code Vulnerability details Impact Missing checks of canSafeBoostVault in onSafeSlurp function in TurboMaster.solL279. The current vault deposit could already be at BoostCap. Calling slurp on this vault would add interest on the getTotalFeiBoostedForVaultV which will now becomes BoostCap...

authenticate modifier can be bypassed

Lines of code Vulnerability details Impact In TurboRouter.sol the authenticate modifier checks if the msg.sender is equal to the owner function of an arbitrary address provided by the caller. Anyone can simply make a contract of the same type with the owner function returning an address that equa...

[WP-M8] ThecosomataETH should have a min_mint_amount parameter for add_liquidity() set off-chain instead of calculating the min_mint_amount on-chain

Lines of code Vulnerability details // Calculate the min. LP token amount after slippage and attempt to add liquidity function addLiquidityuint256 ethAmount, uint256 btrflyAmount internal uint2562 memory amounts = ethAmount, btrflyAmount; uint256 expectedAmount =...

Tokens are directly transferred from briber to BribeVault

Lines of code Vulnerability details Impact Function depositBribeERC20 in BribeVault transfers tokens directly from the briber: // Since this method is called by a depositor contract, we must transfer from the account // that called the depositor contract - amount must be approved beforehand...

[WP-H4] Input should be validated on-chain to avoid fund loss caused by admin's misinput

Lines of code Vulnerability details In the current design/implementation, the admin of BribeVault is a super privileged role of the system. However, the inputs of the admin to some of the most critical methods are not being validated properly. This can lead to loss of funds to users caused by the...

Rewards can be claimed if merkle proof is known

Lines of code Vulnerability details Impact The README describes the following when a voting ends: Outside of the Hidden Hand contract scope, after the Tokemak CoRE round ends, proposal data is compiled and these two things happen: - The following is derived from the data: its hash KECCAK-256 and...

onSafeSlurp() can be called by anyone on TurboMaster.sol

Lines of code Vulnerability details Impact In TurboMaster.sol the onSafeSlurp function can be called directly by anyone while the logic implies that it should only be called by the slurp function on the TurboSafe.sol contract which performs the required calculations beforehand. When onSafeSlurp i...

User can refer themselves

Lines of code Vulnerability details Impact User can refer themselves when collect in any CollectModule that collect fee. The will lead to value leak as user can always refer themselves to receive a referral fee as discount. Proof of Concept Recommended Mitigation Steps Check if referrer ==...

[WP-H3] Imprecise management of users' allowance allows the admin of the upgradeable proxy contract to rug users

Lines of code Vulnerability details In the current implementation, when there is a fee on follow or collect, users need to approve to the follow modules or collect module contract, and then the Hub contract can call processFollow and transfer funds from an arbitrary address as the follower...

[WP-M1] Inappropriate handling of referralFee makes collecting Mirror fails without error when referrerProfileId is burned

Lines of code Vulnerability details In the current implementation, even when the profile's owner burnt the ProfileNFT, as the profile's legacy, the publications can still be collected. However, if the publication is a Mirror and there is a referralFee set by the original publication, the user won...

It's possible to follow deleted profiles

Lines of code Vulnerability details When someone tries to follow a profile, it checks if the handle exists, and if it doesn't, it reverts because the profile is deleted. The problem is that there might be a new profile with the same handle as the deleted one, allowing following deleted profiles...

missing whenNotPaused

Lines of code Vulnerability details All the external function of LensHub have whenNotPasued modifier. However, LensHub is erc721 and the transfer function doesn't have the whenNotPaused modifier. Impact In case where the governance wants to stop all activity, they still can't stop transferring...

Collect modules can fail on zero amount transfers if treasury fee is set to zero

Lines of code Vulnerability details Impact Treasury fee can be zero, while collect modules do attempt to send it in such a case anyway as there is no check in place. Some ERC20 tokens do not allow zero value transfers, reverting such attempts. This way, a combination of zero treasury fee and such...

Zero collection module can be whitelisted and set to a post, which will then revert all collects and mirrors with PublicationDoesNotExist

Lines of code Vulnerability details Impact In case when zero collection module be white listed and then zero collection module set to a post done by different actors, its functionality will be partially broken: every collecting and mirroring of it will be reverted with...

Basis points constant BPS_MAX is used as minimal fee amount requirement

Lines of code Vulnerability details Impact Base fee modules require minimum fixed fee amount to be at least BPSMAX, which is hard coded to be 10000. This turns out to be a functionality restricting requirement for some currencies. For example, WBTC , 10 in ERC20 token rankings, has decimals of 8...

Reentrancy allows commenter to overwrite own comments

Lines of code Vulnerability details Since the Lens platform is a blockchain-based social media platform, it's important that information relevant to users be emitted so that light clients need not continually refer to the blockchain, which can be expensive. From the docs: Events are emitted at...

no back up action if price oracle fails

Lines of code Vulnerability details Impact In ThecosomataETH.sol the calculateAmountRequiredForLP function makes calculations based on the price fetched from a curvepool oracle. There is no back up action if the price returned is 0 or if the oracle fails. This could result in false calculations i...

Ineffective Whitelist

Lines of code Vulnerability details Creating profiles through LensHub.createProfile requires the caller to be whitelisted. function validateCallerIsWhitelistedProfileCreator internal view if !profileCreatorWhitelistedmsg.sender revert Errors.ProfileCreatorNotWhitelisted; However, a single...

Cashback on referral

Lines of code Vulnerability details Impact In the fee collect modules like FeeCollectModule there is no prevention of someone submitting a second profile they own as the referrerProfileId in processCollect to receive back part of the fees paid. The referral system is essentially broken as all...

Name squatting

Lines of code Vulnerability details Impact Creating profiles through LensHub/PublishingLogic.createProfile does not cost anything and will therefore result in "name squatting". A whitelisted profile creator will create many handles that are in demand, even if they don't need them, just to flip th...

Profile creation can be frontrun

Lines of code Vulnerability details Impact The LensHub/PublishingLogic.createProfile function can be frontrun by other whitelisted profile creators. An attacker can observe pending createProfile transactions and frontrun them, own that handle, and demand ransom from the original transaction...

User can collect publication at lesser fees

Lines of code Vulnerability details Impact User will pay lesser Collect Module fees and the difference will be borne by Publication owner. This way Publication owner will always be on loss Attacker can also make this a business in which he can collect victim nft at discounted price and then sell...

Approvals not cleared when transferring profile

Lines of code Vulnerability details Impact The ApprovalFollowModule.approve function is indexed by both owner = IERC721HUB.ownerOfprofileId, profileId in case the profileId NFT is transferred. However, upon transfer, the old approvals are not cleared. This can lead to similar issues as OpenSea no...

Fee amount too restrictive

Lines of code Vulnerability details Impact In the fee collect modules like FeeCollectModule, the initializePublicationCollectModule checks if the fee amount that each user needs to pay is at least = BPSMAX = 10000. For certain currencies with fewer decimals this is too restrictive. As an example,...

Inconsistent behavior in the *FeeCollectModule contract can cause DOS.

Lines of code Vulnerability details Impact In the FeeCollectModule contract, the recipient of the fee is specified by the user when the post is created, that is, even if the profileNFT is transferred or destroyed, the fee will still be sent to the address specified by the user when the post is...

NestedFactory.addOperator/removeOperator have no effect until importOperators

Lines of code Vulnerability details Impact addOperator/removeOperator being run alone don't have any effect, as the cache is used in operations, which is only updated when an implementation is added or removed via importOperators. If an operation is added via addOperator, but importOperators isn'...

NestedFactory: User can utilise accidentally sent ETH funds via processOutputOrders() / processInputAndOutputOrders()

Lines of code Vulnerability details Impact Should a user accidentally send ETH to the NestedFactory, anyone can utilise it to their own benefit by calling processOutputOrders / processInputAndOutputOrders. This is possible because: 1. receive has no restriction on the sender 2. processOutputOrder...

NestedFactory: User can utilise accidentally sent ETH funds via processOutputOrders() / processInputAndOutputOrders()

Lines of code Vulnerability details Impact Should a user accidentally send ETH to the NestedFactory, anyone can utilise it to their own benefit by calling processOutputOrders / processInputAndOutputOrders. This is possible because: 1. receive has no restriction on the sender 2. processOutputOrder...

NestedFactory does not track operators properly

Lines of code Vulnerability details NestedFactory extends the MixinOperatorResolver contract which comes from the synthetix/MixinResolver.sol code base where the expectation is that isResolverCached returns false until rebuildCache is called and the cache is fully up to date. Due to a medium issu...

Reentrancy

Lines of code Vulnerability details Impact Potential Reentrancy Proof of Concept Reentrancy in ConcurRewardPool.claimRewardsaddress contracts/ConcurRewardPool.sol34-40: External calls: - IERC20tokensi.safeTransfermsg.sender,getting contracts/ConcurRewardPool.sol37 State variables written after th...

Destroy can avoid the bulk of fees

Lines of code Vulnerability details Impact A user can destroy their NFTs and not pay fees on most of their assets. Proof of concept Alice has an NFT portfolio with 100 gwei dai and 100 gwei uni. Alice calls destroy on this NFT with buy token marked as dai. We would expect after this destroy step...

Wrong rebuild cache logic

Lines of code Vulnerability details Impact Currently, many core operations like NestedFactory.create, NestedFactory.swapTokenForTokens are dependent on the assumption that the cache is well synced before these functions are executed however this may not necessarily be the case. Proof of Concept T...

Wrong logic around areOperatorsImported

Lines of code Vulnerability details Impact The logic related to the areOperatorsImported method is incorrect and can cause an operator not to be updated because the owner thinks it is already updated, and a vulnerable or defective one can be used. Proof of Concept The operators mapping is made up...

Duplicate tokens can be added which might cause DOS

Lines of code Vulnerability details Impact Duplicate tokens can be added which could exceed maxHoldingsCount and thus rejecting all functions in Factory Proof of Concept 1. Observe the store function in NestedRecords.solL111 function store uint256 nftId, address token, uint256 amount, address...

Undesired behavior

Lines of code Vulnerability details You push a parameter into an array of tokens without checking if it's already exists. And if at first it's added with amount 0 it can later on be pushed with a greater amount and be twice in the array. Then in all processing it will consider the first occurrenc...

Shelter funds can be stolen

Lines of code Vulnerability details Impact Shelter has a function withdraw that lets whitelisted users withdraw a specified amount of some token. The function does not check if the user has already withdrew the tokens. Since, a user can withdraw allowed amount any number of times, stealing all th...

All Tokens Can Be Stolen From Shelter Contract

Lines of code Vulnerability details Impact function withdrawIERC20 token, address to external override requireactivatedtoken != 0 && activatedtoken + GRACEPERIOD block.timestamp, "shelter not activated"; uint256 amount = savedTokenstoken client.shareOftoken, msg.sender / client.totalSharetoken;...

Shelter sets wrong claimed field

Lines of code Vulnerability details The Sheler.withdraw function sets the claimedtokenuser field but uses the shares of msg.sender. An attacker can withdraw tokens several times passing different to addresses, each time the msg.sender's shares will be used to receive tokens. function withdrawIERC...

Shelter withdraw does not check if user already claimed

Lines of code Vulnerability details Impact Shelter withdraw does not check if user already claimed. This allow any user with non-zero claim to drain the Shelter. Proof of Concept function withdrawIERC20 token, address to external override requireactivatedtoken != 0 && activatedtoken + GRACEPERIOD...