_swapExactOutputSingle() and _swapExactOutputMultihop() May Fail When Transfer Token to recipient

Handle Meta0xNull Vulnerability details Impact IERC20input.safeTransferFromaddressthis, recipient, options.limitAmount - amountIn; For safeTransferFrom to work it needs to have an enough approval. In this case, obviously this contract does not approve every recipient when Transfer Token, so we ca...

Any arbitraryCall gathered airdrop can be stolen with recoverTokens

Handle hyh Vulnerability details Impact Any airdrop gathered with arbitraryCall will be immediately lost as an attacker can track arbitraryCall transactions and back run them with calls to recoverTokens, which doesn't track any tokens besides reward, deposit and incentive tokens, and will give th...

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

Creating rewardTokens without streaming depositTokens

Handle bitbopper Vulnerability details Impact stake and withdraws can generate rewardTokens without streaming depositTokens. It does not matter whether the stream is a sale or not. The following lines can increase the reward balance on a withdraw some time after stake: // accumulate reward per...

LockeERC20 is vulnerable to frontrun attack

Handle egjlmn1 Vulnerability details Impact a user can steal another user's tokens if he frontrun before he changes the allowance. The approve function receives an amount to change to. Lets say user A approved user B to take N tokens, and now he wants to change from N to M, if he calls approveM t...

_calculateMaltRequiredForExit makes wrong calculations

Handle 0x0x0x Vulnerability details Proof of concept calculateMaltRequiredForExit calculates the quantity of malts returned. uint256 maltQuantity = userMaltPurchased.mulamount.divuserCommitment; uint256 fullReturn = maltQuantity.mulcurrentPrice / pegPrice; userMaltPurchased / userCommitment...

Permissions - notSameBlock is insufficient for reentrancy protection

Handle ScopeLift Vulnerability details Impact TBD how bad the impact is The docs mention that the notSameBlock modifier and associated notSameBlock method is used to guard against reentrancy. However, users can still call a method twice in a single transaction by using transferring assets/positio...

onUnbond calculations incorrect leading to lost funds

Handle harleythedog Vulnerability details Impact Consider the stake padding example given in the contest description here: . At the end of the example, User A has 100 bonded LP and has 100 stake padding. User B has 100 bonded LP and 200 stake padding. There are 300 in rewards in the system. For...

splitReinvest() can be front run

Handle WatchPug Vulnerability details function splitReinvestuint256 rewardLiquidity external retrieveRewardrewardLiquidity; uint256 rewardBalance = rewardToken.balanceOfaddressthis; rewardToken.safeTransferaddressdexHandler, rewardBalance.div2; dexHandler.buyMalt; bondAccountmsg.sender; emit...

Unable to remove liquidity in Recovery Mode

Handle gzeon Vulnerability details Impact According to When the Malt price TWAP drops below a specified threshold eg 2% below peg then the protocol will revert any transaction that tries to remove Malt from the AMM pool ie buying Malt or removing liquidity. Users wanting to remove liquidity can...

AuctionEschapeHatch.sol#exitEarly updates state of the auction wrongly

Handle 0x0x0x Vulnerability details Vulnerability AuctionEschapeHatch.solexitEarly takes as input amount to represent how much of the When the user exits an auction with profit, to apply the profit penalty less maltQuantity is liquidated compared to how much malt token the liquidated amount...

MiningService _withdrawMultiple will fail most of the times

Handle hyh Vulnerability details Impact Impact depends on subtraction overflow handling and this way on the compiler version used for production deployment. If compiler version above 0.8: The compiler will check subtraction and fail, so: a user will have all withdrawals failed most of the times,...

Anyone can remove liquidity

Handle jayjonah8 Vulnerability details Impact In UniswapHandler.sol, anyone can call the removeLiquidity function stealing all the lp tokens in the contract even if they never added liquidity in the first place since the function simply credits the msg.sender with the entire lp token balance of t...

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

Function may not be implemented

Handle jayjonah8 Vulnerability details Impact In StabilizerNode.sol the distributeSuply function calls the check function on the ISupplyDistributionController. Searching through the code base I didn't location where the function is actually implemented. If this is calling a function within the...

Purchased Malt and Auction Data Can Be Manipulated/Thrown Off

Handle jayjonah8 Vulnerability details Impact In Auction.sol, the purchased variable in the purchaseArbitrageTokens function can be manipulated throwing off the AuctionData and the ratio of the realCommitment vs the purchased amount. This is because "purchased" simply returns...

_totalSupply not updated on transferMint/Burn

Handle cmichel Vulnerability details The OverlayToken has a transferMint and transferBurn function which is supposed to act like a transfer followed by a mint/burn. However, a mint/burn updates the totalSupply see mint/burn but these functions do not. Recommended Mitigation Steps The transferMint...

Timelock and events for governor functions

Handle pauliax Vulnerability details Impact There are contracts that contain functions that change important parameters of the system, e.g. OverlayV1Mothership has setOVL, initializeMarket, disableMarket, enableMarket, initializeCollateral, enableCollateral, disableCollateral, adjustGlobalParams...

_totalSupply not updated in _transferMint() and _transferBurn()

Handle gpersoon Vulnerability details Impact The functions transferMint and transferBurn of OverlayToken.sol don't update totalSupply. Whereas the similar functions mint and burn do update totalSupply. This means that totalSupply and totalSupply will not show a realistic view of the total OVL...

YaxisVaultAdapter.sol#withdraw() will most certainly fail

Handle WatchPug Vulnerability details The actual token withdrawn from vault.withdraw will most certainly less than the amount, due to precision loss in tokensToShares and vault.withdraw. As a result, IDetailedERC20token.safeTransferrecipient, amount will revert due to insufficant balance. Based o...

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

Ensure on-chain that cache is synced

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

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

VaderRouter._swap performs wrong swap

Handle cmichel Vulnerability details The 3-path hop in VaderRouter.swap is supposed to first swap foreign assets to native assets, and then the received native assets to different foreign assets again. The pool.swapnativeAmountIn, foreignAmountIn accepts the foreign amount as the second argument...

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

Lacking Validation Of Chainlink' Oracle Queries

Handle leastwood Vulnerability details Impact TwapOracle.consult is missing additional validations to ensure that the round is complete and has returned a valid/expected price. The consult improperly casts an int256 price to uint256 without first checking the value. As a result, the variable may...

Attacker can claim more IL by manipulating pool price then removeLiquidity

Handle gzeon Vulnerability details Impact Vader reimburse user IL immediately when user withdraw from the pool VaderRouterV2.sol:L227, an attacker can therefore manipulate the pool balance causing a high IL, remove liquidity and restore the pool balance such that he will receive a larger IL...

All user assets which are approved to VaderPoolV2 may be stolen

Handle TomFrench Vulnerability details Impact Total loss of funds which have been approved on VaderPoolV2 Proof of Concept VaderPoolV2 allows minting of fungible LP tokens with the mintFungible function Crucially this function allows a user supplied value for from which specifies where the...

VaderPoolV2.rescue results in loss of funds rather than recoverability

Handle TomFrench Vulnerability details Impact Any unaccounted for tokens on VaderPoolV2 can be siphoned off by anyone Proof of Concept VaderPoolV2 has a rescue function which allows any unaccounted for tokens to be recovered. However there is no access control on this function which means than...

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

Vestings' revoke status can be set by anyone

Handle cmichel Vulnerability details The idea of revoking vesting supposedly exists for the admins to call Vesting.revoke and claim back a user's vesting. However, if the user wants to protect their vesting from being revoked by the admin, they can create a new vest with isRevocable = false and a...

Double Spend in AirDropDistribution.sol

Handle elprofesor Vulnerability details HIGH Impact Due to improper validation of input, approved airdrop users are able to double spend airdrop allocated tokens. This is due to insufficient validation in validate and claimExact which allows the user to reset the amount of tokens they have claime...

FSDVesting: Restrict updateVestedTokens() calls to only FSD token contract

Handle hickuphh3 Vulnerability details Impact The updateVestedTokens increases the amount of tokens to be vested for a beneficiary. There is no access restriction to the function. The intended total vesting duration is 30 months with a 12-month cliff where 5% is immediately unlocked, and the...

Contract BasicSale is missing an approve(address(vestLock), 2**256-1) call

Handle Reigada Vulnerability details Impact As we can see in the contracts AirdropDistribution and InvestorDistribution, they both have the following approve call: mainToken.approveaddressvestLock, 2256-1; This is necessary because both contracts transfer tokens to the vesting contract by calling...

Slingshot: Incorrect initial balance fetched for native token in executeTrades()

Handle hickuphh3 Vulnerability details Impact The executioner contract only supports ERC20ERC20 token trades. Native token swaps are supported by either wrapping / unwrapping the ERC20 wrapped native token before / after the trades respectively. When exchanging from the native token, the wrapping...

pricePerShare is only lazily updated

Handle cmichel Vulnerability details The pricePerShare value is cached in updatePricePerShare and this value can be old and not reflect the actual core.pricePerShare value. The same issue exists on the sidechain variant when oracle price updates happen infrequently. This leads to potentially big...

Null check in pricePerShare

Handle hack3r-0m Vulnerability details oracle can 0 as a price of the share, in that case, 0 will be the denominator in some calculations which can cause reverts from SafeMath for e.g here: resulting in Denial Of Service. Add a null check to ensure that on every update, the price is greater than ...

Profit from difference between core.pricePerShare() and pricePerShare()

Handle gpersoon Vulnerability details Impact Suppose there is a small difference between core.pricePerShare and pricePerShare of WrappedIbbtcEth.sol on Eth main chain. This could be due to the fact that updatePricePerShare hasn't been called for a while, or a sudden change in core.pricePerShare. ...

Wrong calculation of erc20Delta and ethDelta

Handle WatchPug Vulnerability details function fillZrxQuote IERC20 zrxBuyTokenAddress, address payable zrxTo, bytes calldata zrxData, uint256 ethAmount internal returns uint256, uint256 uint256 originalERC20Balance = 0; if!signifiesETHOrZeroaddresszrxBuyTokenAddress originalERC20Balance =...

UserManager: updateLockedData() doesn't check that the amount is actually locked.

Handle itsmeSTYJ Vulnerability details Impact The function updateLockedData does not actually check if the amount required to be locked is actually locked. Proof of Concept Same solution as my other high issue. I've added comments where relevant. function updateLockedData address borrower, uint25...

Rebalance will fail due to low precision of percentages

Handle cmichel Vulnerability details The AssetManager.rebalance function has a check at the end to ensure that all tokens are deposited again: requiretoken.balanceOfaddressthis == 0, "AssetManager: there are remaining funds in the fund pool"; The idea is that the last market deposits all...

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

QuickAccManager.sol Potential replay attack

Handle WatchPug Vulnerability details This issue is possibly duplicate with a previous issue named "Potential replay attack" In QuickAccManager.solsendTxns and QuickAccManager.solsendTransfer, addressidentity is not included in the txHash, makes it possible to replay the transaction on another...

Number of prizes math is wrong

Handle cmichel Vulnerability details The math described in Splitting the prizes and implemented in DrawCalculator.numberOfPrizesForIndex seems to be wrong. Assuming a bit range of 4 16 possibilities per position and cardinality of 8. Note that degree is determined by the first position where it...

unchecked return value from Transfer/TransferFrom()

Handle JMukesh Vulnerability details Impact The ERC20.transfer and ERC20.transferFrom functions return a boolean value indicating success. This parameter needs to be checked for success. Some tokens do not revert if the transfer failed but return false instead. Proof of Concept Tools Used manual...

exitZcTokenFillingZcTokenInitiate in Swivel.sol, token transfer may fail without function reverting

Handle GalloDaSballo Vulnerability details Impact exitZcTokenFillingZcTokenInitiate in Swivel.sol uses transferFrom this function can failmeaning tokens are not transferred,without causing a revert. This can break the accounting of the protocol The reason why this can happen is that certain ERC20...

Wrong indexes: positionId vs incentiveId

Handle 0xsanson Vulnerability details Impact In multiple functions in ConcentratedLiquidityPoolManager, the index positionId is used instead of the correct incentiveId when dealing with the incentives mapping. Of course the issue is that incentives cannot be used, or in some cases only by lucky...

uToken's approve() Function is Susceptible to Frontrunning Attacks

Handle leastwood Vulnerability details Impact Order makers will call the approve function to facilitate transactions between order takers and makers. If a taker sees that a maker is seeking to reduce its order exposure by reducing Swivel.sol's allowance, a taker can frontrun this transaction to...

uToken ERC20 approve method missing return value check #L109

Handle defsec Vulnerability details Impact The initiateVaultFillingZcTokenInitiate function performs an ERC20.approve call but does not check the success return value. Some tokens do not revert if the approval failed but return false instead. Proof of Concept 1. Navigate to "" 2...

User can get more/less interest than required

Handle csanuragjain Vulnerability details Impact It seems that owner must not get interest for transfer fee which is collected using transferNotionalFee No interest is calculated on owner side before reducing the owner notional amount But this can simply be bypassed if owner adds new notional whi...