Missing burn function in _unlock function

Handle Tomio Vulnerability details Impact In the XDEFIDistribution.sol, a user can lock xdefi token and get an erc721/nft token through safeMint function, however when the user call unlock function this contract will delete the position of this user, but didnt call the burn function in ERC721...

Reentrancy at lock can lead to wrong accounting

Handle kenzo Vulnerability details The lock function calls safeMint in the midst of it. This can lead to reentry to updateDistribution, thereby ruining contract invariants. Impact At worst case, loss of user funds: distributableXDEFI will be set to be bigger than it really is, and so when a...

VaderPoolV2 owner can steal all user assets which are approved VaderPoolV2

Handle TomFrenchBlockchain Vulnerability details Impact Possible theft of all user assets with an ERC20 approval on VaderPoolV2 Proof of Concept The owner of VaderPoolV2 can call the setTokenSupport function which allows the caller to supply any address from which to take the assets to provide th...

Pool Manager can frontrun fees to 100% and use it to steal the value from users

Handle pedroais Vulnerability details Impact Pool Manager can front-run entry fee to 100% and users could lose all their deposits Proof of Concept Considering : The pool manager is the creator of the pool Anyone can create a pool Manager is not a trusted actor Anyone can create a pool and get...

Return variable can remain unassigned in _sendForReceiver

Handle sirhashalot Vulnerability details Impact The sendForReceiver function only sets a return function in the "if" code block, not the "else" case. If the "else" case is true, no value is returned. The result of this oversight is that the sendForReceiver function called from the distribute...

Unsafe token transfer

Handle WatchPug Vulnerability details Calling ERC20.transfer without handling the returned value is unsafe. outputToken.transfermsg.sender, outputTokenBalance; Recommendation Consider using OpenZeppelin's SafeERC20 library with safe versions of transfer functions. --- The text was updated...

Improper design/implementation of SingleTokenJoinV2#joinTokenSingle() make it prone to fail

Handle WatchPug Vulnerability details for uint256 i; i bs.tokens.length; i++ IERC20 token = bs.tokensi; uint256 tokenAmount = balanceaddresstoken.mulamount.addfeeAmount.div totalSupply ; requiretokenAmount != 0, "AMOUNTTOOSMALL"; token.safeTransferFrommsg.sender, addressthis, tokenAmount; // If...

Anyone can liquidate credit line when autoLiquidation is false without supplying borrow tokens

Handle harleythedog Vulnerability details Impact It is intended that if a credit line has autoLiquidation as false, then only the lender can be the liquidator see docs here: . However, this is not correctly implemented, and anyone can liquidate a position that has autoLiquidation set to false. Ev...

Attacker can empty all the funds by creating fake promotions

Handle WatchPug Vulnerability details The current implementation of calculateRewardAmount allows a arbitrary epochId, which can even be a epochId numberOfEpochs. A malicious user can call claimRewards with epochIds larger than numberOfEpochs and claim other users' rewards. Furthermore, since...

Non compatile with all tokens

Handle 0x1f8b Vulnerability details Impact It could not work with all tokens. Proof of Concept The method fromTokenAmount inside the contract Token18 can't work with tokens with 0 decimals, the result of UFixed18Lib.ratio will throw and error because it will divide by zero. It's mandatory to chec...

Change in auctionMultiplier/auctionDecrement change profitability of auctions and factory can steal all tokens from a basket abusing it

Handle 0x0x0x Vulnerability details When factory changes auctionMultiplier or auctionDecrement profitability of bonded auctions change. There is no protection against this behaviour. Furthermore, factory owners can decide to get all tokens from baskets where they are bonded for the auction. Proof...

Unlimited allowances let anyone move funds to child vaults

Handle 0x0x0x Vulnerability details A malicious attacker can move someones funds between their vaults, since max allowances are given. As a consequence, a malicious user can block activities such as reclaimTokens. Although, funds are not directly stolen, it is possible to revert transactions of...

Improper implementation of arbitraryCall() allows protocol gov to steal funds from users' wallets

Handle WatchPug Vulnerability details function arbitraryCalladdress who, bytes memory data public lock externallyGoverned // cannot have an active incentive for the callee requireincentiveswho == 0, "inc"; ... When an incentiveToken is claimed after endStream, incentiveswho will be 0 for that...

This protocol doesn't support all fee on transfer tokens

Handle 0x0x0x Vulnerability details Some fee on transfer tokens, do not reduce the fee directly from the transferred amount, but subtracts it from remaining balance of sender. Some tokens prefer this approach, to make the amount received by the recipient an exact amount. Therefore, after funds ar...

Broken logic if rewardToken == depositToken

Handle gzeon Vulnerability details Impact There doesn't seems to be anything to prevent one the deploy a Stream with rewardToken == depositToken. If rewardToken == depositToken, some logic might be broken. Proof of Concept For example, 1. recoverTokens logic would be broken because it does not...

Timelock delay can be changed by without any delay

Handle 0x0x0x Vulnerability details Impact Delay can be changed without any delay. Therefore, it is possible to call functions from this contract in a single block by changing the delay. This creates a huge attack vector, since if governor private keys would be stolen, everything can be withdrawn...

MovingAverage.setSampleMemory() may broke MovingAverage, making the value of exchangeRate in StabilizerNode.stabilize() being extremely wrong

Handle WatchPug Vulnerability details function setSampleMemoryuint256 sampleMemory external onlyRoleADMINROLE, "Must have admin privs" requiresampleMemory 0, "Cannot have sample memroy of 0"; if sampleMemory sampleMemory for uint i = sampleMemory; i sampleMemory; i++ samples.push; counter = count...

Excessive privilege for Timelock governor

Handle gzeon Vulnerability details Impact Timelock governor can change delay and gracePeriod at will, which render the timelock useless. function setDelayuint256 delay public onlyRoleGOVERNORROLE, "Must have timelock role" function setGracePerioduint256 gracePeriod public onlyRoleGOVERNORROLE,...

getAuctionCore function returns wrong values out of order

Handle jayjonah8 Vulnerability details Impact In the AuctionEscapeHatch.sol file both earlyExitReturn and calculateMaltRequiredForExit call the getAuctionCore function which has 10 possible return values most of which are not used. It gets the wrong value back for the "active" variable since it's...

Frontrunning PublicLock.initialize() can prevent upgrades due to insufficient access control

Handle elprofesor Vulnerability details Impact The unlock protocols base contract Unlock.sol uses setLocktemplate to initialize the implementation contract for the PublicLock proxy. This function will initialize the relevant PublicLock contract which has been deployed separately...

Malicious user can get infinite free trial by repeatedly refund and repurchase right before the freeTrial ends

Handle WatchPug Vulnerability details The current design/implementation allows users who are refunded before to get another freeTrial. This can be exploited by malicious users to get an infinite free trial. PoC Given: keyPrice is 1 ETH; freeTrialLength is 31 days. A malicious user can: 1. Call...

MixinPurchase:shareKey allows to generate keys without purchasing

Handle GiveMeTestEther Vulnerability details Impact the shareKey function allows a user to share some time with another user that doesn't already has/had a key and this generates a new key. This even allows to generate more keys than maxNumberOfKeys. attacker generates a lot of EOA adresses, buys...

keyPrice is global and change on keyPrice can damage lockManager or keyOwner

Handle 0x0x0x Vulnerability details Impact A lock manager changes keyPrice. If the price is increased, then possible refunds are also increased and it can result in theft of funds. If the price is reduced, by doing so lock manager can reduce the refund payments and scam the users. Tools Used Manu...

Key transfer will destroy key if from==to

Handle kenzo Vulnerability details If calling transferFrom with from == recipient, the key will get destroyed meaning the key will be set as expired and set the owner's key to be 0. Impact A key manager or approved might accidently destroy user's token. Note: this requires user error and so I'm n...

Missing Emergency Pause Check

Handle defsec Vulnerability details Impact During the manual code review, It has been observed that minting progress is not checked when the contract is emergency paused. This can cause misfunctionality and unlocking user funds during the emergency pausing. Proof of Concept 1- Navigate to ""...

setReserve() can be front-run

Handle palina Vulnerability details Impact The reserve address variable in NestedFactory.sol remains equal to 0 before the setReserve function is called by an owner. This may lead to incorrect transfers of tokens or invalid comparison with e.g., the asset reserve nestedRecords.getAssetReservenftI...

Mixing different types of LP shares can lead to losses for Synth holders

Handle hyh Vulnerability details Impact Users that mint Synths do not get pool shares, so exiting of normal LP can lead to their losses as no funds can be left for retrieval. Proof of Concept 3 types of mint/burn: NFT, Fungible and Synths. Synths are most vilnerable as they do not have share: LP...

Unbounded loops

Handle pauliax Vulnerability details Impact There are several loops in the contract which can eventually grow so large as to make future operations of the contract cost too much gas to fit in a block. Specifically, in contract TwapOracle there is no upper boundary on how many pairs can be...

IbbtcVaultZap.sol#setGuardian doesn't set the guardian, but governance

Handle 0x0x0x Vulnerability details Proof of Concept This function is intended to change the guardian, but the code changes the governance address as seen in the code: function setGuardianaddress guardian external onlyGovernance; governance = guardian; It should be replaced with the following cod...

The first lp provider can destroy the pool

Handle jonah1005 Vulnerability details Impact First lp provider received liquidity amount same as the nativeDeposit amount and decides the rate. If the first lp sets the pool's rate to an extreme value no one can deposit to the pool afterward. please refer to the proof of concept section A...

Multicall.sol batch calls don't update msg.value

Handle jayjonah8 Vulnerability details Impact In NestedFactory.sol using Multicall.sol can be dangerous when it has a msg.value inside a loop since the msg.value doesn't update every iteration. This can lead to a user sending ETH one time and it being counted for every iteration. There is a...

Can not update target price

Handle jonah1005 Vulnerability details Impact The sanity checks in rampTargetPrice are broken SwapUtils.solL1571-L1581 if futureTargetPricePrecise = initialTargetPricePrecise, "futureTargetPrice is too small" ; else require futureTargetPricePrecise =...

WrappedIbbtc and WrappedIbbtcEth contracts do not filter out price feed outliers

Handle hyh Vulnerability details Impact If price feed is manipulated in any way or there is any malfunction based volatility on the market, both contracts will pass it on a user. In the same time it's possible to construct mitigation mechanics for such cases, so user economics be affected by...

Avoid Using Block.timestamp in checkClaimable

Handle Koustre Vulnerability details Impact checkClaimable function is dependent on using block.timestamp and this can be manipulated to claim funds earlier than it should in functions claim, forceClaim and lock Proof of Concept Provide direct links to all referenced code in GitHub. Add...

takeOutRewardTokens(): epochs calculation should be rounded up

Handle hickuphh3 Vulnerability details Impact If the owner would like to remove rewards, the number of epochs affected could potentially be 1 less because solidity division rounds down, resulting in more rewards taken out than allowed. Proof of Concept Assume currentEpoch is 1000 end epoch is 200...

Comptroller rewards can be artificially inflated and drained by manipulating [totalStaked - totalFrozen] (or: wrong rewards calculation)

Handle kenzo Vulnerability details By adding a small of amount of staking to a normal user scenario, and not approving this small amount as a loan for anybody, a staker can gain disproportionate amounts of comptroller rewards, even to the point of draining the contract. For example: Stakers A,B,C...

exitTempusAMM can be made to fail

Handle cmichel Vulnerability details There's a griefing attack where an attacker can make any user transaction for TempusController.exitTempusAMM fail. In exitTempusAMM, the user exits their LP position and claims back yield and principal shares. The LP amounts to redeem are determined by the...

Wrong implementation of CreditLimitByMedian.sol#getLockedAmount() makes it unable to unlock lockedAmount in CreditLimitByMedian model

Handle WatchPug Vulnerability details function getLockedAmount LockedInfo memory array, address account, uint256 amount, bool isIncrease public pure override returns uint256 if array.length == 0 return 0; uint256 newLockedAmount; if isIncrease ... else for uint256 i = 0; i amount newLockedAmount ...

depositAndFix can be made to fail

Handle cmichel Vulnerability details There's a griefing attack where an attacker can make any user transaction for TempusController.depositAndFix fail. In depositAndFix, swapAmount many yield shares are swapped to principal where swapAmount is derived from the function arguments. A final...

disableValidator() can distort totalGlobalShares (edge case)

Handle gpersoon Vulnerability details Impact Assume a validator has been added Assume this validator calls disableValidator maybe by accident or it recognizes a mistake Assume stake hasn't been called yet and endEpoch is still 0 The function disableValidator will execute and set v.disabledEpoch t...

Signature replay attacks for different identities (nonce on wrong party)

Handle cmichel Vulnerability details A single QuickAccount can serve as the "privilege" for multiple identities, see the comment in QuickAccManager.sol: NOTE: a single accHash can control multiple identities, as long as those identities set it's hash in privilegesaddressthis. this is by design If...

unchecked return value from approve()

Handle JMukesh Vulnerability details Impact contract uses an ERC20.approve call but does not check the success return value. Some tokens do not revert if the approval failed, returning false instead. The impact is that, tokens that don’t actually perform the approve and return false are still...

Previously created markets can be overwritten

Handle 0xRajeev Vulnerability details Impact The createMarket function allows accidental overwriting of previously created markets for the same combination of underlying and maturity timestamp u, m because there is no zero-address check to see if a previously created market exists for that...

lack of access modifier in mintReturn()

Handle JMukesh Vulnerability details Impact Due to lack of access modifier in mintReturn , any malicious actor can set its value to false due which function which will call mintaddress f, uint256 a will always fail which include: custodialInitiate mintZcTokenAddingNotional Proof of Concept Tools...

Trying to use timestamps with blocknumbers

Handle goatbug Vulnerability details Impact requirebondTimestamp + ONEDAY block.number; There are require statements comparing timestamps to blocknumbers. We cannot assume one block per second, this code would not work on different chains where block times radically differ. The effect is having...

Zero fees

Handle goatbug Vulnerability details Impact The fees system is broken. Anyone can easily pay zero fees. Proof of Concept Create a bundle of two txs to be mined in one block. First tx calls mintTo with an extrememly small amount, handleFees is called and importantly lastFee = block.timestamp; Seco...

Reentrancy through withdrawBounty

Handle jonah1005 Vulnerability details Impact The function settleAuction Auction.solL69-L109 calls withdrawBounty. However, there's no safety checks in addBounty function.Auction.solL126-L138 The attacker can add malicious contract through addBounty and hijack the control flow of settleAuction...

Reentrancy in settleAuction(): malicious publisher can bypass index timelock mechanism, inject malicious index, and rug the basket

Handle kenzo Vulnerability details The settleAuction function calls withdrawBounty before setting auctionOngoing = false, thereby allowing reentrancy. Impact A malicious publisher can bypass the index timelock mechanism and publish new index which the basket's users won't have time to respond to...

Vault may not have enough tokens for withdraw

Handle 0xRajeev Vulnerability details Impact There is an assumption in LegacyController.vault that the vault will have enough tokens0 to cover the balance difference. If not, the user may receive less than amount requested and balance funds get lost/locked unless the vault withdraws from the...

Controller.withdraw(...) User may lose funds when withdraw wantToken from the underlying contract

Handle WatchPug Vulnerability details The wantToken of the strategy may be different from the token argument of Controller.withdrawaddress token, uint256 amount according to code at line 469-474 of Controller.sol. if want != token address converter = vaultDetailsmsg.sender.converter;...