cancelPromotion() Unable to cancel unstarted promotions

Handle WatchPug Vulnerability details For unstarted promotions, cancelPromotion will revert at block.timestamp - promotion.startTimestamp in getCurrentEpochId. Call stack: cancelPromotion - getRemainingRewards - getCurrentEpochId. function getRemainingRewardsPromotion memory promotion internal vi...

Possibility to drain SavingsAccount contract assets

Handle kemmio Vulnerability details Impact A malicious actor can manipulate switchStrategy function in a way to withdraw tokens that are locked in SavingsAccount contract the risk severity should be reviewed Proof of Concept Firstly an attacker need to deploy a rogue strategy contract implementin...

Wrong emergencyWithdraw logic

Handle 0x1f8b Vulnerability details Impact The contract doesn't work as expected. Proof of Concept The method, emergencyWithdraw inside the contract yield/NoYield doesn't work as expected, the transfer was done with received value, and it should be done with amount, so the emergencyWithdraw never...

AaveVault is vulnerable to flashloan sandwich attack

Handle WatchPug Vulnerability details In the current implementation of AaveVault, tvl is only updated after each deposit/withdraw, which means unclaimed yields won't be included in tvl until someone deposit/withdraw, making it vulnerable to flashloan sandwich attack that steals pending yields. Po...

Governor can steal funds from vaults

Handle 0x0x0x Vulnerability details reclaimTokens function is as follows function reclaimTokensaddress to, address memory tokens external nonReentrant requirenft 0, ExceptionsLibrary.INITIALIZATION; IProtocolGovernance governance = vaultGovernance.internalParams.protocolGovernance; bool...

User deposits don't have min. return checks

Handle cmichel Vulnerability details The LPIssuer.deposit first computes balanced amounts on the user's defined tokenAmounts. The idea is that LP tokens give the same percentage share of each vault tokens' tvl, therefore the provided amounts should be balanced, meaning, the depositAmount / tvl...

Possible price manipulation while adding liquidity to uniV3

Handle 0x421f Vulnerability details Right now if we see the code there are no checks before liq being added to check if pool is manipulated. Hence there rises possibility of sandwich attack vector here, more so with concentrated liq imo Could be done with flash loan or with own tokens Attack woul...

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

Tokens can be stolen when depositToken == rewardToken

Handle cmichel Vulnerability details The Streaming contract allows the deposit and reward tokens to be the same token. I believe this is intended, think Sushi reward on Sushi as is the case with xSushi. The reward and deposit balances are also correctly tracked independently in depositTokenAmount...

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

recoverTokens did not consider depositTokenFlashloanFeeAmount

Handle gzeon Vulnerability details Impact In recoverTokens, when token == depositToken, the excess is defined as follow excess = ERC20token.balanceOfaddressthis - depositTokenAmount - redeemedDepositTokens; Instead we should also consider depositTokenFlashloanFeeAmount gained from flashloan fee...

depositToken has to be not equal rewardToken

Handle 0x0x0x Vulnerability details Creating a stream, where depositToken == rewardToken might be a use case. But since amounts of both of them are accumulated in different variables and there is a recoverTokens function. When depositToken == rewardToken, one can easily organize scams using strea...

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

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

AbstractRewardMine.sol#setRewardToken is dangerous

Handle 0x0x0x Vulnerability details Impact In case the reward token is changed, totalDeclaredReward will be changed and likely equal to 0. Since userStakePadding and globalStakePadding are accumulated, changing the reward token will not reset those values. Thus, it will create problems...

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

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

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

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

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

Using/Importing ERC1155Supply.sol from openzeppelin version 4.3.2

Handle hubble Vulnerability details Using/importing openZeppelin version 4.3.2 file ERC1155Supply.sol which has a security advisory. Reference : GHSA-wmpv-c2jp-j2xg Proof of Concept File : collateral/OverlayV1OVLCollateral.sol line 7: import...

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

Cached version of ovl may be outdated

Handle pauliax Vulnerability details Impact contract OverlayV1OVLCollateral and OverlayV1Governance cache ovl address: IOverlayTokenNew immutable public ovl; This variable is initialized in the constructor and fetched from the mothership contract: mothership = IOverlayV1Mothershipmothership; 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...

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

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

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

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

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

WrappedIbbtcEth contract will use stalled price for mint/burn if updatePricePerShare wasn't run properly

Handle hyh Vulnerability details Impact Malicious user can monitor SetPricePerShare event and, if it was run long enough time ago and market moved, but, since there were no SetPricePerShare fired, the contract's pricePerShare is outdated, so a user can mint with pricePerShare that is current for...

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

anyone can create a vault by directly calling the factory

Handle jonah1005 Vulnerability details Impact MochiVaultFactory.solL26-L37 There's no permission control in the vaultFactory. Anyone can create a vault. The transaction would be reverted when the government tries to deploy such an asset. As the protocol checks whether the vault is a valid vault b...

Debt accrual is path-dependant and inaccurate

Handle cmichel Vulnerability details The total debt in MochiVault.accrueDebt increases by the current debt times the debt index growth. This is correct but the total debt is then reduced again by the calling user's discounted debt, meaning, the total debt depends on which specific user performs t...

liquidation factor < collateral factor for Sigma type

Handle cmichel Vulnerability details The MochiProfileV0 defines liquidation and collateral factors for different asset types. For the AssetClass.Sigma type, the liquidation factor is less than the collateral factor: function liquidationFactoraddress asset public view override returns float memory...

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

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

Chainlink Adapter Missing Validation Of latestRoundData() Outputs

Handle leastwood Vulnerability details Impact ChainlinkAdapter.getPrice queries a Chainlink oracle to retrieve the latest price for a given asset. However, this external call does not validate the data retrieved is fresh. Proof of Concept Tools Used Manual code review Recommended Mitigation Steps...

Change in interest rate can disable repay of loan

Handle pmerkleplant Vulnerability details Impact The ability of a borrower to repay a loan is disabled if the interest rate is set too high by the InterestRateModel. However, there is neither a check when setting the interest rate nor an indication in the IInterestRateModel's specs of this...

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

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

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

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