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

TridentRouter.addLiquidity() Add liquidity to IndexPool through TridentRouter may casue loss of a small portion of users funds

Handle WatchPug Vulnerability details The mint function in IndexPool requires the liquidity provider to transfer in amounts no less than the amounts of tokens' reserve proportionally to the toMint amount. However, the TridentRouter won't calculate the toMint amount and amountsIn for the liquidity...

UniswapV3Helper: getSqrtPriceX96() doesn't work for tokens with non-18 decimals

Handle hickuphh3 Vulnerability details Impact The getSqrtPriceX96 will return incorrect values for pairs comprising of non-18 decimals. This affects the amounts calculated for a position. Proof of Concept Let us take the ETH-WBTC pair as an example. Note that WBTC has 8 decimals, and is an active...

onlyOwner Role Can Manipulate Price Oracle

Handle leastwood Vulnerability details Impact The V2 LendingController.sol contract incorporates a custom oracle used to fetch the relevant price feeds for token pairs. The onlyOwner role is controlled by a modified timelock contract callable only from a single externally owned account. This role...

approveUnderlying isn't safe

Handle 0xsanson Vulnerability details Impact In Basket.sol, approveUnderlying is used to approve tokens to be spent by the Auction. The current implementation uses a simple approve function, instead of the safer safeApprove. Also it's recommended to have an approve to zero first, since the...

Zero fees

Handle goatbug Vulnerability details Impact The fees system is broken. Anyone can easily pay zero fees. Proof of Concept Create a bundle of two txs to be mined in one block. First tx calls mintTo with an extrememly small amount, handleFees is called and importantly lastFee = block.timestamp; Seco...

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

Auction multiplier set to zero

Handle goatbug Vulnerability details Impact function setAuctionMultiplieruint256 newAuctionMultiplier public override onlyOwner auctionMultiplier = newAuctionMultiplier; auction multiplier can be set to zero by factory owner. This would stop the auction settling, function would always revert...

missing access control in basket.sol

Handle jah Vulnerability details Impact function mint and function minTo are not protected so anyone can mint Proof of Concept Tools Used manual analysis Recommended Mitigation Steps use modifier to check who can call the function --- The text was updated successfully, but these errors were...

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

Vault may not have enough tokens for withdraw

Handle 0xRajeev Vulnerability details Impact There is an assumption in LegacyController.vault that the vault will have enough tokens0 to cover the balance difference. If not, the user may receive less than amount requested and balance funds get lost/locked unless the vault withdraws from the...

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

erc20 transfer and transferFrom functions

Handle pauliax Vulnerability details Impact When transfering erc20 tokens, functions transfer and transferFrom are used. These functions return boolean to indicate if the action was successful, however, none of the usages check the returned value: erc20.transferFrommsgSender, addressthis, amount;...

anyone can call function sponsor

Handle pauliax Vulnerability details Impact This function sponsor should only be called by the factory, however, it does not have any auth checks, so that means anyone can call it with an arbitrary sponsorAddress address and transfer tokens from them if the allowance is 0: /// @notice ability to...

Unchecked return value of transferFrom in function timeLockERC20

Handle shw Vulnerability details Impact In the function timeLockERC20 line 610, the return value of IERC20.transferFrom is unchecked. The return value could be false if the transferred token is not ERC20-compliant, indicating that the transfer fails. In that case, the variable timelockERC20Balanc...