Lacking slippage protection in RebalanceManagerV3

Handle p4st13r4 Vulnerability details Impact In RebalanceManagerV3 calls to UniswapV2-like router is lacking slippage protection exposing the rebalance process to front-running attacks. Proof of Concept Tools Used Editor Recommended Mitigation Steps Consider adding a minOutput to...

SingleTokenJoinV2 doesn't take input deadline to consideration

Handle kenzo Vulnerability details SingleNativeTokenExitV2 takes as input from the user a deadline for the trades. However, it does not use this input for the actual trade but sets the deadline to be block.timestamp. Impact Trades will not work as expected. User might set a deadline for the trade...

Attacker can break joinTokenSingle() by transferring basketToken to the contract

Handle WatchPug Vulnerability details uint256 outputAmount = outputToken.balanceOfaddressthis; require outputAmount == joinTokenStruct.outputAmount, "FAILEDOUTPUTAMOUNT" ; In the current implementation, joinTokenSingle requires balanceOf outputToken strictly equal to outputAmount in calldata...

Unsafe token transfer

Handle WatchPug Vulnerability details Calling ERC20.transfer without handling the returned value is unsafe. outputToken.transfermsg.sender, outputTokenBalance; Recommendation Consider using OpenZeppelin's SafeERC20 library with safe versions of transfer functions. --- The text was updated...

It might not be possible to withdraw tokens from the basket

Handle Czar102 Vulnerability details Impact When enough basket token owners exit, it will be impossible to exit pool with the last MINAMOUNT tokens because of this check. This will result in locking some tokens forever. Recommended Mitigation Steps Consider resigning from this check or performing...

SingleNativeTokenExitV2 only returns single output token

Handle cmichel Vulnerability details The SingleNativeTokenExitV2.exit function performs a list of arbitrary user-defined swaps on the exited token basket. These could result in many different final "output" tokens ending up in the contract after the swaps. However, the contract assumes that there...

BasketFacet.sol#joinPool() feeAmount should be considered when checking MAX_POOL_CAP_REACHED

Handle WatchPug Vulnerability details require totalSupply.addamount = this.getCap, "MAXPOOLCAPREACHED" ; uint256 feeAmount = amount.mulbs.entryFee.div1018; feeAmount should be considered. Otherwise, the new totalSupply may surpass pool cap. --- The text was updated successfully, but these errors...

Basket's max cap can be surpassed due to beneficiary entry fee

Handle kenzo Vulnerability details When joining a basket, the function verifies that the total supply + tokens the user asks to mint is smaller than the basket's max supply. However, this doesn't take into account the fact that additional tokens will be minted if there's an entry fee beneficiary...

Owner can lock any of basket tokens

Handle Czar102 Vulnerability details Impact Owner can remove any tokens, and since lock all funds that the contract has in a specific token. The owner may engineer the deletion in such way that afraid people will fear next locks and quit the basket, then the owner may add the token back, gaining...

Annualized fee APY dependence on the frequency of executing a function

Handle Czar102 Vulnerability details Impact The APY of the annualized fee is dependent on the frequency of the execution of the BasketFacet::chargeOutstandingAnnualizedFee. If it is called more frequently, the compounding is more frequent and the APY is higher. For less used baskets, the APY migh...

wrong withdraw

Handle certora Vulnerability details the comment says that the function should byrn user shares, but it also mints them. in addition, that function transfers tokens from msg.sender to addressthis, but it should be upside down. --- The text was updated successfully, but these errors were...

denial of service

Handle certora Vulnerability details the balance of outputToken is checked to be exactly joinTokenStruct.outputAmount. It is not recommeded and it's better to use = Impact The worst scenario is a denial of service in case there is already an amount of the output token in the contract. It is easil...

Unused ERC20 tokens are not refunded, and can be stolen by attacker

Handle WatchPug Vulnerability details Under certain circumstances, e.g. annualizedFee being minted to feeBeneficiary between the time user sent the transaction and the transaction being packed into the block and causing amounts of underlying tokens for each basketToken to decrease. It's possible ...

Unchecked ERC20 transfer call

Handle p4st13r4 Vulnerability details Impact Standard ERC20 tokens return a boolean when transfer method is called. A check should be done to account for ERC20 tokens that are not compliant to the standard. Proof of Concept Relevant lines: 3 Tools Used Editor Recommended Mitigation Steps Check...

ERC20 return values not checked

Handle cmichel Vulnerability details 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. See: SingleNativeTokenExitV2.exit's...

totalSupply may exceed LibBasketStorage.basketStorage().maxCap

Handle Czar102 Vulnerability details Impact Total supply of the token may exceed the maxCap introduced. This can happen when a user wants to join the pool. The check in BasketFacet::joinPool... includes only the base amount, without fee. Thus, if fee is on and someone will want to create as many...

Lock time is dependent on the average block time

Handle Czar102 Vulnerability details Impact Function BasketFacet::getLock... checks the lock based on the block number, so the time of the lock is dependent on average block time. Average block time doesn't have to be maintained by the protocol and is a subject to changes. Furthermore, the...

xToken Approvals Allow Spenders To Spend More Tokens

Handle leastwood Vulnerability details Impact The approve function has not been overridden and therefore uses xToken shares instead of the equivalent rebalanced amount, i.e. the underlying vault token amount. Proof of Concept The approved spender may spend more tokens than desired. In fact, the...

callFacet is based on unprotected calls

Handle 0x0x0x Vulnerability details callFacet is based on unprotected calls and user funds can get stolen using them. This is unsafe for users and at least this risk has to be better documented. --- The text was updated successfully, but these errors were encountered: All reactions...

Dishonest Stakers Can Siphon Rewards From xToken Holders Through The deposit Function In NFTXInventoryStaking

Handle leastwood Vulnerability details Impact xTokens is intended to be a representation of staked vault tokens. As the protocol's vaults accrue fees from users, these fees are intended to be distributed to users in an inconsistent fashion. NFTXInventoryStaking is one of the ways users can stake...

Owner of the BasketFacet can cause DoS for exitPool(...) function

Handle Czar102 Vulnerability details Impact Provided an owner can add any tokens to the basket, it may add a token, for which the owner can burn tokens from any account. Then, after adding the token to the basket, the owner may burn tokens so that token.balanceofBasketFacet will be below MINAMOUN...

_mint721() function can be broken because of strict equality check on token balance

Handle jayjonah8 Vulnerability details Impact In NFTXMarketplaceZap.sol the mint721 function has a require check that the balance must be equal to the amount of vault tokens in the contract. uint256 balance = count BASE - count INFTXVaultvault.mintFee; requirebalance ==...

SHOULD CHECK RETURN DATA FROM CHAINLINK AGGREGATORS

Handle defsec Vulnerability details Impact The latestRoundData function in the contract PriceFeed.sol fetches the asset price from a Chainlink aggregator using the latestRoundData function. However, there are no checks on roundID. Stale prices could put funds at risk. According to Chainlink's...

NFTXMarketplaceZap Performs A Dangerous Equality Check Which Can Brick Contract

Handle leastwood Vulnerability details Impact NFTXMarketplaceZap provides an interface for users to interact with NFTX vaults by buying, selling and swapping ERC1155/ERC721 tokens. The mint721 and mint1155 functions perform strict equality checks on the vaults token balance for the...

NFTXStakingZap Performs A Dangerous Equality Check Which Can Brick Contract

Handle leastwood Vulnerability details Impact NFTXStakingZap provides an interface for users to interact with NFTX vaults by adding ERC1155/ERC721 tokens as liquidity. The addLiquidity721WETH and addLiquidity1155WETH functions perform strict equality checks on the vaults balance for the...

No access control on assignFees() function in NFTXVaultFactoryUpgradeable contract

Handle ych18 Vulnerability details In If the Vault owner decides to set factoryMintFee and factoryRandomRedeemFee to zero, any user could call the function NFTXVaultFactoryUpgradeable.assignFees and hence all the fees are updated. --- The text was updated successfully, but these errors were...

User funds could be stuck

Handle csanuragjain Vulnerability details Impact Although chances are rare but user can be denied to exit pool Proof of Concept 1. If user decides to exit pool and any one token has MINAMOUNT then transaction will revert and user wont be able exit until that token has sufficient balance. This mea...

sendAllocatedYETI() can be called by anyone

Handle jayjonah8 Vulnerability details Impact In TeamAllocation.sol, the sendAllocatedYETI function simply distributes YETI to the team. This is a transfer of value and it currently can be called by anyone as the onlyTeam modifier is not used here. Proof of Concept Tools Used Manual code review...

receiveCollateral() can be called by anyone

Handle jayjonah8 Vulnerability details Impact In StabilityPool.sol, the receiveCollateral function should be called by ActivePool per comments, but anyone can call it passing in tokens and amounts args to update stability pool balances. Proof of Concept Tools Used Manual code review Recommended...

User can lose Funds

Handle csanuragjain Vulnerability details Impact User funds lost Proof of Concept 1. Navigate to contract at 2. User joins a pool having 5 tokens in the basket by using joinPool function 3. Owner decides to remove 1 of the token from the basket 4. User exits the pool by calling exitPool function...

Contract can lose funds

Handle csanuragjain Vulnerability details Impact Fund loss Proof of Concept 1. Navigate to contract at 2. Assume owner created a Basket with 1 token A which is 0.1 USD 3. User joins the pool using joinPool function 4. Owner removes the token so that basket becomes empty and adds another token B...

Token mint without transfer

Handle csanuragjain Vulnerability details Impact User will be minted token without transferring any amount Proof of Concept 1. Owner has removed all tokens from the basket using removeToken function. So bs.tokens.length=0 2. User joins the pool using joinPool function. Now below happens: a. Since...

No checks in setAddresses() functions

Handle jayjonah8 Vulnerability details Impact Since the setAddresses functions can only be called once during deployment and passes in many addresses which are set in storage, its important to have safety checks ensuring that there are no duplicate addresses which can easily happen while passing...

buyAndSwap1155WETH() function may cause loss of user assets

Handle cccz Vulnerability details Impact In the NFTXMarketplaceZap.sol contract, the buyAndSwap1155WETH function uses the WETH provided by the user to exchange VaultToken, but when executing the buyVaultToken method, msg.value is used instead of maxWethIn. Since msg.value is 0, the call will fail...

buyAndSwap1155WETH() function does not work

Handle cccz Vulnerability details Impact The buyAndSwap1155WETH function uses the WETH provided by the user to exchange VaultToken, but when executing the buyVaultToken method, msg.value is used instead of maxWethIn. Since msg.value is 0, the call will fail. function buyAndSwap1155WETH uint256...

Users Can Siphon AaveYield Rewards By Depositing And Withdrawing Collateral

Handle leastwood Vulnerability details Impact The AaveYield contract provides users with the option to choose Aave as their strategy of choice to generate yield. Users will make deposits to this strategy via the SavingsAccount contract. Upon deposit, shares are minted at a 1:1 exchange rate and t...

No use of reentrency guards

Handle jayjonah8 Vulnerability details Impact The README.md makes the point that reentrancy attacks are a cause for concern, but the protocol makes no use of reentrancy guards in any file or in the functions users interact with. Proof of Concept The are no Reentrancy guards in the entire code bas...

_safeJoeTransfer doesn't refund users JOE incase of rounding error

Handle jayjonah8 Vulnerability details Impact In WJLP.sol a user can call the claimReward function to claim the JOE rewards they are owed. This eventually calls the safeJoeTransfer function which will check if the amount to send is greater than the joeBal of the contract. If the amount is greater...

_from and _to can be the same address on wrap() function

Handle jayjonah8 Vulnerability details Impact In WJLP.sol, the wrap function pulls in amount base tokens from from, then stakes them to mint WAssets which it sends to to. It then updates rewardOwner's reward tracking such that it now has the right to future yields from the newly minted WAssets. B...

Constructor does not ensure that addresses are not the same

Handle jayjonah8 Vulnerability details Impact In WJLP.sol, 3 addresses are passed in setting the MasterChefJoe, JLP, and JOE addresses in storage. There are no checks ensuring that these addresses are different from one another. This means that during deployment, a mistake can easily be made of...

Aave's share tokens are rebasing breaking current strategy code

Handle cmichel Vulnerability details When depositing into Aave through the AaveYield.lockTokens contract strategy, one receives the sharesReceived amount corresponding to the diff of aToken balance, which is just always the deposited amount as aave is a rebasing token and 1.0 aToken = 1.0...

ChainLink price data could be stale

Handle cmichel Vulnerability details There is no check in ChainlinkOracle.sync if the oracle return values indicate stale data. This could lead to stale prices according to the Chainlink documentation: under current notifications: "if answeredInRound roundId could indicate stale data." under...

No checks if given product is created by the factory

Handle 0x0x0x Vulnerability details An attacker can create a fake product. Collateral contract does not check whether the given product is created by the factory. A malicious product can return arbitrary maintenance amounts, therefore they can make any deposit to fake product stuck simply return...

Incorrect safeApprove usage

Handle Jujic Vulnerability details Impact safeApprove won't work when current allowance 0, there may be cases when strategies will not use all allowance, so switch to new strategy will be blocked. Proof of Concept Tools Used Recommended Mitigation Steps I recommend approving to zero and then...

No slippage tolerance checks during swap functions opens up flashloan attacks and price manipulation

Handle jayjonah8 Vulnerability details Impact With functions like joinTokenSingle in SingleTokenJoin.sol that handle swapping with uniswapV2 like exchanges there are no slippage tolerance checks in place to prevent flashloan attacks and price manipulation. Proof of Concept Tools Used Manual code...

withdrawTo Does Not Sync Before Checking A Position's Margin Requirements

Handle leastwood Vulnerability details Impact The maintenanceInvariant modifier in Collateral aims to check if a user meets the margin requirements to withdraw collateral by checking its current and next maintenance. maintenanceInvariant inevitably calls AccountPosition.maintenance which uses the...

Missing payable

Handle robee Vulnerability details The following functions are not payable but uses msg.value - therefore the function must be payable. This can lead to undesired behavior. CreditLine.sol, depositCollateral is payable but doesn't use msg.value CreditLine.sol, repay is payable but doesn't use...

Failed transfer with low level call could be overlooked

Handle harleythedog Vulnerability details Impact There are several places where low level call is used within the contract. In particular, in SavingsAccount.sol, the external withdraw function has the to argument, which is eventually used in this code in transfer: bool success, = to.callvalue:...

Yearn token <> shares conversion decimal issue

Handle cmichel Vulnerability details The yearn strategy YearnYield converts shares to tokens by doing pricePerFullShare shares / 1e18: function getTokensForSharesuint256 shares, address asset public view override returns uint256 amount if shares == 0 return 0; // @audit should divided by...

DoS when depositing collateral

Handle cmichel Vulnerability details The Pool.depositCollateral and Pool.addCollateralInMarginCall functions require the caller to have a pool token balance of zero. function depositCollateraluint256 amount, bool transferFromSavingsAccount external payable override requireamount != 0, 'DC1'; //...