Withdraw from AaveVault will receive less than actual share

Handle gzeon Vulnerability details Impact AaveVault cache tvl and update it at the end of each push and pull. When withdrawing from LpIssuer, tokenAmounts is calculated using the cached tvl to be pulled from AaveVault. This will lead to user missing out their share of the accrued interest /...

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

Users can avoid paying vault fees

Handle cmichel Vulnerability details The LPIssuer.deposit/withdraw function charges protocol&management&performance fees through inflating the LP supply in the chargeFees function. However, this LP fees minting is skipped if the elapsed time is less than the managementFeeChargeDelay: if elapsed...

Wrong implementation of performanceFee can cause users to lose 50% to 100% of their funds

Handle WatchPug Vulnerability details A certain amount of lp tokens shares of the vault will be minted to the strategyPerformanceTreasury as performanceFee, the amount is calculated based on the minLpPriceFactor. However, the current formula for toMint is wrong, which issues more than 100% of the...

Admin can break _numberOfValidTokens

Handle cmichel Vulnerability details The ProtocolGovernance.numberOfValidTokens can be decreased by the admin in the ProtocolGovernance.removeFromTokenWhitelist function: function removeFromTokenWhitelistaddress addr external requireisAdminmsg.sender, "ADM"; tokensAllowedaddr = false; if...

Strategy can steal all the funds in ERC20Vault by rebalancing with a fake token in path

Handle WatchPug Vulnerability details PoC Given: A pool with 100 ETH and 40,000 USDC; A malicious or compromised Strategy can do the following: 1. Create a FAKE token, and add liquidity to ETH/FAKE and FAKE/USDC, making the price of ETH/FAKE to be 1 ETH per FAKE and the price of FAKE/USDC is 0.01...

ERC20Vault does not support fee-on-transfer token

Handle gzeon Vulnerability details Impact ERC20Vault does not support fee-on-transfer token since it does not check the balance received by the contract. Proof of Concept for uint256 i = 0; i 0 IERC20tokensi.safeTransferFromfrom, addressthis, tokenAmountsi; actualTokenAmounts = pushtokens,...

Consider fees while calculating tvl of univ3

Handle 0x421f Vulnerability details As of now fees owed are not being considered I think here due to this A comes in deposits at time X his deposit genrates fess Now though B comes later, he gets unfair adv from As deposit Just use pool.burn to update fees I think --- The text was updated...

Swap functions in ERC20 Vault will cause fund loss

Handle WatchPug Vulnerability details The current implementation of the two swap functions: swapExactInput and swapExactOutput in ERC20Vault.sol is using a wrong value for parameter recipient. Per the interface, the third parameter of trader.swapExactInput and trader.swapExactOutput is "recipient...

makePayment() Lack of access control allows malicious lender to retrieve a large portion of the funds earlier, making the borrower suffer fund loss

Handle WatchPug Vulnerability details function makePaymentuint256 amount external override returns uint256 principal, uint256 interest // The amount specified is an optional amount to be transfer from the caller, as a convenience for EOAs. requireamount == uint2560 ||...

Basket:handleFees fee calculation is wrong

Handle GiveMeTestEther Vulnerability details Impact The fee calculation on L141 is wrong. It should only get divided by BASE and not BASE - feePct Proof of Concept This shows dividing only by BASE is correct: Assumptions: BASE is 1e18 accordign to the code timeDiff is exactly ONEYEAR for easier...

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

AaveVault does not update TVL on deposit/withdraw

Handle cmichel Vulnerability details Aave uses rebasing tokens which means the token balance aToken.balanceOfthis increases over time with the accrued interest. The AaveVault.tvl uses a cached value that needs to be updated using a updateTvls call. This call is not done when depositing tokens whi...

UniV3Vault.sol#collectEarnings() can be front run

Handle WatchPug Vulnerability details For UniV3Vault, it seems that lp fees are collected through collectEarnings callable by the strategy and reinvested rebalanced. However, in the current implementation, unharvested yields are not included in tvl, making it vulnerable to front-run attacks that...

Unable to Unlock NFT Once it Locked and Owner may Lose His Token Permanently

Handle Meta0xNull Vulnerability details Impact function lockNft Allow Owner of NFT Lock his NFT. But Once it Locked, there is No UnLock Function and thus Owner may lose his token permanently because it is Required to be Unlock in beforeTokenTransfer. Proof of Concept Tools Used Manual Review...

Wrong calculation of excess depositToken allows stream creator to retrieve depositTokenFlashloanFeeAmount, which may cause fund loss to users

Handle WatchPug Vulnerability details uint256 excess = ERC20token.balanceOfaddressthis - depositTokenAmount - redeemedDepositTokens; In the current implementation, depositTokenFlashloanFeeAmount is not excluded when calculating excess depositToken. Therefore, the stream creator can call...

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

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

Reward token not correctly recovered

Handle cmichel Vulnerability details The Streaming contract allows recovering the reward token by calling recoverTokensrewardToken, recipient. However, the excess amount is computed incorrectly as ERC20token.balanceOfaddressthis - rewardTokenAmount + rewardTokenFeeAmount: function...

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

unstreamed variable is not updated in withdraw function

Handle csanuragjain Vulnerability details Impact Contract variable unstreamed is not updated in withdraw function which can lead to unstability Proof of Concept 1. Observe the stake function of Stream contract function stakeuint112 amount public lock updateStreammsg.sender ... unstreamed +=...

global unstreamed value is not updated after stream depositor withdraws an amount

Handle hubble Vulnerability details Impact Value of unstreamed public variable is not correct after stream depositor withdraws an amount before end of the stream. Proof of Concept File :Locke.sol Contract / Function : Stream / withdraw Line : 469 totalVirtualBalance -= virtualBal;...

Public variable unstreamed can be smaller than ∑ts.tokens due to unstreamed not being updated in withdraw()

Handle WatchPug Vulnerability details unstreamed is a public variable, and it's been actively managed in stake, updateStreamInternal. However, since users can also withdraw unstreamed depositToken, the global variable unstreamed should be updated in withdraw as well. For example: 1. Alice deposit...

DOS while dealing with erc20 when value(i.e amount*decimals) is high but less than type(uint112).max

Handle hack3r-0m Vulnerability details Impact reverts due to overflow for higher values but strictly less than typeuint112.max and hence when user calls exit or withdraw function it will revert and that user will not able to withdraw funds permanentaly. Proof of Concept Attaching diff to modify...

check for deposit token and reward token are not same

Handle hack3r-0m Vulnerability details Impact createStream does not check if deposit token and reward token are different addresses. Proof of Concept Not Required Tools Used Manual Review Recommended Mitigation Steps add check requirerewardToken != depositToken --- The text was updated...

reset depositTokenAmount in creatorClaimSoldTokens

Handle pauliax Vulnerability details Impact function creatorClaimSoldTokens should nullify depositTokenAmount, otherwise it may not be possible to recover deposit tokens later because the balance will be lower than accounted depositTokenAmount: uint256 excess = ERC20token.balanceOfaddressthis -...

Race condition on ERC20 approval

Handle WatchPug Vulnerability details function approveaddress spender, uint256 amount public virtual returns bool allowancemsg.senderspender = amount; emit Approvalmsg.sender, spender, amount; return true; Using approve to manage allowances opens yourself and users of the token up to frontrunning...

ts.tokens can potentially be reduced more than expected

Handle WatchPug Vulnerability details In the current implementation, ts.lastUpdate will only be updated when ts.tokens 0. Thus, ts.lastUpdate can be outdated for an exited user who deposits again. As a result, by the next time updateStreamInternal is called, ts.tokens will be reduced more than...

ArbitraryCall() allows attackers to steal ERC20 tokens from users wallets

Handle Jujic Vulnerability details A call to an arbitrary contract with custom calldata is made in arbitraryCalladdress who, bytes memory data, which means the contract can be an ERC20 token, and the calldata can be transferFrom a previously approved user. Impact The wallet balances for the amoun...

Recover tokens function will become unusable with reward tokens if they are withdrawn first.

Handle pedroais Vulnerability details Impact The recoverTokens function will become unusable with reward tokens if they are withdrawn first. Proof of Concept With reward tokens excess is defined as balance - rewardTokenAmount + rewardTokenFeeAmount The variable rewardTokenAmount never gets update...

governor can steal funds of user from all created streams using arbitraryCall

Handle hack3r-0m Vulnerability details Impact user approves token x to stream contract approval amount is typeuint256.max user calls createIncentivetoken x, someAmount incentivesx = someAmount creator calls claimIncentivetoken x incentivesx = 0 governance can arbitraryCall with data as ERC20token...

Deposit token flash loan fees can be stolen by streamCreator

Handle 0x0x0x Vulnerability details Concept On recoverTokens function in Stream. Excess amount of deposit token is calculated as follows: uint256 excess = ERC20token.balanceOfaddressthis - depositTokenAmount - redeemedDepositTokens; This calculation does not include depositTokenFlashloanFeeAmount...

Locke.sol:Stream - possible DOS on arbitraryCall

Handle ScopeLift Vulnerability details Impact If there's an airdrop that arbitraryCall would like to call, it could be DOSed by first calling createIncentive with the airdropped token, then backrunning calls to claimIncentive with calls to createIncentive. Proof of Concept Tools Used Recommended...

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

Fund freezing is possible as claimed reward tokens aren't accounted for by recoverTokens

Handle hyh Vulnerability details Impact Reward tokens accidently sent to the Stream contract cannot be recovered with recoverTokens if some reward tokens were already claimed with claimReward. As recoverTokens is the only recovering functionality in the contract the corresponding reward tokens wi...

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

Token allowances vulnerable to front-running

Handle toastedsteaksandwich Vulnerability details Impact The LockeERC20.approve function is vulnerable to front-running, as described in the following scenario: 1. Alice approves Bob to transfer 5 tokens 2. Alice decides to reduce this allowance to 3 tokens 3. Bob notices the allowance reduction ...

Extra fees paid for flash loans are unassigned

Handle 0x0x0x Vulnerability details Extra fees paid for flash loan are not included in total collected fees and they create excess balance which can be claimable by streamCreator. Extra fees should also get assigned to factory or atleast it should be clear that extra fees are for streamCreator...

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

arbitraryCall enables streamCreator to remove incentive tokens before endStream

Handle bitbopper Vulnerability details Impact streamCreator can remove incentive tokens before endStream by calling approve on the token beforehand. streamCreator has following methods of attack: guess from whom and with what he is going to be incentiviced listen in the mempool and win PGA in ord...

If the stream is a sale recover tokens function won't work.

Handle pedroais Vulnerability details Impact If the stream is a sale recover tokens function won't work. Proof of Concept Context : The function recover tokens uses balance - depositTokenAmount- redeemedDepositTokens to compute excess deposit tokens. RedeemedDepositTokens keeps track of tokens th...

Locke.sol:Stream - recoverTokens can bork depositToken balance

Handle ScopeLift Vulnerability details Impact if there's been a flashloan which adds to depositTokenFlashloanFeeAmount which gov can then collect but then recoverTokens is called before governor calls claimFees, then depositToken.balanceOfaddressthis is gonna be off by...

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

Possible incentive theft through the arbitraryCall() function

Handle toastedsteaksandwich Vulnerability details Impact The Locke.arbitraryCall function allows the inherited governance contract to perform arbitrary contract calls within certain constraints. Contract calls to tokens provided as incentives through the createIncentive function are not allowed i...

depositToken != rewardToken

Handle pauliax Vulnerability details Impact function createStream should validate that depositToken != rewardToken, otherwise, some functionality may not work as intended, e.g. in function recoverTokens it will become impossible to reach the second 'if' statement. Recommended Mitigation Steps...

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

Locke.sol:Stream - arbitraryCall can be used to drain incentive tokens

Handle ScopeLift Vulnerability details Impact Governor can drain incentive balance via arbitraryCall Proof of Concept The Stream contract offers createIncentive and claimIncentive which is the way the contract "expects" incentives to go. Access to claiming incentives is limited to the stream...

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

arbitraryCall allow inherited governance to steal incentives

Handle gzeon Vulnerability details Impact arbitraryCall did not check the balances of incentives, which allow inherited governance to steal the incentives. Proof of Concept Recommended Mitigation Steps Keep track of incentive token addresses in createIncentive and check the balance of each token...

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