Title: Yield can be unfairly divided because of MEV/Just-in-time stablecoin deposits

Lines of code Vulnerability details Impact An attacker can use MEV via gas auction or Flashbots or control of miners to cause an unfair division of yield. By providing a very large relative to the size of all other stablecoin deposits combined stablecoin deposit Just-in-Time before an admin's cal...

User can forfeit other user rewards

Lines of code Vulnerability details Impact User can forfeit other user rewards by giving a higher startIndex in getReward function Proof of Concept 1. Assume User B has not received any reward yet so that his userClaimstokenUser B=0 2. User A calls getReward function with account as User B and...

L10: Deposits don’t work with FoT tokens

78 comment Warden: hickuphh3 Line References Description FoT token deposits are not supported because amount is used for internal accounting, but the actual amount received will be less than it due to the fee. I gave a low severity rating because of the existence of a token whitelist. Referenced...

[WP-H12] Wrong implementation of ConvexCurveLPVault#withdrawOnLiquidation() may cause the positions collateralized with ConvexCurveLP unbale to be liquidated as withdrawOnLiquidation() will always fail

Lines of code Vulnerability details function withdrawuint256 amount, address to internal returns uint256 // Withdraw from Convex address baseRewardPool = getBaseRewardPool; IConvexBaseRewardPoolbaseRewardPool.withdrawAndUnwrapamount, true; // Deliver Curve LP Token...

Possible lost msg.value

Lines of code Vulnerability details Impact Possible lost value in depositCollateral function call Proof of Concept In call depositCollateral can will send value and the asset can be an ERC20!= address0, if LidoVault and ConvexCurveLPVault contract receive this call the fouds will lost Also in...

Overexert on-chain slippage/loss control may cause users' funds to be frozen in the contract

Lines of code Vulnerability details requirewithdrawAmount = amount.percentMul9900, Errors.VTWITHDRAWAMOUNTMISMATCH; The GeneralVault.sol contract comes with a on-chain slippage/loss control to ensure the output amount is no more than 1% less of the requested amount. This can be a problem when the...

Unchecked call in _withdrawFromYieldPool

Lines of code GeneralVault.solL100-L128 LidoVault.solL122-L149 Vulnerability details Impact In the withdrawFromYieldPool function in LidoVault.sol, the receivedETHAmount value is returned before the sent value for the call function is checked on line 142. When withdrawCollateral is called in...

C4-004 : Centralization Risk

112 comment Warden: defsec Impact - LOW The system is heavily relies on the ExecutorManager. Therefore, It contains centralization risk If the execution manager is EOA and captured. Proof of Concept 1. Navigate to the following contract...

Reward Manager of the Convex Base Reward Pool Can DoS processYield()

Lines of code Vulnerability details Impact The ConvexCurveLPVault.sol contract allows users to earn a yield on curve token deposits. Rewards are paid out in native CRV and CVX tokens but the reward manager of the base pool may opt to add extra rewards. Because the reward manager has the ability t...

Wrong order of return and require

Lines of code Vulnerability details Impact It should check require before return, but withdrawFromYieldPool function just return before require, resulting in not checking the result of the call, users may lose their collateral. Proof of Concept The withdrawFromYieldPool function in LidoVault.sol:...

Withdraw success check don't happen

Lines of code Vulnerability details Impact The protocol suppose to send ETH to the user, but in case of fail, nothing will check it. Proof of Concept the code is 140 bool sent, bytes memory data = addressto.callvalue: receivedETHAmount''; 141 return receivedETHAmount; 142 requiresent,...

Initialization function can be front-run

Lines of code Vulnerability details Impact The call to the initialize function can be front-run, and a malicious provider can be passed in and set to addressesProvider on any contract that inherits GeneralVault. This is significant because the onlyAdmin modifier checks if the msg.sender is...

Users Can Game Yield Distributions

Lines of code Vulnerability details Impact processYield restricts who can call this function to just the vault admin. Upon being processed, the treasury receives its fair share of the yield and the rest is transferred to the YieldManager.sol contract. To distribute yield, the manager calls...

[LidoVault.sol][_withdrawFromYieldPool] Should not call return receivedETHAmount before the require check

Lines of code Vulnerability details Impact It is possible that users cannot receive ETH if sending ETH to users is not successful because the require check is unreachable. Proof of Concept withdrawFromYieldPool function executes the following code if user requests ETH withdrawal. if asset ==...

hard-coded slippage may freeze user funds during market turbulence (resubmit to downgrade severity)

Lines of code Vulnerability details Impact GeneralVault.solL125 GeneralVault set a hardcoded slippage control of 99%. However, the underlying yield tokens price may go down. If Luna/UST things happen again, users' funds may get locked. LidoVault.solL130-L137 Moreover, the withdrawal of the...

YieldManager's distributeYield can be subject to sandwich attacks

Lines of code Vulnerability details distributeYield uses Uniswap swaps via convertAssetToExchangeToken and Curve swaps via convertToStableCoin. UniswapAdapter and CurveswapAdapter do use Oracle for price estimation, but distributeYield calls use hard coded 5% SLIPPAGE, which is wide enough to mak...

Race condition in exercising the options

Lines of code Vulnerability details Impact Option buyers might never exercise their options with a bad actor vault owner, hence creating trust issues to the platform since the promised functionality causes only loss of funds. Proof of Concept Alice creates a vault for her high valued NFT. Bob buy...

Setting a high feeRate can block exercise or cause negative flow of funds

Lines of code Vulnerability details Impact When an admin intentionally or unintentionally sets a feeRate greater than 1e18 100%, The exercise function can fail with arithmetic operation underflow at line 289 In the case, when beneficiary is connected to multiple vaults, the exercise function will...

Loss of funds due to premiums and proceeds accruing to address(0) by default

Lines of code Vulnerability details Premiums and proceeds earned will accrue to the zero address by default, instead of the vault owner, and the funds will be irrecoverable. Proof of concept vaultBeneficiariesvaultId is not set during createVault, so any call to getVaultBeneficiary will return...

Fee-on-transfer / rebasing ERC20 cannot be used as underlying

Lines of code Vulnerability details Some ERC20 tokens may have fee-on-transfer or change balance without owner intervention. If these tokens are used as underlying in the protocol they can be lost. Proof of concept Alice creates a vault with a token that has a 1% fee on transfer. She sends...

function exercise might fail sporadically due to UI & transaction delays.

Lines of code Vulnerability details Impact The function exercise is checking for an exact match of msg.value for it to be successful. // check correct ETH amount was sent to pay the strike requiremsg.value == vault.currentStrike, "Incorrect ETH sent for strike"; There can be a time delay between...

call to harvest() can be manipulated to steal funds from the contract

Lines of code Vulnerability details Impact When calling harvest, attacker can device a cross-contract reentrancy to steal NFT owned by the contract Cally. Proof of Concept 1. While creating a vault, the owner of NFT transfers the NFT to the contract i.e. ownerOfnftID is Cally. 2. Attacker contrac...

YieldManger swap highly vulnerable to sandwich attacks

Lines of code Vulnerability details Impact Swap slippage of 5% allows malicious parties sandwich attack distributeYield swaps Proof of Concept Swap slippage of 5% allows significant extraction of funds from swaps via sandwich attack. The worst offender of this is the stable-stable swaps which...

TokenType mismatch with true type can be exploited to steal funds

Lines of code Vulnerability details A vault can be created for different TokenTypes, namely ERC20 and ERC721. These tokens have different logic, so a user needs to pass the type of the underlying token when creating a vault. At the end of createVault the user's tokens are pulled into the contract...

getDutchAuctionStrike () will return an unfavourable output for seller

Lines of code Vulnerability details Impact The protocol depends on the mentioned function decrementing to 0 or to the reserveStrikeif any is set by the seller of the option. After testing the efficiency of of getDutchAuctionStrike , it will return a value lower than the reserveStrike 2-3hrs prior...

Option buyers might loose funds due to vault owner's manipulation

Lines of code Vulnerability details Impact Option buyers might loose funds due to vault owner's manipulation Proof of Concept Alice creates a vault for her high valued NFT with a low price. Bob buys the option and sends premium. Alice buys the option from Bob by using an other EOA. Bob re-buys th...

[WP-H0] Fake balances can be created for not-yet-existing ERC20 tokens, which allows attackers to set traps to steal funds from future users

Lines of code Vulnerability details function createVault uint256 tokenIdOrAmount, address token, ... external returns uint256 vaultId ... Vault memory vault = Vault ... ; // vault index should always be odd vaultIndex += 2; vaultId = vaultIndex; vaultsvaultId = vault; // give msg.sender vault tok...

Loss of asset due to improperly setting dutchAuctionReserveStriked

Lines of code Vulnerability details The Option writer Victim believes they will be writing a call option, but ends up selling their asset at a discount. Proof of Concept If dutchAuctionReserveStrike is improperly set, the writer is effectively writing a call option that is immediately in the mone...

Strike price curve of getDutchAuctionStrike is not what users expect

Lines of code Vulnerability details Note: This submission contains links to a private fork of the contest repo. User code423n4 has been added as a collaborator in order to view. Impact Function getDutchAuctionStrike does not implement the function that Option buyers would expect. They probably...

Rebasing tokens lock excess balance in contract

Lines of code Vulnerability details Rebasing tokens lock excess balance in contract If a vault is created with a rebasing ERC20 as its token, additional balance accrued through rebases while the token is owned by the vault may be locked in the contract. If the rebasing token balance decreases whi...

Cally does not support ERC20 tokens with built-in fee

Lines of code Vulnerability details Impact Contract Cally does not properly handle ERC20 tokens that charge fee on their transfers. Implementation of such a tokens does not transfer exact amount provided to transfer but part of it is charged as a fee, burned or used in some other way. This leads ...

Add max fee in setFee and emit event

Lines of code Vulnerability details Impact Malicious owner can steal all ETH of a sell. Proof of Concept The function setFeeCallyNFT.sol is critical as it set the amount of ETH that the protocol will receive. A malicious owner can set the fee to 1e18 and all ETH after exercise will go to the owne...

lowerHasMinted() can be used by minters to bypass mintCeiling

Lines of code Vulnerability details Impact It is possible for any whitelisted used to reduce totalMinted for themselves. This value is used in mint to prevent a malicious minter from minting an infinite number of tokens. By allowing a minter to reduce their own totalMinted they are able to perfor...

Loss of funds due to beneficiary override to address(0) during transfer

Lines of code Vulnerability details Premiums or proceeds earned after the transfer will accrue to the zero address, instead of to the new vault owner, and the funds will be irrecoverable. Proof of concept vaultBeneficiariesvaultId is overridden to the zero address during transfer: File: Cally.sol...

Fee can possibly be set maliciously

Lines of code Vulnerability details Fee is set by an admin and can be set maliciously to steal the funds that are entitled to go to the user. Impact Fee can be set to a maliciously high value to unfairly extract funds from protocol users. An owner can buy options, set fee to 100% and exercise...

“No Revert on Failure“ tokens can be stolen from the contract by disguising them as NFTs.

Lines of code Vulnerability details Impact If a user creates a vault with an ERC20 token with the following two properties: 1. There’s a way to make transferFrom fail to transfer tokens without reverting e.g. some tokens like ZRX return false on failure instead of reverting. 2. fransferFromfrom,...

???

Lines of code ??? Vulnerability details Impact Detailed description of the impact of this finding. Proof of Concept Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept. Tools Used Recommended Mitigation Steps --- T...

Use ERC721.safertransferFrom instead of ERC721.transferFrom when you send out NFT

Lines of code Vulnerability details Impact Buyers can accidentally lose their NFT if they send to incorrect address. Proof of Concept When the buyer decide to call exercise the NFT is transfered using transferFrom. This is risky because if the destination msg.sender is a contract and it is unable...

Fee on transfer tokens block exercises and withdrawals

Lines of code Vulnerability details Fee on transfer tokens block exercises and withdrawals If a vault is created with a fee-on-transfer ERC20 as its token, the underlying asset may be locked in the contract. Scenario: Setup: 1. Alice calls createVault with a fee-on-transfer token address and 1000...

UNISWAP_FEE is hardcoded which will lead to significant losses compared to optimal routing

Lines of code Vulnerability details Impact In YieldManager, UNISWAPFEE is hardcoded, which reduce significantly the possibilities and will lead to non optimal routes. In particular, all swaps using ETH path will use the wrong pool as it will use the ETH / USDC 1% one due to this line. Proof of...

Payble function allows for Eth transfer even when ERC20 tokens are being used

Lines of code Vulnerability details Impact The function depositCollateral in GeneralVault.sol is payable. This needs to be payable for the case in LidoVault where depositToYeild expects ETH to be transferred. However for the ConvexCurveLPVault.sol and the case when LidoVault is not using Eth as a...

Reentrancy in LidoVault

Lines of code Vulnerability details Impact The withdrawFromYieldPool method is vulnerable to a reentry problem, it depends on the caller implementation and CurveswapAdapter.swapExactTokensForTokens in order to exploit it. Proof of Concept Use call instead of transfer to send ether. Because the...

Inefficiency in the Dutch Auction due to lower duration

Lines of code Vulnerability details The vulnerability or bug is in the implementation of the function getDutchAuctionStrike The AUCTIONDURATION is defined as 24 hours, and consider that the dutchAuctionReserveStrike or reserveStrike will never be set to 0 by user. Now if a vault is created with...

No cap on fee parameters

143 comment Warden: gzeon Consider adding caps to fee parameters to reduce rug risk --- The text was updated successfully, but these errors were encountered: All reactions...

createVault() does not verify ERC20 token contract exists before transferring

Lines of code Vulnerability details Impact There are no checks to ensure the the vault.token contract exists when creating a vault. Token address are deterministic in the EVM and can be known ahead of time. As a result it is possible for a user to call createVault with token set to a ERC20 contra...

Re-entance steal token

Lines of code Vulnerability details Impact An attacker can steal tokens from the protocol after it sent ETH to him. Proof of Concept 1.Alice create vault of some ERC20/NFT. and buy the option 2.Alice pay for itself, so ethBalanceAlice is greater than 1 3. Alice call harvest, and get eth. Alice...

Malicious ERC20 or ERC777 could be used as collateral

Lines of code Cally.solL296 Vulnerability details Impact A malicious ERC20 or ERC777 could be used as collateral and rug the exerciser on exercise through a malicious implementation of ERC20vault.token.safeTransfer. Proof of Concept Any ERC20/ERC777 token is accepted as there is no whitelist. Thi...

EthAssetManager and ThreePoolAssetManager don't control Meta tokens decimals

Lines of code Vulnerability details Both contracts treat meta assets as if they have fixed decimals of 18. Minting logic breaks when it's not the case. However, meta tokens decimals aren't controlled. If actual meta assets have any other decimals, minting slippage control logic of both contracts...

Consider remove setLpToken function

143 comment Warden: gzeon Owner can call setLpToken to change the value of lpToken in WhitelistPeriodManager, which will make all onlyLpNft function revert function setLpTokenaddress lpToken internal lpToken = ILPTokenlpToken; function setLpTokenaddress lpToken external onlyOwner setLpTokenlpToke...

Protocol does not support fee-on-transfer tokens

62 comment Warden: cmichel Protocol does not support fee-on-transfer tokens, see addLiquidity and LiquidityPool.depositErc20. The amount value is stored and available to be withdrawn later but the contract receives amount - fees. Note that there are tokenCheckstokenAddress and the protocol would...