[WP-M6] Inappropriate support of EIP-2981

Lines of code Vulnerability details if nftContract.supportsERC165InterfacetypeIRoyaltyInfo.interfaceId try IRoyaltyInfonftContract.royaltyInfo gas: READONLYGASLIMIT tokenId, BASISPOINTS returns address receiver, uint256 / royaltyAmount / if receiver != address0 recipients = new address payable;...

NFT Creators Can Avoid Paying The Primary Foundation Fee

Lines of code Vulnerability details Impact The primary foundation fee is charged on the initial sale of newly minted NFTs. However, the isCreator && !nftContractToTokenIdToFirstSaleCompletednftContracttokenId check can easily be bypassed to avoid paying the primary foundation fee by selling the...

_getCreatorPaymentInfo() is Not Equipped to Handle Reverts on an Unbounded _recipients Array

Lines of code Vulnerability details Impact The getCreatorPaymentInfo function is utilised by distributeFunds whenever an NFT sale is made. The function uses try and catch statements to handle bad API endpoints. As such, a revert in this function would lead to NFTs that are locked in the contract...

An offer made after auction end can be stolen by an auction winner

Lines of code Vulnerability details Impact An Offer which is made for an NFT when auction has ended, but its winner hasn't received the NFT yet, can be stolen by this winner as transferFromEscrow being called by acceptOffer will transfer the NFT to the winner, finalising the auction, while no...

Upgradable escrow contract

Lines of code Vulnerability details Impact Upgradable escrow contract pose great risk to user who approved their NFT to the contract. Most popular token / NFT exchange do not require user approve their asset to admin upgradable contract. This also increase user gas usage because they would have t...

Creators can steal sale revenue from owners' sales

Lines of code Vulnerability details According to the README.md All sales in the Foundation market will pay the creator 10% royalties on secondary sales. This is not specific to NFTs minted on Foundation, it should work for any NFT. If royalty information was not defined when the NFT was originall...

ERC20 decimals can be different than 18

Lines of code Vulnerability details Impact setDefaultFeePercentage, setCustomFeePercentageForCollateral, setCustomFeePercentageForSafe functions assume that the underlying ERC20 token has 18 decimal digits. Whilst this is true most of the time, an ERC20 token can have a different decimals value...

Ownership of Swap.vy cannot be transferred

Lines of code Vulnerability details Impact Ownership transfer function of Swap.vy is commented out. Fund can be stuck if an AMM and governance change/upgrade is required. Proof of Concept --- The text was updated successfully, but these errors were encountered: All reactions...

QA Report

LOW : 1. Title : Missing limit on how many AMMs can be added Impact : The governance can add an amm, by calling whitelistAmm function, however there is no limit on how many amm that the contract can be held, if the governance keep adding amm, then the clearing house will brick with out of gas,...

InsuranceFund depositors can be priced out & deposits can be stolen

Lines of code Vulnerability details Impact The InsuranceFund.deposit function mints initial shares equal to the deposited amount. The deposit / withdraw functions also use the VUSD contract balance for the shares computation. balance = vusd.balanceOfaddressthis It's possible to increase the share...

ERC4626 mints more shares than it should

Lines of code Vulnerability details bug in the mint function of the ERC4626 contract The mint function recieves an amount of shares and an address to and mints the amount of shares to the to address. The sender must transfer an amount of token, so that the ratio will be saved - shares / totalShar...

Use of IERC20.transfer() instead of SafeERC20.safeTransfer()

Lines of code Vulnerability details Impact There are multiple external calls to IERC20.transfer that suppose to transfer out tokens from the system. However, there are tokens like USDT that don't return any return value in their implementation of the transfer function, so calling IERC20.transfer ...

canSafeBoostVault() doesn't actually check the feiAmount passed in

Lines of code Vulnerability details Impact In TurboMaster.sol the onSafeBoost function calls canSafeBoostVault on the booster contract to check if the safe is allowed to boost the vault using the feiAmount which is passed in by the caller as an argument. The problem is that the feiAmount is never...

Profile creation can be frontrun

Lines of code Vulnerability details Impact The LensHub/PublishingLogic.createProfile function can be frontrun by other whitelisted profile creators. An attacker can observe pending createProfile transactions and frontrun them, own that handle, and demand ransom from the original transaction...

NestedFactory.addOperator/removeOperator have no effect until importOperators

Lines of code Vulnerability details Impact addOperator/removeOperator being run alone don't have any effect, as the cache is used in operations, which is only updated when an implementation is added or removed via importOperators. If an operation is added via addOperator, but importOperators isn'...

unsafe cast can lead to theft

Lines of code Vulnerability details in the first link I provided, a hacker can call withdraw with amount = 2196, the amount of shares that will be burned is uint1922192 == 0. if the system has enough money, the hacker can steal 2192 tokens and pay nothing. for the second link, a user that provide...

extractTokensForCollateralShortfall can be called repeated to extract more than 50% of BPT

Handle gzeon Vulnerability details Impact During a collateral shortfall event, it is designed that the DAO can extract up to 50% of the BPT tokens. The 50% is set to limit the value the DAO can extract from the stakers. However, it is possible for the DAO to extract more than 50% by calling...

Double transfer in the transferAndCall function of ERC677

Handle shw Vulnerability details Impact The implementation of the transferAndCall function in ERC677 is incorrect. It transfers the value amount of tokens twice instead of once. Since the Flan contract inherits ERC667, anyone calling the transferAndCall function on Flan is affected by this...

Unable to withdraw if block.number > 32-bit

Handle gzeon Vulnerability details Impact If block.number 32-bit, updateTotalSupplyCheckPoints would revert. updateTotalSupplyCheckPoints is called from mint and burn, which mean withdraw would also revert. Proof of Concept uint32 blockNumber = safe32block.number, "block number exceeds 32 bits";...

Use of deprecated Chainlink function latestAnswer

Handle WatchPug Vulnerability details According to Chainlink's documentation, the latestAnswer function is deprecated. This function does not revert if no answer has been reached but returns 0, causing an incorrect price fed to the EIP1271Wallet. See: code-423n4/2021-07-wildcredit-findings75 ---...

FarmingPools.sol Some users may not be able to get back their funds for tokens with tax on transfer

Handle WatchPug Vulnerability details Per the doc: Token with tax and rewards should accounted correctly and share with all holder accordingly. In the current implementation of FarmingPools.solstake, it assumes that the received amount is the same as the transfer amount, and recording it to...

Tolerance is not enforced during a flash governance decision

Handle shw Vulnerability details Impact Most of the functions with a governanceApproved modifier call flashGoverner.enforceTolerance to ensure the provided parameters are restricted to some range of their original values. However, in the governanceApproved modifier,...

Locking of User Funds Without Permission

Handle kirk-baird Vulnerability details Impact The function assertGovernanceApprovedaddress sender, address target, bool emergency has public visibility and may be called by any user. Since the user who creates the transaction is able to specify the sender address they are able to force users who...

Stacking with 0 amount will reset rewarded without claiming any flan.

Handle Randyyy Vulnerability details Impact A user can stake their token by calling stake function, by supplying a token, however staking 0 amount token is allowed, staking 0 amount will reset the reward debt, without minting a single flan token, the function will treat as if the user do the...

Upper limit for set CoolDownTime

Handle Jujic Vulnerability details Impact There is no upper limit for coolDownTimeInSeconds. It may be set too large. Proof of Concept function setCoolDownTimeuint32 coolDownTimeInSeconds external onlyOwner coolDownTimeInSeconds = coolDownTimeInSeconds; emit...

Cvx3CrvOracle.sol _peek() returns wrong units

Handle sirhashalot Vulnerability details Impact The Cvx3CrvOracle.sol contract claims it "provides current values for Cvx3Crv". When getting the current values, "only cvx3crvid and ethId are accepted as asset identifiers" for the base and quote parameters to the peek and get functions. peek and g...

Oracle might return stale or incorrect results (Cvx3CrvOracle.sol)

Handle ye0lde Vulnerability details Impact Oracle might return stale or incorrect results Cvx3CrvOracle.sol The peek function in the contract Cvx3CrvOracle.sol fetches the daiPrice, usdcPrice, usdtPrice from a Chainlink aggregator using the latestRoundData function. If there is a problem with...

Reentrancy in withdrawGovernanceAsset

Handle 0x1f8b Vulnerability details Impact The method withdrawGovernanceAsset it's vulnerable to reentrancy attacks. Proof of Concept The method FlashGovernanceArbiter.withdrawGovernanceAsset it's vulnerable to reentrancy attacks because the flag stored in pendingFlashDecision it's deleted AFTER...

Attacker can collect all positive rebase from the poll

Handle gzeon Vulnerability details Impact The concept of ElasticSwap is not to change relative price upon positive rebase event. However, this allow an attacker to sandwich a known positive rebase event for profit. Proof of Concept 1. Assuming we have pool with 10000 base and 10000 quote token 2...

The amount of tokens received by the pool might be less than expected for feeOnTransfer tokens

Handle UncleGrandpa925 Vulnerability details Impact This issue impacts any pools that use fee-on-transfer quoteTokens. The codebase is inspired by Uniswap, with one important change: the router is built right into the pool themselves. In addLiquidity, after having updated all internal parameters...

Owner should be able to choose to not ignore failures in updateYieldStrategy

Handle harleythedog Vulnerability details Impact In Sherlock.sol, the function updateYieldStrategy ignores all errors when yieldStrategy.withdrawAll is called, and I believe this is an dangerous design choice. If yieldStrategy.withdrawAll reverts, then all the funds that are deposited in the yiel...

calculateLiquidityTokenFees returns zero fee

Handle sirhashalot Vulnerability details Impact The MathLib.sol calculateLiquidityTokenFees function returns a non-zero fee quantity only if rootK rootKLast. The rootK and rootKLast values are calculated from the same values, so this will never occur. This will result in the DAO never receiving a...

Attacker can grief initial pool by providing 1 baseToken, 1 quoteToken, and manually transferring 1 baseToken

Handle camden Vulnerability details Impact Read the attack composition below. But the main criteria is that the attacker has to be the first person to provide liquidity. They can at least from my testing permanently grief a pool and make it impossible for any later person to get liquidity tokens,...

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

Arbitrager can take more arbReward than expected.

Handle wuwe1 Vulnerability details Proof of Concept Arbitrager can reentry arbRestake This line will not revert because stakeShares ≤ stakeSharesid 0.2 Recommended Mitigation Steps Use ReentrancyGuard to guard arbRestake --- The text was updated successfully, but these errors were encountered: Al...

Add a timelock to ExchangeFactory.sol:setFeeAddress() (function that sets a key variable)

Handle Dravee Vulnerability details Impact To give more trust to users: functions that set key/critical variables should be put behind a timelock. Proof of Concept Here, a malicious owner could set the feeAddress to his own and the fee from liquidity tokens that get minted would get stolen: . As...

fundDepositAndReserveFor function does not exist in protocol

Handle harleythedog Vulnerability details Impact The L2Migrator contract makes use of the function fundDepositAndReserveFor on the ticket broker. In the commit hash for the contest seen from this snippet from the contest page: git clone https://github.com/livepeer/protocol git checkout...

LivepeerToken.burn function could burn tokens of any user

Handle cccz Vulnerability details Impact Same as code-423n4/2021-11-overlay-findings22, the burner could burn any amount of tokens of any user. Proof of Concept Tools Used Manual analysis Recommended Mitigation Steps Update burn function for only owner can burn his tokens. --- The text was update...

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

transfer return value of a general ERC20 is ignored

Handle pants Vulnerability details Need to use safeTransfer instead of transfer. As there are popular tokens, such as USDT that transfer/trasnferFrom method doesn’t return anything. The transfer return value has to be checked as there are some other tokens that returns false instead revert, that...

[WP-H32] PoolTemplate.sol Attacker can call Factory#createMarket() and transfer funds from another user's wallet to the pool

Handle WatchPug Vulnerability details function initialize string calldata metaData, uint256 calldata conditions, address calldata references external override require initialized == false && bytesmetaData.length 0 && references0 != address0 && references1 != address0 && references2 != address0 &&...

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

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

Reentrancy vulnerability allows attacker to gain more shares than deserved

Handle harleythedog Vulnerability details Impact In Vault.sol, the deposit function is the external function that allows transferring underlying tokens to mint position NFTs. The deposit function first calls createDeposit which creates the position/determines how many shares to allocate, and then...

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

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

Locks can be denied

Handle cmichel Vulnerability details The XDEFIDistribution.lock function mints a new token and the generateNewTokenId function returns a token ID as the concatenation of the points and totalSupply + 1: function generateNewTokenIduint256 points internal view returns uint256 tokenId // Points is...

USDV.sol Mint and Burn Amounts Are Incorrect

Handle leastwood Vulnerability details Impact The USDV.mint function queries the price of Vader from the LiquidityBasedTwap contract. The calculation to determine uAmount in mint is actually performed incorrectly. uAmount = vPrice vAmount / 1e18; will return the USD amount for the provided Vader ...

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

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