pricePerShare is only lazily updated

Handle cmichel Vulnerability details The pricePerShare value is cached in updatePricePerShare and this value can be old and not reflect the actual core.pricePerShare value. The same issue exists on the sidechain variant when oracle price updates happen infrequently. This leads to potentially big...

Null check in pricePerShare

Handle hack3r-0m Vulnerability details oracle can 0 as a price of the share, in that case, 0 will be the denominator in some calculations which can cause reverts from SafeMath for e.g here: resulting in Denial Of Service. Add a null check to ensure that on every update, the price is greater than ...

Profit from difference between core.pricePerShare() and pricePerShare()

Handle gpersoon Vulnerability details Impact Suppose there is a small difference between core.pricePerShare and pricePerShare of WrappedIbbtcEth.sol on Eth main chain. This could be due to the fact that updatePricePerShare hasn't been called for a while, or a sudden change in core.pricePerShare. ...

Wrong calculation of erc20Delta and ethDelta

Handle WatchPug Vulnerability details function fillZrxQuote IERC20 zrxBuyTokenAddress, address payable zrxTo, bytes calldata zrxData, uint256 ethAmount internal returns uint256, uint256 uint256 originalERC20Balance = 0; if!signifiesETHOrZeroaddresszrxBuyTokenAddress originalERC20Balance =...

UserManager: updateLockedData() doesn't check that the amount is actually locked.

Handle itsmeSTYJ Vulnerability details Impact The function updateLockedData does not actually check if the amount required to be locked is actually locked. Proof of Concept Same solution as my other high issue. I've added comments where relevant. function updateLockedData address borrower, uint25...

Rebalance will fail due to low precision of percentages

Handle cmichel Vulnerability details The AssetManager.rebalance function has a check at the end to ensure that all tokens are deposited again: requiretoken.balanceOfaddressthis == 0, "AssetManager: there are remaining funds in the fund pool"; The idea is that the last market deposits all...

disableValidator() can distort totalGlobalShares (edge case)

Handle gpersoon Vulnerability details Impact Assume a validator has been added Assume this validator calls disableValidator maybe by accident or it recognizes a mistake Assume stake hasn't been called yet and endEpoch is still 0 The function disableValidator will execute and set v.disabledEpoch t...

QuickAccManager.sol Potential replay attack

Handle WatchPug Vulnerability details This issue is possibly duplicate with a previous issue named "Potential replay attack" In QuickAccManager.solsendTxns and QuickAccManager.solsendTransfer, addressidentity is not included in the txHash, makes it possible to replay the transaction on another...

Number of prizes math is wrong

Handle cmichel Vulnerability details The math described in Splitting the prizes and implemented in DrawCalculator.numberOfPrizesForIndex seems to be wrong. Assuming a bit range of 4 16 possibilities per position and cardinality of 8. Note that degree is determined by the first position where it...

unchecked return value from Transfer/TransferFrom()

Handle JMukesh Vulnerability details Impact 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. Proof of Concept Tools Used manual...

exitZcTokenFillingZcTokenInitiate in Swivel.sol, token transfer may fail without function reverting

Handle GalloDaSballo Vulnerability details Impact exitZcTokenFillingZcTokenInitiate in Swivel.sol uses transferFrom this function can failmeaning tokens are not transferred,without causing a revert. This can break the accounting of the protocol The reason why this can happen is that certain ERC20...

uToken ERC20 approve method missing return value check #L109

Handle defsec Vulnerability details Impact The initiateVaultFillingZcTokenInitiate function performs an ERC20.approve call but does not check the success return value. Some tokens do not revert if the approval failed but return false instead. Proof of Concept 1. Navigate to "" 2...

User can get more/less interest than required

Handle csanuragjain Vulnerability details Impact It seems that owner must not get interest for transfer fee which is collected using transferNotionalFee No interest is calculated on owner side before reducing the owner notional amount But this can simply be bypassed if owner adds new notional whi...

Auction.sol#settleAuction() Mishandling bounty state could potentially disrupt settleAuction()

Handle WatchPug Vulnerability details function withdrawBountyuint256 memory bountyIds internal // withdraw bounties for uint256 i = 0; i bountyIds.length; i++ Bounty memory bounty = bountiesbountyIdsi; requirebounty.active; IERC20bounty.token.transfermsg.sender, bounty.amount; bounty.active =...

Use mutex lock on VaultHelper.sol

Handle tensors Vulnerability details Impact I strongly recommend adding a nonreentrant modifier on the functions within VaultHelper.sol The contract makes a bunch of unsafe external calls to the user submitted addresses vault and gauge. Also, add some checks to make sure vault and gauge are...

Controller.withdraw(...) User may lose funds when withdraw wantToken from the underlying contract

Handle WatchPug Vulnerability details The wantToken of the strategy may be different from the token argument of Controller.withdrawaddress token, uint256 amount according to code at line 469-474 of Controller.sol. if want != token address converter = vaultDetailsmsg.sender.converter;...

Missing check for duplicate token in addToken

Handle 0xRajeev Vulnerability details Impact addToken does not check for token being added a duplicate of what was already added. If a duplicate token is added, removeToken only removes the first matching token and the later duplicates still remain. With the vaulttoken deleted, this may lead to...

harvestNextStrategy never executes because lastCalled is 0 and stays 0

Handle gpersoon Vulnerability details Impact The function harvestNextStrategy of Harvester.sol checks canHarvest to make sure it can harvest. Initially strategy.lastCalled will be 0 so canHarvest will return false. Thus the require in harvestNextStrategy fails And it never reaches the point where...

lack of validation for the v and s value in recover() funciton

Handle JMukesh Vulnerability details Impact due to lack of checking of v and s value in recover it become prone to signature malleability Proof of Concept check out the tryRecover of ECDSA.sol Tools Used manual reveiw Recommended Mitigation Steps add necessary check to make the signature unique -...

Arithmetic Error - manualRebalance function has multiple arithmetic bugs

Handle tabish Vulnerability details In short there are 2 errors in manualRebalance function : a ratio currentLockRatio has been compared to balance newLockRatio and at another point in the same function subtracted . 1 Expanding on the first one - checking newLockRatio Solution: Instead the if...

Lack of sufficient power check in updateValset of Gravity

Handle shw Vulnerability details Impact The updateValset function does not check whether the new valset has sufficient power to pass a vote see the constructor for more details. If the new valset does not, any function calling checkValidatorSignatures will be disabled since the transaction revert...

ERC20s that block transfer to particular addresses enable DoS/Censorship

Handle nascent Vulnerability details Tokens that prevent transfers to particular addresses most commonly address0 as is the OpenZeppelin standard enables DoS against a batch. If the attacker submits the bad transaction, the relayer wont submit the batch. The attacker never has to worry about the...

Owner has a rugpull function

Handle tensors Vulnerability details Impact The owner of the contract has a rugpull function. This can be unsafe if the private key for the owner account falls into the wrong hands, allowing instant withdrawal of all the funds. In general, having a single point of failure like this is not...

UberOwner has too much power

Handle tensors Vulnerability details Impact The Uber Owner has too much power within the system. This makes the protocol closer to a centralized prediction market whose rules are determined by the Uber Owner. Proof of Concept The above functions can be used by the Uber Owner to completely change...

Rewards squatting - setting rewards in different ERC20 tokens opens various economic attacks.

Handle moose-code Vulnerability details Impact Users have essentially have an option to either claim currently earned reward amounts on future rewards tokens, or the current rewards token. Although stated on line 84, it does not take into account the implications and lock in this contract will ha...

Incorrect use of latestMarket instead of marketIndex in several functions of LongShort

Handle shw Vulnerability details Impact Some part of the logic in the initializeMarket and seedMarketInitially functions of LongShort incorrectly operates on the latestMarket instead of marketIndex, the provided parameter. Since the latestMarket is not necessary to be the market to be initialized...

Incorrect balance computed in getUsersConfirmedButNotSettledSynthBalance()

Handle hack3r-0m Vulnerability details Consider the following state: longsynthbalace = 300; shortsynthbalace = 200; marketUpdateIndex1 = x; userNextPricecurrentUpdateIndex = 0; userNextPricesyntheticTokentoShiftAwayFrommarketSide1true = 0; batchedamountSyntheticTokentoShiftAwayFrommarketSide1true...

OracleManagerFlippening_V0 wrong decimals

Handle cmichel Vulnerability details The OracleManagerFlippeningV0.updatePrice function states that it wants to return the eth dominance as a percentage where 100% = 1e20. It's unclear why there is a division by 1e10 to compute the bitcoin market cap: uint256btcPrice btcSupply 1e10 Impact The pri...

Synths minted to the wrong market when initializing

Handle 0xImpostor Vulnerability details Impact Synthetix tokens are not minted to the correct market index since the creation of the synth market and the initialization are 2 separate steps. Proof of Concept 1. Create 2 synth market without initializing them 2. Call initializeMarket twice 3. Synt...

Router.zapLiquidity(uint256,address,address) has unchecked transfers

Handle heiho1 Vulnerability details Impact Router.zapLiquidityuint256,address,address has unchecked transfers on lines 65, 67 and 69. Several tokens do not revert in case of failure and return false. If one of these tokens is used in Router, deposit will not revert if the transfer fails, and an...

Unchecked transfers found in 3 contracts

Handle maplesyrup Vulnerability details Impact This is a high priority vulnerability because it definitely affects the way that funds are transferred and sent between the contracts. You want to make sure that you check the boolean value from these transfer functions in order to make sure that the...

Pool._addPoolMetrics(uint256) is subject to potential miner manipulation

Handle heiho1 Vulnerability details Impact Pool.addPoolMetricsuint256 on line 334 relies on block.timestamp and is potentially vulnerable to miner manipulation. This could lead to erroneous pool metrics. Proof of Concept Tools Used Slither Recommended Mitigation Steps An external time oracle like...

Missing slippage checks

Handle cmichel Vulnerability details The Router and Pool does not implement any slippage checks with comparing the swap / liquidity results with a minimum swap / liquidity value. Impact Users can be frontrun and receive a worse price than expected when they initially submitted the transaction...

Missing input validation in realise()

Handle 0xsanson Vulnerability details Impact In synth.sol, the function realisepool can be called using any existing pool as input. From my understanding, it's supposed that pool and synth must have the same underlying token. With the current implementation an user can call various synth contract...

Router.removeLiquidityExact(uint256,address) has unchecked transfers

Handle heiho1 Vulnerability details Impact Router.removeLiquidityExactuint256,address line 114 does not check the boolean return of a token transfer. This is a brittle implementation because it relies on the boolean return value being hard-coded to true. Tokens may not revert in case of failure a...

Total LP supply & total debt accrual is wrong

Handle cmichel Vulnerability details The total debt and total supply only increase when debt/supply is minted to the user when it should increase by the entire new interest amount on each accrual. function accrueAccountaddress account public distributeRewardaccount; // accrue only updates...

Use of deprecated Chainlink API

Handle 0xRajeev Vulnerability details Impact UniswapV3Oracle contract uses Chainlink’s deprecated API latestAnswer. Such functions might suddenly stop working if Chainlink stopped supporting deprecated APIs. Impact: Deprecated API stops working. Prices cannot be obtained. Protocol stops and...

ChainLink price data could be stale

Handle cmichel Vulnerability details There is no check in UniswapV3Oracle.ethPrice if the return values indicate stale data. This could lead to stale prices according to the Chainlink documentation: under current notifications: "if answeredInRound roundId could indicate stale data." under...

Lending Pair initialize function can be front run.

Handle jonah1005 Vulnerability details Impact LendingPair does not initialize tokenMaster, controller, tokens. A hacker can listen the deployer address and front run the initialize transaction. The initialized contract would look almost exactly the same if the hacker only replace lpTokenMaster wi...

Logic error in fulfill() function

Handle s1m0 Vulnerability details The fulfill function could create a loss for the protocol in case addFunds succeed and execute function fails. Impact In the fulfill function the toSend amount could be transferred twice: 1. In the external call addFunds to the txData.callTo. 2. If the execute...

Approval is not reset if the call to IFulfillHelper fails

Handle pauliax Vulnerability details Impact Function fulfill first approves the callTo to transfer an amount of toSend tokens and tries to call IFulfillHelper but if the call fails it transfers these assets directly. However, in such case the approval is not reset so a malicous callTo can pull...

Exposure.sortVaultsByDelta can underflow

Handle cmichel Vulnerability details Vulnerability Details The sortVaultsByDelta function performs an unsafe subtraction on two uint256 before casting them to int256. The subtraction can underflow and the cast to int256 can either fail and revert the transaction if greater than typeint256.max, or...

Buoy3Pool.safetyCheck is not precise and has some assumptions

Handle cmichel Vulnerability details Vulnerability Details The safetyCheck function has several issues that impact how precise the checks are: 1. only checks if the a/b and a/c ratios are within BASISPOINTS. By transitivity b/c is only within 2 BASISPOINTS if a/b and a/c are in range. For a more...

Unused return value from transfer()/transferFrom()

Handle JMukesh Vulnerability details Impact return value from transfer/transferFrom ensure success of the call and if not , it describe the reason for. 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...

Wrong price scale for GasOracle

Handle cmichel Vulnerability details The GasOracle uses two chainlink oracles GAS in ETH with some decimals, USD per ETH with some decimals and multiplies their raw return values to get the gas price in USD. However, the scaling depends on the underlying decimals of the two oracles and could be...

Malicious owner can arbitrarily change fee to any % value

Handle 0xRajeev Vulnerability details Impact Tracer protocol like any other allows market creators to charge fees for trades. However, a malicious/greedy owner can arbitrarily change fee to any % value and without an event to observe this change or a timelock to react, there is no easy way for...

Awarding takes reserve fee several times

Handle cmichel Vulnerability details The PrizePool.captureAwardBalance function takes fees repeatedly on the same interest. One would expect unaccountedPrizeBalance to be 0 in any repeated calls, but it's not. Assume the following example scenario with a 10% reserve fee: user calls...

maxSumOfPrices check is broken

Handle 0xRajeev Vulnerability details Impact rentAllCards requires the sender to specify a maxSumOfPrices parameter which specifies “limit to the sum of the bids to place” as specified in the Natspec @param comment. This is apparently for front-run protection. However, this function parameter...

Deposits can be denied by abusing maxContractBalance

Handle cmichel Vulnerability details Vulnerability Details The treasury implements a max contract balance check in the deposit function: require erc20.balanceOfaddressthis + amount This is not only restricted to whales, miners/users can do the same using same-block cross-transaction flashloans an...

Pot distribution does not need to add up to 100%

Handle cmichel Vulnerability details Vulnerability Details The Factory.setPotDistribution allows specifying values that add up to less than 100% because of the inequality = 1000 instead of an equality == 1000. Impact If using less than 100%, funds could become stuck in the market for certain mode...