An attacker can steal funds from multi-token vaults

Handle WatchPug Vulnerability details The total balance should NOT be simply added from different tokens' tokenAmounts, considering that the price of tokens may not be the same. function balanceOfThis public view returns uint256 balance address memory tokens = manager.getTokensaddressthis; for...

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

Missing validation on latestRoundData

Handle adelamo Vulnerability details On ExchangeRate.sol, we are using latestRoundData, but there are no validations that the data is not stale. The current code is: / uint80 /, rate, / uint256 /, / uint256 /, / uint80 / = AggregatorV2V3InterfacerateOracle.latestRoundData; requirerate 0,...

Crash Eth Oracle On Any LogicCallEvent

Handle nascent Vulnerability details Severity: Medium Likelihood: High In ethoraclemainloop, getlastcheckedblock is called. Followed by: let logiccallexecutedevents = web3 .checkforevents endsearch.clone, Somecurrentblock.clone, vec!gravitycontractaddress, vec!LOGICCALLEVENTSIG, .await; and may h...

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

Can't call external functions internally

Handle tensors Vulnerability details Impact Within GovernerAlpha.sol, certain functions are locked because of the improper modifier. addressthis cannot call external functions in lines L470-497. Proof of Concept Recommended Mitigation Steps Change external to internal. --- The text was updated...

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

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

initialMarket always initialize the latest market

Handle jonah1005 Vulnerability details Impact In longshort contract's initializeMarket, while it should initialize according to the parameter marketIndex, it initialize latestMarket. This would break two market, the market of marketIndex' and the latest market. User's fund would get stuck at the...

Wrong aave usage of claimRewards

Handle jonah1005 Vulnerability details Impact Aave yield manager claims rewards with the payment token. According to aave's document, aToken should be provided. The aave rewards would be unclaimable. Proof of Concept YieldManager's logic: Reference: Tools Used None Recommended Mitigation Steps...

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

Potential underflow on userAmountStaked[token][msg.sender] in _withdraw

Handle 0xImpostor Vulnerability details Impact Underflowing userAmountStakedtokenmsg.sender once will let me exploit the entire token balance in the Staker contract. This can only be exploited if marketUnstakeFeee18 is ≥ 50%. Proof of Concept 1. Admin sets marketUnstakeFeee18 for this marketIndex...

getUsersConfirmedButNotSettledSynthBalance is potentially calculated wrongly

Handle 0xImpostor Vulnerability details Impact Incorrect tabulation of getUsersConfirmedButNotSettledSynthBalance will lead to the wrong balances returning. Fortunately, there are no important functions that are dependent on balanceOf so the impact of this erroneous calculation is limited. Tools...

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

Deposits don't work with fee-on transfer tokens

Handle cmichel Vulnerability details There are ERC20 tokens that may make certain customizations to their ERC20 contracts. One type of these tokens is deflationary tokens that charge a certain fee for every transfer or transferFrom. Others are rebasing tokens that increase in value over time like...

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

supplyTokenTo doesn't account for safeTransferFrom fees

Handle gpersoon Vulnerability details Impact The function supplyTokenTo of MStableYieldSource retrieves the tokens from the msg.sender and deposits them. However some tokens, like USDT might subtract a fee when transferring tokens. This means less tokens would be transferred than expected. If thi...

[Gov.sol] Ignoring the return value of function _token.approve(...)

Handle eriksal1217 Vulnerability details Impact Medium Risk vulnerability - This does not immediately affect the contract, tokens, or funds associated but could have negative effects in regards to how the contract behaves when executing this functionality. Proof of Concept According to Slither...

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

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

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

No ERC20 safe* versions called

Handle cmichel Vulnerability details Some tokens like USDT don't correctly implement the EIP20 standard and their transfer/transferFrom function return void instead of a success boolean. Calling these functions with the correct EIP20 function signatures will always revert. This is generally not a...

SynthVault withdraw forfeits rewards

Handle cmichel Vulnerability details Vulnerability Details The SynthVault.withdraw function does not claim the user's rewards. It decreases the user's weight and therefore they are forfeiting their accumulated rewards. The synthReward variable in processWithdraw is also never used - it was probab...

LendingPair.liquidateAccount fails if tokens are lent out

Handle cmichel Vulnerability details The LendingPair.liquidateAccount function tries to pay out underlying supply tokens to the liquidator using safeTransferIERC20supplyToken, msg.sender, supplyOutput but there's no reason why there should be enough supplyOutput amount in the contract, the contra...

Unused return value from transfer()

Handle JMukesh Vulnerability details Impact 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 token reverts in case of a failure. Proof of Concept Tools Used manual review Recommended Mitigation Steps che...

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

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

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

Trader orders can be frontrun and users can be denied from trading

Handle cmichel Vulnerability details The Trader contract accepts two signed orders and tries to match them. Once they are matched and become filled, they can therefore not be matched against other orders anymore. This allows for a griefing attack where an attacker can deny any other user from...

No support for token with decimals > 18

Handle s1m0 Vulnerability details Impact The smart contract doesn't behave correctly if deployed with token that have decimals 18. Proof of Concept The functions tokenToWad and wadToToken revert if the tokenDecimals is 18. These functions are called in critical places like deposit and withdraw...

prb-math not audited

Handle gpersoon Vulnerability details Impact The library prb-math documents that it is not audited by a security researcher. This means its more risky to rely on this library. Proof of Concept // The contracts have not been audited by a security researcher. Tools Used Recommended Mitigation Steps...

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

Using transferFrom on ERC721 tokens

Handle shw Vulnerability details Impact In the function awardExternalERC721 of contract PrizePool, when awarding external ERC721 tokens to the winners, the transferFrom keyword is used instead of safeTransferFrom. If any winner is a contract and is not aware of incoming ERC721 tokens, the sent...

YearnV2YieldSource wrong subtraction in withdraw

Handle cmichel Vulnerability details YearnV2YieldSource.withdrawFromVault uses a wrong subtraction. When withdrawing from the vault one redeems yTokens for tokens, thus the token balance of the contract should increase after withdrawal. But the contract subtracts the currentBalance from the...

Test configuration carryover to production deployment limits pool users to 5

Handle 0xRajeev Vulnerability details Impact The project uses a data structure for indexing ticket tokens/users called SortitionSumTreeFactory which as explained in the overview video time 14:20-14:50 is used to capture users’ token balances in the leaves where internal nodes represent their sums...

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

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

Missing checkOnERC721Received deviates from ERC721 and could lock/lose NFTs

Handle 0xRajeev Vulnerability details Impact ERC721 specification for safeTransferFrom says: “this function checks if to is a smart contract code size 0. If so, it calls onERC721Received on to and throws if the return value is not bytes4keccak256“onERC721Receivedaddress,address,uint256,bytes”.”...

Missing threshold check on critical protection mechanism minRentalDayDivisor

Handle 0xRajeev Vulnerability details Impact Minimum rental duration is acknowledged as one of the two critical protection mechanisms for the market functioning. The setMinRental is called from the constructor with 246 which sets the minimum duration to 10 minutes. However, a threshold check is...

Market-specific pause is not checked for payout

Handle cmichel Vulnerability details Vulnerability Details The treasury only checks its globalPause field but does not check its market-specific marketPaused field for Treasury.payout. A paused market contract can therefore still pay out using payArtist, payCardAffiliate, payMarketCreator,...

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

Unchecking the ownership of mph in function distributeFundingRewards could cause several critical functions to revert

Handle shw Vulnerability details Impact In contract MPHMinter, the function distributeFundingRewards does not check whether the contract itself is the owner of mph. If the contract is not the owner of mph, mph.ownerMint could revert, causing functions such as withdraw, rolloverDeposit,...

Unbounded loop in function transferERC721

Handle shw Vulnerability details Impact The function transferERC721 loops over an unbounded array, timelockERC721KeysnftContract, whose length never decreases but increases whenever the owner locks an ERC721 token. Therefore, the required gas for executing this loop grows over time and could reac...

Anyone can call onERC721Received() function and spam the array "nfts"

Handle Sherlock Vulnerability details Impact An attacker can deal direct economic damage to the owner/delegate spending some gas to spam the array of "nfts" with different values. It will be more costly to remove these nfts one-by-one, transaction-by-transaction. Also, it makes other functions...

Unbounded iteration

Handle cmichel Vulnerability details Vulnerability Details The Visor.removeNft iterates over all nfts. Anyone can add to this array by depositing NFTs, see Visor.onERC721Received. Other occurences that makes an unbounded iteration over arrays: Visor.getBalanceLocked Visor.getNftIdByTokenIdAndAddr...

Missing nonReentrant in swapTo

Handle adelamo Vulnerability details Here you have more info: --- The text was updated successfully, but these errors were encountered: All reactions...

Missing SafeMath

Handle adelamo Vulnerability details Here more info: --- The text was updated successfully, but these errors were encountered: All reactions...