Non-transferable critical privileged role

Handle gzeon Vulnerability details Impact DEPLOYER is a constant in Manager and it is the only role that can call setSherlockCoreAddress to change sherlockCore address. Consider this is a critical function and there might be a need to change the deplorer address in the future e.g. governance...

Big deposits will revert

Handle pedroais Vulnerability details Impact Big deposits will revert Proof of Concept If a deposit is greater than the total deposited the transaction will revert. In this line of code totalTokenBalanceStakers - amount is computed. If amount is bigger than TVL staking will revert. I consider thi...

Owner can set arbitrary premium which allow nonStakers drain funds

Handle wuwe1 Vulnerability details Impact Owner can set arbitrary premium, this will cause protocol lose all the activeBalance, stakers lose all the claimable premium and nonStakers can drain all the usdc. Proof of Concept setProtocolPremium does not check the value of premium. premium can be...

DoS and stealing users' USDC

Handle OriDabush Vulnerability details Sherlock.sol An attacker can DoS the system and steal user's USDC if he manages to stake his USDC first i.e. minting token ID 1. It can be done by calling the initialStake with every amount let's say amount = 1 for example. Let's assume the lock period is...

No check that _baseToken or _quoteToken address is not address(this)

Handle jayjonah8 Vulnerability details Impact In ExchangeFactory.sol the createnewExchange function takes in the addresses baseToken and quoteToken. There are require checks to make sure that these are not zero addresses but no require checks to ensure that either of these addresses are not...

Potential Reentrancy at multiple places

Handle 0v3rf10w Vulnerability details Impact Potential Reentrancy at multiple places Proof of Concept L2Migrator.finalizeMigrateDelegatorIMigrator.MigrateDelegatorParams contracts/L2/gateway/L2Migrator.sol130-188: L1Escrow.approveaddress,address,uint256 contracts/L1/escrow/L1Escrow.sol21-28...

Griefing attack on migrateLPT can prevent token transfer to L2

Handle harleythedog Vulnerability details Impact In L1Migrator.sol the function migrateLPT can be called by anyone. A malicious user can call migrateLPT with a small maxSubmissionCost argument to intentionally make the retryable ticket creation fail. This will lock the LPT in the L1 escrow. Now,...

L2Migrator allows a user to migrate once through claimStake() and once through finalizeMigrateDelegator()

Handle Ruhum Vulnerability details Impact There are two ways to migrate from L1 to L2. Either through the cross-chain or the snapshot migration, as specified here But, a user is able to migrate twice by using both options. Proof of Concept The issue is that the migratedDelegator map is not used...

L1Migrator.migrateETH can be used to take away protocol's access to funds

Handle Ruhum Vulnerability details Impact The L1Migrator.migrateETH function can be called by anyone. It pulls all the ETH from the BridgeMinter contract and starts the process of moving the funds to L2. First of all, this function is only executable once. The RetryableTicket created with the fir...

L1LPTGateway.sol does not make use of safeTransferFrom

Handle jayjonah8 Vulnerability details Impact In the L1LPTGateway.sol transferFrom is used in several parts of the file. Tokens that don’t correctly implement the latest EIP20 spec will be unusable in the protocol as they revert the transaction because of the missing return value. Proof of Concep...

[WP-H24] Wrong design/implementation of permission control allows malicious/compromised Registry or Factory admin to steal funds from users' wallet balances

Handle WatchPug Vulnerability details The current design/implementation allows a market address registered on registry to call VaultaddValue and transfer tokens from an arbitrary address to a specified beneficiary up the approved amount at any time, and the beneficiary can withdraw the funds by...

Expired insurance status set incorrectly after unlock of funds

Handle ye0lde Vulnerability details Impact Expired insurance status set incorrectly after unlock of funds The insurance status is not set to false and the unlock function can be called over and over driving the lockedAmount to 0. The distorted lockedAmount will then cause liquidity and utilizatio...

the first depositor to an index can drain all users

Handle danb Vulnerability details if there is no liquidity in the pool, the first deposit determines the total liquidity, if the amount is too small the minted liquidity for the next liquidity providers will round down to zero. Impact An attacker can steal all money from liquidity providers. Proo...

applyCover() Does Not Enforce Index Market Lock

Handle leastwood Vulnerability details Impact The applyCover function is called by the insurance pool owner and intends to store data related to an insurance incident. Upon function execution, applyCover iterates over all available index markets and calls lock, denying all deposits and withdrawal...

Index compensate is 0 when totalLiquidity() is enough to cover the whole amount

Handle pauliax Vulnerability details Impact In IndexTemplate, function compensate, When amount value, and = totalLiquidity, the value of compensated is not set, so it gets a default value of 0: if value = amount ... compensated = amount; else ... if totalLiquidity amount ... compensated = value +...

Vault: Reduce reliance on manual rebalances from strategy to vault

Handle hickuphh3 Vulnerability details Impact Withdrawals are processed solely with funds that are held by the vault. Should there be insufficient liquidity Eg. many withdrawals in a short time, users have to rely on a trusted party operator to move funds from the investment strategy to the vault...

Signature replay

Handle 0x1f8b Vulnerability details Impact Signature replay in PoolTemplate. Proof of Concept The redeem method of PoolTemplate verifies the data stored in incident, and the verification logic of this process is performed as following: require MerkleProof.verify merkleProof, targets, keccak256...

No slippage protection on _swapUnderlyingToUst can lead to lost funds

Handle harleythedog Vulnerability details Impact The function swapUnderlyingToUst exists to swap underlying tokens to Ust. The last argument to exchangeunderlying is mindy, which specifies the minimum number of Ust to be returned from the swap. Currently, this value is set to 0, so the function i...

Unsafe mint is a reentrancy door

Handle pedroais Vulnerability details Impact Dangerous external calls in the middle of various state changes could cause reentrancy issues since there is no reentrancy guard in any functions. Proof of Concept When users call the deposit or sponsor functions a deposit NFT is minted. The safeMint...

No slippage tolerance set in NonUSTStrategy

Handle palina Vulnerability details Impact The exchange performed in NonUSTStrategy.sol via Curve is executed with "0" as the minimum amount received as the result of the operation, which is likely to be exploited by front-running and may lead to the loss of funds. Proof of Concept...

Both _mint and _safeMint are used to mint NFTs

Handle palina Vulnerability details Impact Both mint and safeMint functions are used to mint ERC721 NFTs in Claimers and Depositors, respectively. The usage of the mint is, however, discouraged by the used ERC721 implementation see PoC section, in favor of its safe counterpart. Proof of Concept...

Manipulation of the Y State Results in Interest Rate Manipulation

Handle Rhynorater Vulnerability details Impact Due to lack of constraints on user input in the TimeswapPair.solmint function, an attacker can arbitrarily modify the interest rate while only paying a minimal amount of Asset Token and Collateral Token. Disclosure: This is my first time attempting...

Claimer can reenter contract on deposit withdrawal

Handle kenzo Vulnerability details Upon withdrawal of deposit, the claimer will be called with onDepositBurned. This happens after the claimer shares have been updated, but before the underlying has been sent away from the contract. Therefore the claimer can reenter the contract, at an intermedia...

investedAssets ignores fees and can cause insolvency

Handle danb Vulnerability details investedAssets doesn't substract the fees owed to the treasury, this makes the system think that it has more than it really has. Proof of Concept consider the following scenario: perfFeePct is 20%. the system generated 1M dollars yield in aust that it didn't rede...

timeswap testnet

Handle 0x1f8b Vulnerability details Error: java.lang.IndexOutOfBoundsException: Index: 204, Size: 45 --- The text was updated successfully, but these errors were encountered: All reactions...

Unbounded loop on array controlled by owner can lead to DoS

Handle robee Vulnerability details A malicious attacker that is also a protocol owner can push unlimitedly to an array, that some function loop over this array. If increasing the array size enough, calling the function that does a loop over the array will always revert since there is a gas limit...

Convenience contract fails to function if asset or collateral is an ERC20 token with fees

Handle Ruhum Vulnerability details Impact There are ERC20 tokens that collect fees with each transfer. If the asset or collateral used in a pair is of that type, the Convenience contract fails to function. It always sends the flat amount specified in the function's parameter. If the token collect...

Its possible to have different bonusMultiplier for same lock duration

Handle hubble Vulnerability details Impact The owner has previledge to change the bonusMultipler for a duration anytime, using the setLockPeriods function. So, its possible to have different bonusMultiplier for different users for same lock duration, which may not be desirable from the protocol...

NFT token id repeated

Handle MaCree Vulnerability details Impact 1. merge funtion may lead to create repeated NFT token id, so user can not lock XDEFI Proof of Concept run the test case below please beforeEachasync = god, account1, account2, account3 = await ethers.getSigners; XDEFI = await await await...

No guard against zero address can possibly break functions with onlyOwner modifier

Handle jayjonah8 Vulnerability details Impact In XDEFIDistribution.sol the proposeOwnership function should check that the newOwner arg is not a zero address because one error can break every function in the contract that uses the onlyOwner modifier because if the owner is set to the zero address...

Iterating over large count numbers can cause out of gas errors

Handle jayjonah8 Vulnerability details Impact In XDEFIDistributionHelper.sol the getAllTokensForAccount function iterates over the count and performs actions on each iteration. Iterating over large arrays can cause out of gas failures and so the count number should be checked or limited to a max...

merge is loss of all assets

Handle danb Vulnerability details merge function mints nft, but doesn't add it to positionOf, which makes it worthless, in addition it burns all the nfts of the user which means they lost all assets. --- The text was updated successfully, but these errors were encountered: All reactions...

Non unique token ID might lead to collusion when merging

Handle kenzo Vulnerability details Token IDs are defined as concatenation of points, total supply + 1. The total supply can decrease when merging. This means that the contract might try to mint a token with an ID which already exists. Impact Under specific circumstances, users won't be able to lo...

borrow() function has state updates after a callback to msg.sender

Handle jayjonah8 Vulnerability details Impact In TimeswapPair.sol, the borrow function has a callback to the msg.sender in the middle of the function while there are still updates to state that take place after the callback. The lock modifier guards against reentrancy but not against cross functi...

Reserve does not properly apply prices of VADER and USDV tokens

Handle TomFrenchBlockchain Vulnerability details Impact Reserve pays out vastly higher or lower IL protection than it should Proof of Concept Consider the lines 98 and 102 as shown on the link below: Here we multiply the IL experienced by the LP by a price for USDV or VADER as returned by the LBT...

Rewards can be stolen

Handle cmichel Vulnerability details The NFTXInventoryStaking contract distributes new rewards to all previous stakers when the owner calls the receiveRewards function. This allows an attacker to frontrun this receiveRewards transaction when they see it in the mem pool with a deposit function. Th...

Missing of _userUpdate in unwrapFor in WJLP.sol

Handle UncleGrandpa925 Vulnerability details Impact Users' rewards in Wrapped JLP will be miscalculated. Hackers can exploit this to steal users' rewards. All WJLP's unwrapFor transactions will trigger the bug. Location Function unwrapFor in WJLP.sol Explanation of the bug So the nature of this...

Oracle returns an improperly scaled USDV/VADER price

Handle TomFrenchBlockchain Vulnerability details Impact Invalid values returned from oracle in vast majority of situations Proof of Concept The LBT oracle does not properly scale values when calculating prices for VADER or USDV. To show this we consider the simplest case where we expect USDV to...

Unsafe transfer in XTokenUpgradeable

Handle 0x1f8b Vulnerability details Impact Unsafe transfer was done. Proof of Concept In the method XTokenUpgradeable.burnXTokens it's made a transfer without checking the boolean result, ERC20 standard specify that the token can return false if the transfer was not made, so it's mandatory to che...

Unsafe transfers in NFTXFlashSwipe

Handle 0x1f8b Vulnerability details Impact Unsafe transfers. Proof of Concept In the methods NFTXFlashSwipe.flashSwipe and NFTXFlashSwipe.onFlashLoan there are some transfers, transferFroms and approve made without checking the boolean result, ERC20 standard specify that the token can return fals...

Staking Zap add liquidity can be denied

Handle cmichel Vulnerability details The NFTXStakingZap.addLiquidity721WETH function verifies if the contract indeed received the expected amount of vault tokens of balance = count BASE by checking: function addLiquidity721WETH uint256 vaultId, uint256 memory ids, uint256 minWethIn, uint256 wethI...

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

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

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

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

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

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

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

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