Return variable can remain unassigned in _sendForReceiver

Handle sirhashalot Vulnerability details Impact The sendForReceiver function only sets a return function in the "if" code block, not the "else" case. If the "else" case is true, no value is returned. The result of this oversight is that the sendForReceiver function called from the distribute...

Attacker can break addLiquidity721() by transferring vaultToken to the contract

Handle WatchPug Vulnerability details function addLiquidity721WETH uint256 vaultId, uint256 memory ids, uint256 minWethIn, uint256 wethIn, address to internal returns uint256, uint256, uint256 address vault = nftxFactory.vaultvaultId; requirevault != address0, "NFTXZap: Vault does not exist"; //...

Use safeTransfer/safeTransferFrom consistently

Handle sirhashalot Vulnerability details Impact A transfer function can fail without reverting. To prepare for such situations, a contract should either check the return value of the transfer function or use a solution such as Open Zeppelin's SafeTransfer function. Only a few places in this proje...

NFTXSimpleFeeDistributor#distribute() Wrong implementation of NFTXSimpleFeeDistributor#_sendForReceiver can potentially cause receivers to get wrong amounts of tokens

Handle WatchPug Vulnerability details function sendForReceiverFeeReceiver memory receiver, uint256 vaultId, address vault, uint256 amountToSend internal virtual returns bool if receiver.isContract IERC20Upgradeablevault.approvereceiver.receiver, amountToSend; // If the receive is not properly...

A vault can be locked from MarketplaceZap and StakingZap

Handle p4st13r4 Vulnerability details Impact Any user that owns a vToken of a particular vault can lock the functionalities of NFTXMarketplaceZap.sol and NFTXStakingZap.sol for everyone. Every operation performed by the marketplace, that deals with vToken minting, performs this check:...

NFTXMarketplaceZap and NFTXStakingZap don't check WETH.transfer result

Handle hyh Vulnerability details Impact Transactions will not be reverted on failed transfer, setting system state as if it was successful. User will have the remainder funds frozen within the system. Proof of Concept NFTXStakingZap and NFTXMarketplaceZap perform unchecked transfers in several...

Unchecked return value for token.transfer call

Handle WatchPug Vulnerability details It is usually good to add a require-statement that checks the return value or to use something like safeTransfer; unless one is sure the given token reverts in case of a failure. Instances include: IERC20Upgradeablevault.transferto, minTokenIn-amountToken;...

Missing Slippage Protection

Handle robee Vulnerability details Missing slipage protection may lead to losing assets while swapping them. Without slipage protection the swapper is allowed to give much less worth of target tokens than it should in a fair swap. to Missing slippage protection at: no slippage protection at swap ...

VaderPoolV2.mintFungible exposes users to unlimited slippage

Handle TomFrenchBlockchain Vulnerability details Impact Frontrunners can extract up to 100% of the value provided by LPs to VaderPoolV2 as fungible liquidity. Proof of Concept Users can provide liquidity to VaderPoolV2 through the mintFungible function. This allows users to provide tokens in any...

No way to remove GasThrottle from VaderPool after deployment

Handle TomFrenchBlockchain Vulnerability details Impact Potential DOS on swaps on VaderPool Proof of Concept BasePool makes use of a validateGas modifier on swaps which checks that the user's gas price is below the value returned by FASTGASORACLE. Should FASTGASORACLE be compromised to always...

Out of gas.

Handle Jujic Vulnerability details There is no upper limit on poolColl.tokens, it increments each time when a new collateral is added. Eventually, as the count of collateral increases, gas cost of smart contract calls will raise and that there is no implemented function to reduce the array size...

VaderReserve.reimburseImpermanentLoss improperly converts USDV to VADER

Handle TomFrenchBlockchain Vulnerability details Impact IL isn't properly converted from being in terms of USDV to VADER, resulting in reserve paying out incorrect amount. Proof of Concept VaderReserve.reimburseImpermanentLoss receives an amount in terms of USDV and converts this to an amount of...

USDV uses incorrect exchange rate for burning

Handle TomFrenchBlockchain Vulnerability details Impact A money pump exists whenever VADER is worth more than $1. Proof of Concept When minting USDV, the amount minted is uAmount = vPrice vAmount / 1e18 where vPrice is the price of VADER in terms of USD. When burning USDV, the amount of VADER...

Anybody can claim JLP tokens approved to WJLP

Handle kenzo Vulnerability details WJLP's wrap takes from and to as parameter, and doesn't check that msg.sender=from. This means that anybody can claim to himself tokens that a user approved for WJLP. Impact Loss of user funds. Proof of Concept The problem is in the wrap function. Code ref...

Council veto protection does not work

Handle TomFrenchBlockchain Vulnerability details Impact Council can veto proposals to remove them to remain in power. Proof of Concept The Vader governance contract has the concept of a "council" which can unilaterally accept or reject a proposal. To prevent a malicious council preventing itself...

VaderRouter exposes LPs to unlimited slippage on joins

Handle TomFrenchBlockchain Vulnerability details Impact LPs using VaderRouter are subject to incurring unlimited slippage due to manipulation of the pool's reserves. Proof of Concept This finding is similar to finding "LPs are subject to unlimited slippage on joins" however that applies to...

NFTXSimpleFeeDistributor._sendForReceiver doesn't return success if receiver is not a contract

Handle hyh Vulnerability details Impact Double spending of fees being distributed will happen in favor of the first fee receivers in the feeReceivers list at the expense of the last ones. As sendForReceiver doesn't return success for completed transfer when receiver isn't a contract, the...

WJLP will continue accruing rewards after user has unwrapped his tokens

Handle kenzo Vulnerability details WJLP doesn't update the inner accounting for JOE rewards when unwrapping user's tokens. The user will continue to receive rewards, on the expanse of users who haven't claimed their rewards yet. Impact Loss of yield for users. Proof of Concept The unwrap function...

VaderRouterV2 exposes LPs to unlimited slippage on joins

Handle TomFrenchBlockchain Vulnerability details Impact LPs using VaderRouterV2 are subject to incurring unlimited slippage due to manipulation of the pool's reserves. Proof of Concept This finding is similar to finding "VaderRouter exposes LPs to unlimited slippage on joins" however that applies...

WJLP loses unclaimed rewards when updating user's rewards

Handle kenzo Vulnerability details After updating user's rewards in userUpdate, if the user has not claimed them, and userUpdate is called again eg. on another wrap, the user's unclaimed rewards will lose the previous unclaimed due to wrong calculation. Impact Loss of yield for user. Proof of...

Reward not transferred correctly

Handle csanuragjain Vulnerability details Impact Monetary loss for user Proof of Concept 1. Navigate to contract at 2. Let us see sendJoeReward function function sendJoeRewardaddress rewardOwner, address to internal // harvests all JOE that the WJLP contract is owed MasterChefJoe.withdrawpoolPid,...

Duplicate Collateral could cause financial instability

Handle csanuragjain Vulnerability details Impact Duplicate collaterals can be added which makes getValidCollateral return duplicate items. This impacts all function which uses getValidCollateral function like getPendingCollRewards, which will now calculate the pending reward twice for the duplica...

Minting and burning synths exposes users to unlimited slippage

Handle TomFrenchBlockchain Vulnerability details Impact LPs remaining in VaderPool end up paying all transfer fees for LPs which leave the pool. Proof of Concept See VaderPoolV2.mintSynth: Here a user sends nativeDeposit to the pool and the equivalent amount of foreignAsset is minted as a synth t...

Unwhitelisted token can cause disaster

Handle csanuragjain Vulnerability details Impact Contract instability and financial loss. This will happen if one of the allowed contract calls sendCollaterals with non whitelisted token may happen with user input on allowed contract Proof of Concept 1. Navigate to contract at 2. Assume...

XTokenUpgradeable burnXTokens doesn't check baseToken transfer result

Handle hyh Vulnerability details Impact Transaction will not be reverted on failed transfer, setting system state as if it was successful. User will have the remainder funds frozen within the system. Proof of Concept burnXTokens does the baseToken transfer after burning the shares: Recommended...

ActivePool does not update rewards before unwrapping wrapped asset

Handle kenzo Vulnerability details When ActivePool sends collateral which is a wrapped asset, it first unwraps the asset, and only after that updates the rewards. This should be done in opposite order. As a comment in WJLP's unwrapFor rightfully mentions - "Prior to this being called, the user...

Oracle can be manipulted to consider only a single pair for pricing

Handle TomFrenchBlockchain Vulnerability details Impact Loss of resilience of oracle to a faulty pricing for a single pair. Proof of Concept In the oracle we calculate the TVL of each pool by pulling the reserves and multiplying both assets by the result of a supposedly manipulation resistant...

Unused WJLP can't be simply unwrapped

Handle kenzo Vulnerability details WJLP can only be unwrapped from the Active Pool or Stability Pool. A user who decided to wrap his JLP, but not use all of them in a trove, Wouldn't be able to just unwrap them. Impact Impaired functionality for users. Would have to incur fees for simple...

LPs of VaderPoolV2 can manipulate pool reserves to extract funds from the reserve.

Handle TomFrenchBlockchain Vulnerability details Resubmission as the form crashed apologies if this is a duplicate Impact Impermanent loss protection can be exploited to drain the reserve. Proof of Concept In VaderPoolV2.burn we calculate the current losses that the LP has made to impermanent los...

Zap contracts vulnerable to DoS

Handle shenwilly Vulnerability details Impact If the zap contract has a non-zero balance of vault token then users won't be able to mint that particular vault. This is because the minting functions only check if the minted token is equal to the current balance inside the contract, which will alwa...

SafeERC20 Is Not Used On Token Transfers

Handle leastwood Vulnerability details Impact The transfer function is used on WETH and vault token transfers over OpenZeppelin's SafeERC20 library. As a result, these function may return unintended results which are not handled correctly. The SafeERC20 library ensures transfer and transferFrom...

WJLP._safeJoeTransfer should return amount actually sent

Handle hyh Vulnerability details Impact User's rewards can be partial lost as system will write down all current rewards as transferred, while actual transfer amount can be smaller. WJLP will be still accounted for the remaining part of rewards by Trader Joe, but there will be no link to the user...

The return value of the _sendForReceiver function is not set, causing the receiver to receive more fees

Handle cccz Vulnerability details Impact In the NFTXSimpleFeeDistributor.sol contract, the distribute function is used to distribute the fee, and the distribute function judges whether the fee is sent successfully according to the return value of the sendForReceiver function. function...

Yeti token rebase checks the additional token amount incorrectly

Handle hyh Vulnerability details Impact The condition isn't checked now as the whole balance is used instead of the Yeti tokens bought back from the market. As it's not checked, the amount added to effectiveYetiTokenBalance during rebase can exceed the actual amount of the Yeti tokens owned by th...

WJLP contract doesn't check for JOE and JLP token transfers success

Handle hyh Vulnerability details Impact Transactions will not be reverted on failed transfer call, setting system state as if it was successful. This will lead to wrong state accounting down the road with a wide spectrum of possible consequences. Proof of Concept safeJoeTransfer do not check for...

Malicious receiver can make distribute function denial of service

Handle cccz Vulnerability details Impact In the NFTXSimpleFeeDistributor.sol contract, the distribute function calls the sendForReceiver function to distribute the fee function distributeuint256 vaultId external override virtual nonReentrant requirenftxVaultFactory != address0; address vault =...

PriceFeed ignores ChainLink roundId and will treat stale price as fresh

Handle hyh Vulnerability details Impact Stale 'carried over' price can be used for liquidations. This can cause various types of malfunctions and manipulated liquidations. For example, if a portfolio consists of two inversely correlated assets, which move in opposite directions most of the times,...

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

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

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

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

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

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

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

unchecked return value from transfer()

Handle JMukesh Vulnerability details Impact transfer does not check the return , due which transfer may get failed without reverting . some tokens does not revert on failure , they just return false Proof of Concept Tools Used manual review Recommended Mitigation Steps use safetransferFrom from o...

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

Dos in callFacet.call()

Handle JMukesh Vulnerability details Impact In function call address memory targets, bytes memory calldata, uint256 memory values if any one of the address is contract and implemented revert in its fallback , then all other transaction will get failed due this one transation Proof of Concept Tool...

Frontrunning attack via swap token functionality

Handle thankyou Vulnerability details Impact Amun utilizes both Pangolin and Uniswap's Routers to swap tokens within a given pair. One of the router functions used by Amun is swapExactTokensForTokens. Amun provides this function several arguments that the Router contract then utilizes to commence...

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

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