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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

Oracle doesn't calculate USDV/VADER price correctly

Handle TomFrenchBlockchain Vulnerability details Impact Invalid values returned from oracle for USDV and VADER prices in situations where the oracle uses more than one foreign asset. Proof of Concept The USDV price is calculated as so for simplicity we'll consider a two pairs: totalUSD =...

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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