Vault.withdraw sometimes burns too many shares

Handle cmichel Vulnerability details The Vault.withdraw function attempts to withdraw funds from the controller if there are not enough in the vault already. In the case the controller could not withdraw enough, i.e., where diff toWithdraw, the user will receive less output tokens than their fair...

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

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

Controller.withdrawAll sets wrong vault balance

Handle cmichel Vulnerability details The Controller.withdrawAll decreases the vault balance by amount, the want token amount that has been withdrawn from the strategy and transferred to the vault. Note that amount gets overwritten in the convert != address0 branch and is a convert token value...

convert fails for fee-on-transfer tokens

Handle 0xsanson Vulnerability details Impact The Controller contract can call converter.convert inside earn and withdraw functions, after transferring amount of tokens to the Converter contract. This contract assumes that it has received exactly amount tokens, however this isn't true for...

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

PostAuctionLauncher.sol#finalize() Adding liquidity to an existing pool may allows the attacker to steal most of the tokens

Handle WatchPug Vulnerability details PostAuctionLauncher.finalize can be called by anyone, and it sends tokens directly to the pair pool to mint liquidity, even when the pair pool exists. An attacker may control the LP price by creating the pool and then call finalize to mint LP token with unfai...

Insufficient validation of rate value

Handle JMukesh Vulnerability details Impact it lack the checking of the value that it is fresh or not, if data is not fresh it can affect exchange rate. these following condition should be met to ensure that data is fresh requireupdateTime != 0, "Incomplete round"; requireansweredInRound = roundI...

DAO proposals can be executed by anyone due to vulnerable TimelockController

Handle cmichel Vulnerability details Vulnerability Details The GovernorAlpha inherits from a vulnerable TimelockController. This TimelockController allows an EXECUTOR role to escalate privileges and also gain the proposer role. See details on OZ and the fix here. The bug is that executeBatch chec...

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

Freeze Bridge via Non-UTF8 Token Name/Symbol/Denom

Handle nascent Vulnerability details Manual insertion of non-utf8 characters in a token name will break parsing of logs and will always result in the oracle getting in a loop of failing and early returning an error. The fix is non-trivial and likely requires significant redesign. Proof of Concept...

Freeze The Bridge Via Large ERC20 Names/Symbols/Denoms

Handle nascent Vulnerability details Ethereum Oracles watch for events on the Gravity.sol contract on the Ethereum blockchain. This is performed in the checkforevents function, ran in the ethoraclemainloop. In this function, there is the following code snippet: let erc20deployed = web3...

Settle Portfolio state could be griefed.

Handle tensors Vulnerability details Impact It could be possible for a user to get a portfolioState that is large enough to be unfeasible to compute, either because of the block gas limit or simply because gas fees are so high and the code itself is complex. If this is possible, than he could tak...

Can a small order change the lastImpliedRate significantly?

Handle tensors Vulnerability details Impact Consider the following attack vector. An attacker risks a very small amount of capital $0.01, for example to alter the lastImpliedRate, losing the $0.01 by executing a very bad trade. If no one is willing to arbitrage the rate down for the $0.01, or no...

Reentrancy Bug in TimelockController.sol

Handle leastwood Vulnerability details Impact Notional's governance framework utilises a fork of Compound's Governor Alpha and ERC20 token. These are denoted specifically as the GovernorAlpha.sol and NoteERC20.sol contracts. However, the GovernorAlpha.sol has a key difference when compared to...

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

ERC20Rewards breaks when setting a different token

Handle cmichel Vulnerability details The setRewards function allows setting a different token. Holders of a previous reward period cannot all be paid out and will receive their old reward amount in the new token. This leads to issues when the new token is more less valuable, or uses different...

TimeLock cannot schedule the same calls multiple times

Handle cmichel Vulnerability details The TimeLock.schedule function reverts if the same targets and data fields are used as the txHash will be the same. This means one cannot schedule the same transactions multiple times. Impact Imagine the delay is set to 30 days, but a contractor needs to be pa...

Missing events/timelocks for owner/admin only functions that change critical parameters

Handle 0xRajeev Vulnerability details Impact Owner/admin only functions that change critical parameters should emit events and have timelocks. Events allow capturing the changed parameters so that off-chain tools/interfaces can register such changes with timelocks that allow users to evaluate the...

Incorrect parameters passed while adding new staking fund

Handle hack3r-0m Vulnerability details initializeMarket can be called with different marketIndex each time while calling IStakerstaker.addNewStakingFund with the same parameters resulting in overriding of mapping in the staker contract and hence removing past staking funds. latestMarket should be...

Use safeTransfer instead of transfer

Handle shw Vulnerability details Impact Tokens not compliant with the ERC20 specification could return false from the approve call to indicate the approval fails, while the calling contract would not notice the failure if the return value is not checked. Proof of Concept Referenced code:...

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

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

Staker.sol: Wrong values returned in edge cases of _calculateFloatPerSecond()

Handle hickuphh3 Vulnerability details Impact In calculateFloatPerSecond, the edge cases where full rewards go to either the long or short token returns return 1e18 k longPrice, 0; and return 0, 1e18 k shortPrice; respectively. This is however 1e18 times too large. We can verify this by checking...

The approveMax function of MStableYieldSource always reverts

Handle shw Vulnerability details Impact The approveMax function of MStableYieldSource calls the safeApprove function to set the allowance to the maximum. However, at the time of call, the allowance should be non-zero since it was set to the maximum in the constructor function. The non-zero...

Usage of safeApprove

Handle pauliax Vulnerability details Impact function approveMax uses safeApprove. This function only works if the current approval is 0. Consider clearing previous approval safeApprove0 before setting the max value again. The same issue can happen with SwappableYieldSource if, for example, source...

Inconsistent balance when supplying transfer-on-fee or deflationary tokens

Handle shw Vulnerability details Impact The supplyTokenTo function of SwappableYieldSource assumes that amount of depositToken is transferred to itself after calling the safeTransferFrom function and thus it supplies amount of token to the yield source. However, this may not be true if the...

Transfer-on-fee/deflationary tokens are not correctly accounted for

Handle shw Vulnerability details Impact When a user stakes or a protocol deposits a transfer-on-fee/deflationary token, the solution does not correctly handle the received amount, which could be less than what is accounted for. Proof of Concept Referenced code: PoolOpen.solL36-L38...

Difficult for the project to be decentralized if the Watsons share one address.

Handle tensors Vulnerability details Impact The Watsons share a single address. As it stands right now the Watsons could be a single person effectively providing insurance with other peoples risk. There should be mechanisms in place to make sure Watson's have an accurate amount of skin in the gam...

Broken access control leads to protocol functionality freeze

Handle 0xRajeev Vulnerability details Impact The contracts use an access control pattern where the contract deployer is included in the onlyDAO modifier which is used for authorized access to critical functions. Such contracts also include a purgeDeployer function which renounces sets to...

Misplaced logic leads to loss of member bondedLP funds

Handle 0xRajeev Vulnerability details Impact In claimForMember, the member claims back some of their bonded LPs. The check to see if claimRate can be made 0 should preceed the claimable deduction on L110. This misplaced check after deduction leads to incorrect zero-ing of member’s non-zero...

Dividend reward can be gamed

Handle cmichel Vulnerability details The Router.addDividend function tells the reserve to send dividends to the pool depending on the fees. The attacker provides LP to a curated pool. Ideally, they become a large LP holder to capture most of the profit, they should choose the smallest liquidity...

Router.removeLiquiditySingle(uint256,bool,address) has unchecked transfers

Handle heiho1 Vulnerability details Impact Router.removeLiquiditySingleuint256,bool,address on lines 121, 126, 129 ignores the boolean return on transfers. This is a brittle implementation because it relies on the boolean return value being hard-coded to true. Tokens may return false instead of...

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

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

Owner can burn other users token shares

Handle JMukesh Vulnerability details Impact This privilege should not be given to the owner, which can burn other users token shares, due to this user will not be able to claim their liquidity. Due to this privilege user will lost control over liquidity amount Proof of Concept Tools Used manual...

addLiquidity transfers tokens from the wrong sender

Handle pauliax Vulnerability details Impact In addLiquidity function, a router is passed as a sender in LibERC20.transferFrom, not msg.sender, so it basically transfers assets from the router to the contract. Recommended Mitigation Steps requireLibERC20.transferFromassetId, msg.sender, addressthi...

Expired transfers will lock user funds on the sending chain

Handle 0xRajeev Vulnerability details Impact The cancelling relayer is being paid in receivingAssetId on the sendingChain instead of in sendingAssetID. If the user relies on a relayer to cancel transactions and that receivingAssetId asset does not exist on the sending chain assuming only...

addFunds and execute may send tokens twice

Handle pauliax Vulnerability details Impact Both calls to IFulfillHelper addFunds and execute are wrapped in separate try/catch statements so basically if addFunds succeeds but execute fails or both of these functions fail, the catch will still send assets to the receivingAddress. I think these...

Signatures use only tx ID instead of entire digest

Handle 0xRajeev Vulnerability details Impact The signature check in recoverFulfillSignature only uses transaction ID along with the relayer fee which can be accidentally reused by the user, in which case the older signatures with the older relayer fees can be replayed. The signature should be on...

User prepare can be denied

Handle cmichel Vulnerability details Vulnerability Details Upon observing a prepare transaction, an attacker can frontrun it with the same invariantData but an amount of a single wei. This inserts a value into variantTransactionDatadigest and the original transcation will fail because of the...

Use latestRoundData instead of latestAnswer

Handle adelamo Vulnerability details Impact Use latestRoundData instead of latestAnswer for chainlink Oracles in order to be able to run more validations like roundId, rawPrice, , updateTime, answeredInRound = AggregatorV3Interfacesource.source.latestRoundData; requirerawPrice 0, "Chainlink price...

Return values of ERC20 transfer and transferFrom are unchecked

Handle shw Vulnerability details Impact In the contracts BadgerYieldSource and SushiYieldSource, the return values of ERC20 transfer and transferFrom are not checked to be true, which could be false if the transferred tokens are not ERC20-compliant e.g., BADGER. In that case, the transfer fails...

SafeMath not completely used in yield source contracts

Handle shw Vulnerability details Impact SafeMath is not completely used at the following lines of yield source contracts, which could potentially cause arithmetic underflow and overflow: 1. line 78 in SushiYieldSource 2. line 67 in BadgerYieldSource 3. line 91 and 98 in IdleYieldSource Proof of...

YieldSourcePrizePool_canAwardExternal does not work

Handle cmichel Vulnerability details The idea of YieldSourcePrizePoolcanAwardExternal seems to be to disallow awarding the interest-bearing token of the yield source, like aTokens, cTokens, yTokens. "@dev Different yield sources will hold the deposits as another kind of token: such a Compound's...

Yearn vault withdrawals in redeems will always fail leading to lock/loss of user deposits

Handle 0xRajeev Vulnerability details Impact The withdrawFromVault calculates the token balance of contract before withdrawal and saves it in previousBalance. It then withdraws from the Yearn vault and calculates the token balance after withdrawal to save it in currentBalance. So currentBalance...

BadgerYieldSource balanceOfToken share calculation seems wrong

Handle cmichel Vulnerability details When suppling to the BadgerYieldSource, some amount of badger is deposited to badgerSett and one receives badgerSett share tokens in return which are stored in the balances mapping of the user. So far this is correct. The balanceOfToken function should then...

Market-specific pause is not checked for sponsor

Handle cmichel Vulnerability details Vulnerability Details The treasury only checks its globalPause field but does not check its market-specific marketPaused field for Treasury.sponsor. A paused market contract can therefore still deposit as a sponsor using Market.sponsor Impact The market-specif...

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