Storage variable unstreamed can be artificially inflated

Handle harleythedog Vulnerability details Impact The storage variable unstreamed keeps track of the global amount of deposit token in the contract that have not been streamed yet. This variable is a public variable, and users that read this variable likely want to use its value to determine wheth...

Undesired tokens can be called

Handle 0x1f8b Vulnerability details Impact Possible call to unwanted tokens. Proof of Concept In the method reclaimTokens of Vault contract it was called tranfer to tokens provided by the caller, but maybe these tokens are not allowed by the governance, it must be checked that isAllowedTokenaddre...

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

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

arbitraryCall() can get blocked by an attacker

Handle GiveMeTestEther Vulnerability details Impact "arbitraryCall"'s L733 use case is to claim airdrops by "gov". If the address "who" is a token that could be send as an incentive by an attacker via "createIncentive" then such claim can be made unusable, because on L735 there is a...

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

AuctionBurnReserveSkew.getPegDeltaFrequency() Wrong implementation can result in an improper amount of excess Liquidity Extension balance to be used at the end of an auction

Handle WatchPug Vulnerability details function getPegDeltaFrequency public view returns uint256 uint256 initialIndex = 0; uint256 index; if count auctionAverageLookback initialIndex = count - auctionAverageLookback; uint256 total = 0; for uint256 i = initialIndex; i count; ++i index =...

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

_transferBurn should reduce totalSupply

Handle WatchPug Vulnerability details function transferBurn address sender, address recipient, uint256 amount, uint256 burnt internal uint256 senderBalance = balancessender; requiresenderBalance = amount + burnt, "OVL:balance= amount + burnt, "OVL:balanceamount+burnt"; unchecked balancessender =...

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

Transmuter yield can be gamed by only staking when yield is distributed

Handle cmichel Vulnerability details The Transmuter.distribute function distributes the yield to the buffer which is then distributed to all stakers over a TRANSMUTATIONPERIOD, see runPhasedDistribution and updateAccount. If the elapsed time from the last phase distribution is greater than the...

Missing _token.approve() to curvePool in setZapConfig

Handle WatchPug Vulnerability details function setZapConfig uint256 idx, address sett, address token, address curvePool, address withdrawToken, int128 withdrawTokenIndex external onlyGovernance; requiresett != address0; requiretoken != address0; require withdrawToken == addressWBTC || withdrawTok...

Unused slippage params

Handle pauliax Vulnerability details Impact Unused slippage params. function addLiquidity in VaderRouter both V1 and V2 do not use slippage parameters: uint256, // amountAMin = unused uint256, // amountBMin = unused making it susceptible to sandwich attacks / MEV. For a more detailed explanation,...

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

Governor's veto protection can be exploited

Handle cmichel Vulnerability details The GovernorAlpha's council cannot veto proposals that perform a call to the contract itself. This can be exploited by malicious proposal creators by appending a new call at the end of their proposal that simply calls an innocent function like...

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

Anyone can DDOS vesting contract

Handle pauliax Vulnerability details Impact Vest function can be accessed by anyone. It accepts arbitrary beneficiary and pushes new vesting to the array of this beneficiary timelocks. As a malicious actor I can block any user by just invoking vest function with a tiny amount of vest token. The...

Claim airdrop repeatedly

Handle gpersoon Vulnerability details Impact Suppose someone claims the last part of his airdrop via claimExact of AirdropDistribution.sol Then airdropmsg.sender.amount will be set to 0. Suppose you then call validate again. The check "airdropmsg.sender.amount == 0" will allow you to continue,...

Trades where toToken is feeOnTransferToken might send user less tokens than finalAmountMin

Handle kenzo Vulnerability details Slingshot's executeTrades checks that the trade result amount to be sent to the user is bigger than finalAmountMin, and after that sends the user the amount. But if the token charges fee on transfer, the final transfer to the user will decrease the amount the us...

DOS attack possible for Token->ETH trades

Handle pmerkleplant Vulnerability details Impact Any user's trade from some token to a native token e.g. ETH can be DOSed if an attacker sends native tokens to the Executioner contract. If the attacker watches for trades, he/she could either make the whole trade fail or, depending on the...

Admin can abuse grantSlingshot and steal user funds

Handle kenzo Vulnerability details After user has .approved ApprovalHandler, admin can grantSlingshot himself, and then call ApprovalHandler.transferFrom with parameters that will transfer all tokens to himself before the user calls Slingshot's executeTrades. Although this vulnerability requires ...

Curve Metapool does not support rebasing token

Handle gzeon Vulnerability details Impact For interest baring token, Curve use another version of the contract with consideration of underlying asset. The metapool contracts does not support such behaviour. Proof of Concept Actually it is written in Curve's doc Token balances must not change...

Inconsistent ERC20 function may lead to DOS and unexpected behaviour

Handle gzeon Vulnerability details Impact There are 2 important yet similar concept in this wrapper contract, namely "SHARE" and "BALANCE". SHARE equivalent to the number of underlying ibbtc and is stored in balance as the native value, while BALANCE is SHAREpricepershare and is the return value ...

Missing slippage checks

Handle cmichel Vulnerability details The contracts are missing slippage checks which can lead to being vulnerable to sandwich attacks. A common attack in DeFi is the sandwich attack. Upon observing a trade of asset X for asset Y, an attacker frontruns the victim trade by also buying asset Y, lets...

FeePoolV0.sol#distributeMochi() will unexpectedly flush treasuryShare, causing the protocol fee cannot be properly accounted for and collected

Handle WatchPug Vulnerability details distributeMochi will call buyMochi to convert mochiShare to Mochi token and call shareMochi to send Mochi to vMochi Vault and veCRV Holders. It wont touch the treasuryShare. However, in the current implementation, treasuryShare will be reset to 0. This is...

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

Deposits don't work with fee-on transfer tokens

Handle cmichel Vulnerability details There are ERC20 tokens that may make certain customizations to their ERC20 contracts. One type of these tokens is deflationary tokens that charge a certain fee for every transfer or transferFrom. Others are rebasing tokens that increase in value over time like...

If newRatio > ibRatio after a settlement the protocol could lose its funds.

Handle tensors Vulnerability details Suppose that after a certain settleAuction call we have that newRatio ibRatio. I don't see any reason why this couldn't be possible, going through the math and solving for this condition we can see that: if b ibRatio then newRatio ibRatio assuming...

Basket.sol#mint() Malfunction due to extra nonReentrant modifier

Handle WatchPug Vulnerability details function mintuint256 amount public nonReentrant override mintToamount, msg.sender; function mintTouint256 amount, address to public nonReentrant override requireauction.auctionOngoing == false; The mint method is malfunction because of the extra nonReentrant...

Setting Factory.bondPercentDiv to zero cause Denial of Service in Auction.bondForRebalance()

Handle pants Vulnerability details The function Factory.setBondPercentDiv allows the owner to set the state variable Factory.bondPercentDiv to zero. Impact If Factory.bondPercentDiv equals zero then the function Auction.bondForRebalance will always revert due to a division by zero: bondAmount =...

Prevent creating the same market twice

Handle gpersoon Vulnerability details Impact The function createMarket of MarketPlace.sol doesn't check if the market already exists. So it could accidentally deploy a market with has the same maturity timestamp twice and overwrite the previous values of the market. The previously deployed market...

ConcentratedLiquidityPool.burn() Wrong implementation

Handle WatchPug Vulnerability details The reserves should be updated once LP tokens are burned to match the actual total bento shares hold by the pool. However, the current implementation only updated reserves with the fees subtracted. Makes the reserve0 and reserve1 smaller than the current...

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

Overflow in the mint function of ConcentratedLiquidityPool causes LPs' funds to be stolen

Handle broccoli Vulnerability details Impact Similar to a previous finding in the IndexPool contract, the mint function of ConcentratedLiquidityPool allows integer overflows when checking the balance is larger or equal to the received amount of token plus the reserve. As a result, the attacker...

ConcentratedLiquidityPosition.sol#collect() Users may get double the amount of yield when they call collect() before burn()

Handle WatchPug Vulnerability details When a user calls ConcentratedLiquidityPosition.solcollect to collect their yield, it calcuates the yield based on position.pool.rangeFeeGrowth and position.feeGrowthInside0, position.feeGrowthInside1: When there are enough tokens in bento.balanceOf, it will...

Wrong inequality when adding/removing liquidity in current price range

Handle cmichel Vulnerability details The ConcentratedLiquidityPool.mint/burn functions add/remove liquidity when priceLower currentPrice && currentPrice priceUpper. Shouldn't it also be changed if priceLower == currentPrice? Impact Pools that mint/burn liquidity at a time where the currentPrice i...

Erc20.transfer() return value is ignored

Handle pants Vulnerability details According to the ERC-20 Token Standard, the function transfer returns false on failure. However, the return value of Erc20.transfer is ignored 8 times: 1. In Swivel.exitVaultFillingZcTokenExit, line 313. 2. In Swivel.exitVaultFillingZcTokenExit, line 315. 3. In...

Swivel: Taker is charged fees twice in exitVaultFillingVaultInitiate

Handle itsmeSTYJ Vulnerability details Impact Taker is charged fees twice in exitVaultFillingVaultInitiate . Maker is transferring less than premiumFilled to taker and then taker is expected to pay fees i.e. taker's net balance is premiumFilled - 2fee Recommended Mitigation Steps function...

IndexPool does not apply barFee

Handle pauliax Vulnerability details Impact IndexPool does not mint fees barFee and barFeeTo. It initializes these variables but never uses them in a meaningful way. Recommended Mitigation Steps Consider handling the fees in this contract similarly to other pools. --- The text was updated...

Incorrect multiplication in _computeSingleOutGivenPoolIn of IndexPool

Handle broccoli Vulnerability details Impact The computeSingleOutGivenPoolIn function of IndexPool uses the raw multiplication i.e., to calculate the zaz variable. However, since both BASE - normalizedWeight and swapFee are in WAD, the mul function should be used instead to calculate the correct...

onlyOwner Role Can Manipulate Price Oracle

Handle leastwood Vulnerability details Impact The V2 LendingController.sol contract incorporates a custom oracle used to fetch the relevant price feeds for token pairs. The onlyOwner role is controlled by a modified timelock contract callable only from a single externally owned account. This role...

Basket.sol#auctionBurn() A failed auction will freeze part of the funds

Handle WatchPug Vulnerability details Given the auctionBurn function will burn the auction bond without updating the ibRatio. Once the bond of a failed auction is burned, the proportional underlying tokens won't be able to be withdrawn, in other words, being frozen in the contract. Proof of Conce...

Adding assymetric liquidity in _addLiquidity results in fewer LP tokens minted than what should be wanted

Handle tensors Vulnerability details Impact Because the call in addLiquidity forwards the entire balances of the 3 stablecoins without checking the ratio between the 3, less liquidity is minted than what should be wanted. Furthermore, an attacker can abuse this arbitrage the forwarded balances if...

Vault.withdraw sometimes burns too many shares

Handle cmichel Vulnerability details The Vault.withdraw function attempts to withdraw funds from the controller if there are not enough in the vault already. In the case the controller could not withdraw enough, i.e., where diff toWithdraw, the user will receive less output tokens than their fair...

wrong YAXIS estimates

Handle cmichel Vulnerability details The Harvester.getEstimates contract tries to estimate a YAXIS amount but uses the wrong path and/or amount. It currently uses a WETH input amount to compute a YAXIS - WETH trade. address memory path; path0 = IStrategystrategy.want; path1 =...

Issue in balance update in setCap

Handle 0xsanson Vulnerability details Impact During Controller.setCap we change vaultDetailsvault.balance to vaultDetailsvault.balance.subbalance. This is wrong, and the correct value should be vaultDetailsvault.balance.subdiff, because diff is the value withdrawn from the strategy. High risk...

PostAuctionLauncher.sol#finalize() Adding liquidity to an existing pool may allows the attacker to steal most of the tokens

Handle WatchPug Vulnerability details PostAuctionLauncher.finalize can be called by anyone, and it sends tokens directly to the pair pool to mint liquidity, even when the pair pool exists. An attacker may control the LP price by creating the pool and then call finalize to mint LP token with unfai...