Improper design/implementation of SingleTokenJoinV2#joinTokenSingle() make it prone to fail

Handle WatchPug Vulnerability details for uint256 i; i bs.tokens.length; i++ IERC20 token = bs.tokensi; uint256 tokenAmount = balanceaddresstoken.mulamount.addfeeAmount.div totalSupply ; requiretokenAmount != 0, "AMOUNTTOOSMALL"; token.safeTransferFrommsg.sender, addressthis, tokenAmount; // If...

SingleNativeTokenExitV2 assumes first exchange holds the outputToken

Handle kenzo Vulnerability details SingleNativeTokenExitV2 allows the user to exit and execute trades via multiple exchanges. When finishing the trades and sending a single output token back to the user, the contract takes that token from the last swap in the first exchange's trades. There is...

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

totalSupply may go above this.getCap()

Handle gzeon Vulnerability details Impact Pool cap is checked in L154 require totalSupply.addamount = this.getCap, "MAXPOOLCAPREACHED" ; but since we mint amount to the user and some % of feeAmount to Beneficiary, totalSupply can actually go above the defined cap. Recommended Mitigation Steps...

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

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

Tokens can be sent directly

Handle pauliax Vulnerability details Impact Token join contracts check that the final outputAmount is equal to joinTokenStruct.outputAmount: uint256 outputAmount = outputToken.balanceOfaddressthis; require outputAmount == joinTokenStruct.outputAmount, "FAILEDOUTPUTAMOUNT" ; While these contracts...

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

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

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

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

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

Unchecked return value from low-level call()

Handle JMukesh Vulnerability details Impact The return value of the low-level call is not checked, so if the call fails, the Ether will be locked in the contract. If the low level is used to prevent blocking operations, consider logging failed calls. Proof of Concept Tools Used manual review...

Creator of pie can mint any amount of _initialSupply, and drain underlying tokens via exitPool

Handle hubble Vulnerability details Impact The Creator of pie or msg.sender of bakePie in PieFactoryContract, can set any high value of initialSupply and get the ERC20 tokens minted. There is no relation to the intialSupply and the amount of underlying Tokens added to the Pie during the bakePie...

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

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

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

It is possible to "uninitialize" ERC20Facet contract

Handle Czar102 Vulnerability details Impact The initialization status is defined by the name and symbol. It is possible it set them back to an empty string, uninitializing the contract and letting the initialize.. function be called again. This way, the owner may, for example, hide minting...

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

With protectedCall permission funds of other users can be stolen

Handle 0x0x0x Vulnerability details When the diamond has BasketFacet, then following attack is possible: 1- Create a new ERC20 token A with a lot of quantity 2- Lock the basket 3- Send MINAMOUNT many token A to the basket 4- Remove all tokens except token A 5- Unlock the basket 6- Call joinPool a...

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

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

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

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

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

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

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

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

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

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

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

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

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

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

Wrong returns of SavingsAccountUtil.depositFromSavingsAccount() can cause fund loss

Handle WatchPug Vulnerability details The function SavingsAccountUtil.depositFromSavingsAccount is expected to return the number of equivalent shares for given asset. / @notice internal function used to get amount of collateral deposited to the pool @param fromSavingsAccount if true, collateral i...

It is possible to liquidate not existing account

Handle 0x0x0x Vulnerability details It is possible to liquidate an address for any product when collateral = maintenance = 0. So in other words, if a user have never used a product and deposited collateral, the user can get liquidated blacklisted from the product by anyone, since a liquidated...

In CreditLine#liquidate, lender doesn't receive _borrowTokens, when it is ETH

Handle 0x0x0x Vulnerability details When borrowAsset == address0, the liquidator sends ethereum via transaction value msg.value, but borrowTokens amount is not sent to the lender. Therefore, lender losses funds. As seen in: Mitigation step: Add bool success, = lender.callvalue: borrowTokens''; at...

Incentivizer rewards are unclear and can be more than allotted?

Handle cmichel Vulnerability details Note that the Incentivizer.create allocates programInfo.amount.sum tokens to be rewarded. It's unclear how the rewards that are actually paid out see Program.unsettled over the duration relate to this value. The ProgramInfo.amountPerShare returns the...

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'; //...

Wrong implementation of AaveYield.sol causing users to lose yields

Handle WatchPug Vulnerability details The current implementation of AaveYield.sol is taking AAVE aToken as a share token eg, cToken and yToken. However, AAVE's aTokens are quite different from cToken and yToken as it's always 1:1 to the underlying token, and the holder's balance will keep changin...

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

Ether can be locked in the PoolFactory contract without a way to retrieve it

Handle broccolirob Vulnerability details If a borrower calls the createPool function with a non-zero value, but also includes an ERC20 token address for collateralToken, then the Ether value sent will be locked in the PoolFactory contract forever. createPool L260-317 In the createPool function, a...

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

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