The check for value transfer success is made after the return statement in _withdrawFromYieldPool of LidoVault

Lines of code Vulnerability details Impact Users can lose their funds Proof of Concept The code checks transaction success after returning the transfer value and finishing execution. If the call fails the transaction won't revert since requiresent, Errors.VTCOLLATERALWITHDRAWINVALID; won't execut...

User may lose ETH when depositing stETH

Lines of code GeneralVault.solL75-L89 LidoVault.solL79-L104 Vulnerability details Impact When the depositCollateral function is used in the LidoVault.sol contract, a user depositing stETH may mistakenly have a non-zero msg.value. The ETH would be passed to the contract and the user would lose tha...

[WP-H5] LidoVault.sol Wrong implementation can cause the users to lose their funds when withdrawing ETH

Lines of code Vulnerability details function withdrawFromYieldPool address asset, uint256 amount, address to internal override returns uint256 address LIDO = addressesProvider.getAddress'LIDO'; if asset == address0 // Case of ETH withdraw request from user, so exchange stETH - ETH via curve uint2...

depositCollateral allows user's ETH sent along with the tx to be stuck forever

Lines of code Vulnerability details Impact LidoVault and ConvexCurveLPVault both inherit from GeneralVault that implement the method depositCollateral. This method has the keyword payable so it allows users to send ETH with the tx. depositToYieldPool that is called inside depositCollateral in bot...

Loss of funds due to beneficiary override to address(0) during transfer

Lines of code Vulnerability details Premiums or proceeds earned after the transfer will accrue to the zero address, instead of to the new vault owner, and the funds will be irrecoverable. Proof of concept vaultBeneficiariesvaultId is overridden to the zero address during transfer: File: Cally.sol...

Race condition in exercising the options

Lines of code Vulnerability details Impact Option buyers might never exercise their options with a bad actor vault owner, hence creating trust issues to the platform since the promised functionality causes only loss of funds. Proof of Concept Alice creates a vault for her high valued NFT. Bob buy...

Strike price curve of getDutchAuctionStrike is not what users expect

Lines of code Vulnerability details Note: This submission contains links to a private fork of the contest repo. User code423n4 has been added as a collaborator in order to view. Impact Function getDutchAuctionStrike does not implement the function that Option buyers would expect. They probably...

Fee can possibly be set maliciously

Lines of code Vulnerability details Fee is set by an admin and can be set maliciously to steal the funds that are entitled to go to the user. Impact Fee can be set to a maliciously high value to unfairly extract funds from protocol users. An owner can buy options, set fee to 100% and exercise...

Fee on transfer tokens block exercises and withdrawals

Lines of code Vulnerability details Fee on transfer tokens block exercises and withdrawals If a vault is created with a fee-on-transfer ERC20 as its token, the underlying asset may be locked in the contract. Scenario: Setup: 1. Alice calls createVault with a fee-on-transfer token address and 1000...

call to harvest() can be manipulated to steal funds from the contract

Lines of code Vulnerability details Impact When calling harvest, attacker can device a cross-contract reentrancy to steal NFT owned by the contract Cally. Proof of Concept 1. While creating a vault, the owner of NFT transfers the NFT to the contract i.e. ownerOfnftID is Cally. 2. Attacker contrac...

Executors need to be trusted

62 comment Warden: cmichel Executors need to be trusted, otherwise, they can just call sendFundsToUser to transfer out any funds as they please. Consider adding threshold signatures to prevent all funds from being lost when a single executor is compromised. --- The text was updated successfully,...

Nft.upaidRewards reset upon withdrawal

129 comment Warden: 0xDjango Nft.upaidRewards reset upon withdrawal. This feels a bit unfair if a user means to collect rewards while withdrawing NFT. If there are not enough rewards in the contract balance, the unpaidRewards will be deleted upon withdrawal with the line delete nftInfonftId;. I'm...

Users will pay more than required for NFT Minting

Lines of code Vulnerability details Impact NFTs should be sold for their best price without descending into a gas race: Due to SpeedBumpPriceGate.sol function passThruGate code users will pay more than required for NFT Minting. User will pay msg.value and not the NFT "price". It is very likely th...

Not checking returned bool by transfer can lead to loss of funds

Lines of code Vulnerability details Impact Loss of funds Proof of Concept The ERC20 interface ensures a token transfer will return false on failure. In merkleVesting there is no requirement for this to be true. The contract doesn't ensure all the funds to cover the MerkleTree are present since it...

ERC20 transfers does not work on non-standard compliant tokens like USDT

Lines of code Vulnerability details Impact PermissionlessBasicPoolFactory.sol Consider this function: function deposituint poolId, uint amount external Pool storage pool = poolspoolId; requirepool.id == poolId, 'Uninitialized pool'; requireblock.timestamp pool.startTime, 'Cannot deposit before po...

decimalMultiplier assumes tokens have <= 18 decimals

Originally submitted by warden StyxRave in 153, duplicate of 49. BkdTriHopCvx.sol decimalMultiplier assumes tokens have = 18 decimals. Will always be 0 for tokens with more than 18 which would be still ERC20 compliant. --- The text was updated successfully, but these errors were encountered: All...

Users can not initialize and withdraw tokens if coinsPerSecond is 0

Lines of code Vulnerability details Impact If a user tries to claim a few totalCoins with a long vestingTime, this user will call the initialize function failed, and can not withdraw funds. Proof of Concept In MerkleResistor.sol L259: uint coinsPerSecond = totalCoins uint100 - tree.pctUpFront /...

Reward Token Transfer Failure Can Lead to Loss of Deposit in PermissionlessBasicPoolFactory

Lines of code Vulnerability details Impact If transfer of any reward token returns False or reverts for whatever reason, users who deposited will not be able to withdraw their deposit. A malicious pool creator could abuse this to lock tokens from victims by using two reward tokens, one...

Forget to remove account out of _roleMembers[role]

Originally submitted by warden TrungOre in 89, duplicate of 164. For get to remove account out of roleMembersrole --- The text was updated successfully, but these errors were encountered: All reactions...

Using deprecated Chainlink function latestAnswer() in PriceOracleImplementation.sol

Lines of code Vulnerability details Impact In the function getUnderlyingPrice, Chainlink's latestAnswer is used and this function is deprecated and should not be used as this function returns 0 when there is no data rather than giving out error. Precedent of this issue can be found in...

Admin can manipulate nft prices by _setNftPriceOracle

Lines of code Vulnerability details impact setNftPriceOracle is an admin-only function which can set a new nftOracle. But there is no restriction in this function. An admin can set any oracle unconditionally. It could be used to manipulate nft prices in liquidateCalculateSeizeNfts. Proof of Conce...

Using deprecated Chainlink function latestAnswer

Lines of code Vulnerability details Impact According to Chainlink's documentation, the latestAnswer function is deprecated. Proof of Concept This function does not error if no answer has been reached but returns 0. Besides, the latestAnswer is reported with 18 decimals for crypto quotes but 8...

Outdated Interface Implementation

Lines of code Vulnerability details Impact The Chainlink aggregator interface used by the project is outdated and does not permit the project to properly sanitize the price values it receives. Recommended Mitigation Steps The interface is to be updated according to the official Chainlink...

Unbound loop can brick contract and freeze miner payments

Lines of code Vulnerability details Impact The payees array in the MinerPayer.sol contract can grow to an unbounded size. That means it could grow so large that the MinerPayer.release function can't loop through them all without triggering an out of gas error. Updating the payees array in the...

VaultCore's withdrawETH uses transfer function, which can be unusable for some smart contracts

Lines of code Vulnerability details Impact When a user wishes to withdraw their collateral from a WETH vault, the funds are returned to the user with msg.sender.transfer. Using transfer is not recommended anymore, especially for critical operations like withdrawing collateral from a vault as it...

PARMinerV2's liquidate can become stuck

Lines of code Vulnerability details PARMinerV2's liquidate can be run repeatedly for the same collateralToken with different arguments. For example, different Vaults can have the same token, so there can be unrelated runs with different vaultId / DEX data, but the same collateralToken and proxy. ...

Lender can immediately liquidate valued Collateral in NFTPairWithOracle contract

Lines of code Vulnerability details Impact A lender can liquidate a borrower's collateral immediately by calling updateLoanParams in the NFTPairWithOracle contract where the ltvBPS for the params struct is set to 0. This bypasses the checks to make sure that the terms are favourable to the...

Malicious lender can change price oracle for outstanding loan

Lines of code Vulnerability details The updateLoanParams function in NFTPairWithOracle.sol allows the lender to update parameters for an outstanding loan duration, valuation, annual interest, and collateralization ratio as long as they are the same or better for the borrower. These params are...

Users Who approve() ERC20 Tokens for NFTPair of NFTPairWithOracle Contracts May Have Their Allowances Stole By Any User

Lines of code Vulnerability details Impact User's who approve ERC20 or any other token type excluding the collateral token may have their balances stolen by any user. The ACTIONCALL allows users to call any function on any smart contract excluding this, collateral and bentoBox. Thus, if any user...

Lender can adjust ltvBPS and instantly sieze collateral

Lines of code Vulnerability details Impact After the loan is accepted and is in the LOANOUTSTANDING phase, updateLoanParams allows the lender to update the TokenLoanParams.ltvBPS parameter. The require check allows for the parameters to change as long as they are favorable for the borrower, but...

Owner or Managers can rug Aave rewards

Lines of code Vulnerability details Impact A malicious owner or manager can steal all Aave rewards that are meant for PoolTogether users Even if the user is benevolent the fact that there is a rug vector available may negatively impact the protocol's reputation. Proof of Concept File:...

Borrower can frontrun lender's call to lend to change the price oracle

Lines of code Vulnerability details Impact In NFTPairWithOracle.lend, params.oracle is not checked. This allow a borrower to watch the mempool and front-run the lender's call and change oracle to avoid liquidation. Proof of Concept function lend address lender, uint256 tokenId, TokenLoanParams...

Manager or owner can send rewards to any address

Lines of code Vulnerability details Impact In the claimRewards function, manager or owner can send rewards to any address. function claimRewardsaddress to external onlyManagerOrOwner returns bool requireto != address0, "AaveV3YS/payee-not-zero-address"; address memory assets = new address; assets...

User fund loss in supplyTokenTo() because of rounding

Lines of code Vulnerability details Impact When user use supplyTokenTo to deposit his tokens and get share in FeildSource because of rounding in division user gets lower amount of share. for example if token's decimal was 1 and totalSupply was 1000 and aToken.balanceOfFieldSource.address was 2100...

_amount requires to be updated to contract balance increase (2)

Lines of code Vulnerability details Impact Every time transferFrom or transfer function in ERC20 standard is called there is a possibility that underlying smart contract did not transfer the exact amount entered. It is required to find out contract balance increase/decrease after the transfer. Th...

_depositAmount requires to be updated to contract balance increase

Lines of code Vulnerability details Impact Every time transferFrom or transfer function in ERC20 standard is called there is a possibility that underlying smart contract did not transfer the exact amount entered. It is required to find out contract balance increase/decrease after the transfer. Th...

Lack of safeApprove(0) prevents some registrations, and the changing of stakers and LP tokens

Lines of code Vulnerability details OpenZeppelin's safeApprove will revert if the account already is approved and the new safeApprove is done with a non-zero value function safeApprove IERC20 token, address spender, uint256 value internal // safeApprove should only be called when setting an initi...

BufferStored could be larger than BufferCap after _setBufferCap()

Judge @jack-the-pug has assessed the second item in QA Report 64 as Medium risk. The relevant finding follows: Impact In RateLimited.sol BufferCap should be the upper bound of BufferStored, However in setBufferCap it calls updateBufferStored before replacing the old BufferCap. If old BufferCap is...

Swapper3Crv's swapping path can be suboptimal

Lines of code Vulnerability details Impact Swapper3Crv.swap result can be suboptimal as only paths with ETH are evaluated. Setting severity to medium as despite function availability not affected there can be some fund losses as a result. Proof of Concept tokenAmountOut uses fixed tokenIn, ETH,...

FlywheelCore's setFlywheelRewards can remove access to reward funds from current users

Lines of code Vulnerability details Impact FlywheelCore.setFlywheelRewards can remove current reward funds from the current users' reach as it doesn't check that newFlywheelRewards' FlywheelCore is this contract. If it's not, by mistake or with a malicious intent, the users will lose the access t...

Unchecked parameters in Chainlink latestRounddata() could lead to retrieve stale data

Lines of code Vulnerability details Impact Oracle can retrieve unchecked stale data Proof of Concept In ChainlinkpriceOracle.sol the function function refreshedAssetPerBaseInUQaddress asset public override returns uint AssetInfo storage assetInfo = assetInfoOfasset; , int basePrice, , , =...

Should implement a periphery contract for user to mint indexToken

Lines of code Vulnerability details Impact User can lose their fund Proof of Concept When users want to mint an index token, users need to transfer their assets to addressvToken first, then call the mint function of IndexLogic.sol. If users make it into 2 transactions, miner can manipulate it/...

Potentially Incomplete or Stale Data from Oracle

Lines of code Vulnerability details Impact Calls to the Chainlink price oracle via refreshedAssetPerBaseInUQ in ChainlinkPriceOracle.sol use the correct function latestRoundData per Chainlink's documentation, but lacks the recommended validations to ensure that the round is complete and does not...

The name of the function to setup a vesting in the interface IVesting.sol doesn’t match with the name of the function to setup a vesting in StakeCitadelVester.sol.

Lines of code Vulnerability details Impact Users will not be able to withdraw their funds . Proof of Concept When a user wants to withdraw his tokens from StakedCitadel.sol, vesting is supposed to be set and tokens are sent to the vesting contract where they are vested linearly for 21 days. This ...

TokenInLimit can be set higher than what the contract has to sell

Lines of code Vulnerability details Impact If totalTokenOutBought tokenOut.balanceOfaddressthis all tokens will be unclaimable and all funds would be lost. Proof of Concept TokenInLimit can be set arbitrarily high even if the contract doesn't have enough tokens to sell. Then users can buy more...

DoS at CitadelMinter.sol

Lines of code Vulnerability details Impact At CitadelMinter.sol, Funding Pool Weight can't be set at the beginning since totalFundingPoolWeight value is not assigned and Zero meanwhile being cached to newTotalWeight. Hence the substraction will not perform as it will yield to a negative value whi...

withdrawal amount might be wrong

Lines of code Vulnerability details r is the user's part of the contract balance, but is supposed to be the user's part of the total funds, including the strategy funds. therefore the check at line 816 will always return false because the user's part of the contract balance is smaller than the...

TECH_OPERATIONS_ROLE is not initialized properly

Lines of code Vulnerability details Impact setGuestlist and setTokenInLimit are unavailable for use during KnightingRound to use due to a mistake in initializing roles. Proof of Concept The role TECHOPERATIONSROLE, created in L30 of GlobalAccessControl.sol , is not initialized in the initialize t...

Improper Validation Of Chainlink's latestRoundData Function

Lines of code Vulnerability details Impact When using Chainlink Price feeds, it is important to ensure the price feed data was updated recently. While getting started with chainlink requires just one line of code, it is best to add additional checks for "in production" environments. Here,...

JPEGLock lockups overwrite previous data

Lines of code Vulnerability details Impact If a user calls NFTVault.finalizePendingNFTValueETH a second time without first calling JPEGLock.unlock to recover their previous lockup, their balance will be overwritten leaving the previous lockup balance unrecoverable. Proof of Concept POC by adding...