Reentrancy in depositBribe in TokemakBribe.sol

Lines of code Vulnerability details Description The contract was found vulnerable to Reentrancy attack. It was noticed that the function depositBribe makes an external call to another untrusted address or a contract before it resolves any effects at line "" If the attacker controls the untrusted...

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...

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...

Arbitrary fees

Lines of code Vulnerability details Impact Function transferBribes has a parameter named fees. Essentially, it allows an admin to apply any arbitrary fees and send them to the feeRecipient. This makes no sense because admins can manipulate the amounts however they want with 0 restrictions. I see...

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...

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 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...

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...

RewardDistributor._claim uses native token payable.transfer, which is usafe for smart contracts

Lines of code Vulnerability details Impact When reward.token is set to vault address and native token is used, it is sent out via payable.transfer call. This is unsafe as transfer has hard coded gas budget and can fail when the account is a smart contract. Such transactions will fail for smart...

Non-Standard ERC20 Tokens Are Not Supported

Lines of code Vulnerability details Impact There are several contracts which do not utilise OpenZeppelin's SafeERC20 library when performing token transfers. The Redacted Cartel suite of smart contracts intends to support bribes from all tokens, including non-standard tokens such as USDT which do...

[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,...

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...

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...

DEPOSITOR_ROLE can manipulate b.amount value

Lines of code Vulnerability details Impact Malicious DEPOSITORROLE can doing self transfer and manipulate b.amount Proof of Concept In case malicious DEPOSITORROLE inputing WETH address and putting briber == addressthis in safeTransferFrom argument which is self transfering. Therefore, it is...

Reentrancy in depositBribeERC20 function

Lines of code Vulnerability details Impact depositBribeERC20 function in BriveVault is reentrant in line 187, where an address supplied by the caller is called. A bad actor that has DEPOSITORROLE and is a contract can execute a folowing attack: 1. Create a dummy token contract, reentrant in the...

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...

[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...

canSafeBoostVault() doesn't actually check the feiAmount passed in

Lines of code Vulnerability details Impact In TurboMaster.sol the onSafeBoost function calls canSafeBoostVault on the booster contract to check if the safe is allowed to boost the vault using the feiAmount which is passed in by the caller as an argument. The problem is that the feiAmount is never...

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 ==...

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...

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...

[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...

[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...

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...

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...

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...

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...

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...

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...

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...

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...

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...

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,...

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...

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...

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 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...

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.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...

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 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...

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...

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...

Users’ staking rewards can get redeemed by other users

Lines of code Vulnerability details Can be medium or high: When a user deposits through ConvexStakingWrapper, the contract calls the MasterChef's deposit function and deposits to it with recipient = msg.sender, but the MasterChef actually updates the deposited amount of the ConvexStakingWrapper...

[WP-H5] ConvexStakingWrapper.sol#withdraw() Use of non-existing method on convexPool[_pid] makes withdraw() always reverts

Lines of code Vulnerability details function withdrawuint256 pid, uint256 amount external nonReentrant WithdrawRequest memory request = withdrawRequestpidmsg.sender; requirerequest.epoch = amount, "too much"; checkpointpid, msg.sender; depositspidmsg.sender.amount -= uint192amount; if amount 0...

Wrong pools reward calculation. User will get smaller rewards (always)

Lines of code Vulnerability details Impact When adding new token pool for staking in MasterChef contract function addaddress token, uint allocationPoints, uint16 depositFee, uint startBlock All other, already added, pools should be updated but currently they are not. Instead, only totalPoints is...

Re-entrancy vulnerabilities

Lines of code Vulnerability details Impact Function claimRewards in ConcurRewardPool should be re-entrancy protected or first nullify the reward before sending it, otherwise, if any token contains a transfer callback hook, users can claim the same rewards multiple times, by re-entering the functi...