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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

User created baskets are owned by defaultController

Handle 0x0x0x Vulnerability details User created baskets are owned by defaultController and help from the defaultController is needed to use any functionality or setup the basket. In test simply one account is used, but when a user wants to create a basket, they can’t control it properly...

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

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

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

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

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

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