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

No check that _to and from are different addresses in outboundTransfer() function

Handle jayjonah8 Vulnerability details Impact In L1LPTGateway.sol the outboundTransfer function transfers the l1Token from the msg.sender to the l1LPTEscrow contract. It also takes in the to argument which is set in the outboundCalldata variable. This function does not check if the msg.sender and...

Unbounded iteration over all pools

Handle Dravee Vulnerability details Impact The transactions could fail if the array get too big and the transaction would consume more gas than the block limit. This will then result in a denial of service for the desired functionality and break core functionality. Proof of Concept Tools Used VS...

If Vault contains tokens that charge a fee on transfer the internally kept balance will be wrong

Handle Ruhum Vulnerability details Impact Some tokens charge a fee for each transfer. USDT, for example, has the possibility of enabling fees at any time. If the vault is used for that kind of token, the internal balance keeping will be wrong. The vault will think that it owns more tokens than it...

Check _to is not empty

Handle pauliax Vulnerability details Impact functions claimYield, withdraw, and unsponsor should validate that to is not an empty 0x0 address to prevent accidental burns. Recommended Mitigation Steps Consider implementing the proposed validation: require to != address0 --- The text was updated...

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

[WP-M4] NonUSTStrategy.sol Lack of slippage control

Handle WatchPug Vulnerability details function swapUnderlyingToUst internal uint256 underlyingBalance = getUnderlyingBalance; if underlyingBalance 0 // slither-disable-next-line unused-return curvePool.exchangeunderlying underlyingI, ustI, underlyingBalance, 0 ; The current implementation of...

Vault: onDepositBurn() causes users to unfairly have their deposits allocated as yield

Handle hickuphh3 Vulnerability details Impact The withdraw and forceWithdraw functions do not have reentrancy protection. This allows reentrancy to occur through the implementation of a malicious claim’s beneficiary onDepositBurn function that will cause the incorrect amount of shares to be minte...

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

Incompatibility With Rebasing/Deflationary/Inflationary tokens

Handle defsec Vulnerability details Impact The Strategy contracts do not appear to support rebasing/deflationary/inflationary tokens whose balance changes during transfers or over time. The necessary checks include at least verifying the amount of tokens transferred to contracts before and after...

Missing slippage/min-return check in the curve Pool

Handle defsec Vulnerability details Impact Trades can happen at a bad price and lead to receiving fewer tokens than at a fair market price. The attacker's profit is the protocol's loss. Proof of Concept The NonUSTStrategy contract is missing slippage checks which can lead to being vulnerable to...

Lack of slippage checks during swap

Handle ye0lde Vulnerability details Impact Since the code does not use a minimum return value for swaps it is susceptible to sandwich attacks. More information here: Proof of Concept Here the code Calls Curve to convert the existing underlying balance into UST: / Calls Curve to convert the existi...

Reentrancy vulnerability allows attacker to steal underlying tokens with withdraw

Handle harleythedog Vulnerability details Impact The withdrawDeposit function has the following code which runs before the underlying tokens are transferred to the to address: if isIntegrationclaim.beneficiary bytes4 ret = IIntegrationclaim.beneficiary.onDepositMinted tokenId, newShares, claim.da...

TimeswapConvenience.sol#borrowGivenDebt() Attacker can increase state.y to an extremely large value with a dust amount of assetOut

Handle WatchPug Vulnerability details This issue is similar to the two previous issues related to state.y manipulation. Unlike the other two issues, this function is not on TimeswapPair.sol but on TimeswapConvenience.sol, therefore this can not be solved by adding onlyConvenience modifier...

TimeswapPair.sol#mint() Lack of input validation allows attacker to set pool.state.y to an arbitrary target value

Handle WatchPug Vulnerability details pool.state.y += yIncrease; TimeswapPair.solmint takes a user input value of yIncrease without proper validation, which means that it allows the state of pool.state.y to increase by the arbitrary value set by the caller. Impact When pool.state.y is extremely...

Missing noreentrant check on mint function

Handle Fitraldys Vulnerability details Impact in there is no reentrant check, because when using safeMint, the function will make a call to the to address, through / checkOnERC721Received, that will call to the user to address, and the to address is controllable by the to address. Proof of Concep...

safeSymbol() can revert causing DoS

Handle sirhashalot Vulnerability details Impact The safeSymbol function, found in the SafeMetadata.sol contract and called in 4 Timeswap Convenience contracts in the symbol functions, can cause a revert. This could make the 4 contracts not compliant with the ERC20 standard for certain asset pairs...

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

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

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

Missing noreentrant modifier on merge() and updateDistribution(), can reenter from the _lock()

Handle Fitraldys Vulnerability details Impact A user can call lock that will call lock, that eventually will call safeMint, the safeMint function will make an external call to the destination address through checkOnERC721Received, and since the merge and updateDistribution didnt have noreentrant...

DOS - preventing users from locking assets

Handle egjlmn1 Vulnerability details An attacker can prevent any user from locking assets due to the unsafe id generation for the nfts. The id is generated by using only the amount deposited, the duration to lock and the totalSiupply of nfts. The first two are easy for the attacker to copy, so al...

Dashboard is not working , lend positiona are still loading, the same is for Lend button, Lend Amount to lend is not refresjing

Handle 0v3rf10w Vulnerability details Impact Detailed description of the impact of this finding. Proof of Concept Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept. Tools Used Recommended Mitigation Steps --- The...

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

Anyone Can Frontrun VaderPoolV2.mintSynth() To Steal Synthetic Assets

Handle leastwood Vulnerability details Impact The mintSynth function is callable by any user and creates a synthetic asset against foreignAsset if it does not already exist. The protocol expects a user to first approve the contract as a spender before calling mintSynth. However, any arbitrary use...

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

validCollateral[0] can be added multiple times

Handle gzeon Vulnerability details Impact In addCollateral of Whitelist.sol, whatever in index 0 of validCollateral can be added multiple times. if validCollateral.length != 0 && validCollateral0 != collateral requirecollateralParamscollateral.index == 0, "collateral already exists"; Proof of...

'wrap' tokens that you are not entitled to

Handle pauliax Vulnerability details Impact function wrap can be called by anyone. It accepts arbitrary from and to, and transfers 'from', and mints 'to'. A malicious actor can transfer from other users that have approved the contract before, e.g. an EOA user will need to execute 2 txs: first,...

NFTXStakingZap and NFTXMarketplaceZap's transferFromERC721 transfer Cryptokitties to the wrong address

Handle hyh Vulnerability details Impact transferFromERC721address assetAddr, uint256 tokenId, address to should transfer from msg.sender to to. It transfers to addressthis instead when ERC721 is Cryptokitties. As there is no additional logic for this case it seems to be a mistake that leads to...

Missing access restriction on StabilityPool's receiveCollateral

Handle kenzo Vulnerability details StabilityPool's receiveCollateral should only be called by ActivePool, but that check is missing. Anybody can call it and update StabilityPool's total collateral variable. Impact Wrong amounts of total collateral in StabilityPool totalColl. As far as I can see,...

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

Borrow should increase principal by withdrawn amount

Handle cmichel Vulnerability details The CreditLine.borrow function increases the principal by the amount parameter, but the borrower only receives tokenDiffBalance. // @audit increases by amount instead of tokenDiffBalance creditLineVariablesid.principal =...

Missing payable

Handle robee Vulnerability details The following functions are not payable but uses msg.value - therefore the function must be payable. This can lead to undesired behavior. CreditLine.sol, depositCollateral is payable but doesn't use msg.value CreditLine.sol, repay is payable but doesn't use...

Collateral can be deposited in a finished pool

Handle pedroais Vulnerability details Proof of Concept The depositCollateral function doesn't check the status of the pool so collateral can be deposited in a finished loan. This can happen by mistake and all funds will be lost. Recommended Mitigation Steps Require loan status to be collection or...

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

Approved by zero first

Handle Jujic Vulnerability details Some tokens like USDT do not work when changing the allowance from an existing non-zero allowance value. They must first be approved by zero and then the actual allowance must be approved. Impact When using one of these unsupported tokens, all transactions rever...