rebalance function will fail due to invalid condition

Handle csanuragjain Vulnerability details Impact User will be unable to rebalance the fund Proof of Concept 1. Navigate to contract at 2. Check the rebalance function function rebalanceaddress tokenAddress, uint256 calldata percentages external override checkMarketSupportedtokenAddress onlyAdmin...

If newRatio > ibRatio after a settlement the protocol could lose its funds.

Handle tensors Vulnerability details Suppose that after a certain settleAuction call we have that newRatio ibRatio. I don't see any reason why this couldn't be possible, going through the math and solving for this condition we can see that: if b ibRatio then newRatio ibRatio assuming...

Two-step change of a swivel address

Handle pauliax Vulnerability details Impact MarketPlace allows an admin to change swivel to a different address. This function has no validations, even a simple check for zero-address is missing, and there is no validation of the new address being correct. If the admin accidentally uses an invali...

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

unchecked return value from approve()

Handle JMukesh Vulnerability details Impact contract uses an ERC20.approve call but does not check the success return value. Some tokens do not revert if the approval failed, returning false instead. The impact is that, tokens that don’t actually perform the approve and return false are still...

Unsafe cast in IndexPool mint leads to attack

Handle cmichel Vulnerability details The IndexPool.mint function performs an unsafe cast of ratio to the uint120 type: uint120 ratio = uint120divtoMint, totalSupply; Note that toMint is chosen by the caller and when choosing toMint = 2120 totalSupply / BASE, the ratio variable will be 2120 and th...

bondBurn() Leads to Unrecoverable Tokens

Handle leastwood Vulnerability details Impact An auction bonder has a single day's worth of blocks to call settleAuction and perform the necessary rebalance. In the event this does not happen, any user is able to call bondBurn and have the bond burnt in its entirety. As a result, the basketTokens...

Trying to use timestamps with blocknumbers

Handle goatbug Vulnerability details Impact requirebondTimestamp + ONEDAY block.number; There are require statements comparing timestamps to blocknumbers. We cannot assume one block per second, this code would not work on different chains where block times radically differ. The effect is having...

Bond div set to zero

Handle goatbug Vulnerability details Impact bondPercentDiv can be set to zero by the owner. This would give a div by zero error in line 61 bondAmount = basketToken.totalSupply / factory.bondPercentDiv; and brick any portfolio balancing ever. Proof of Concept Provide direct links to all referenced...

Owner can steal all Basket funds during auction

Handle 0xsanson Vulnerability details Impact The owner of Factory contract can modify the values of auctionMultiplier and auctionDecrement at any time. During an auction, these values are used to calculate newRatio and thereby tokensNeeded: specifically, it's easy to set the factory parameters so...

Risk of memory overflow

Handle 0xsanson Vulnerability details Impact It's possible to overflow the memory in multiple points in the contract, for example by having a long list of tokens and weights: function settleAuction uint256 memory bountyIDs, address memory inputTokens, uint256 memory inputWeights, address memory...

lack of checks in Factory::setBondPercentDiv allow owner to prevent bonding in Auction::bondForRebalance()

Handle GalloDaSballo Vulnerability details Impact setBondPercentDiv has no checks for min and max Setting bondPercentDiv to 0 will cause Auction::bondForRebalance to revert This allows the owner to prevent bonding by setting the bondPercentDiv to 0 Recommended Mitigation Steps Refactor to functio...

Reentrancy through withdrawBounty

Handle jonah1005 Vulnerability details Impact The function settleAuction Auction.solL69-L109 calls withdrawBounty. However, there's no safety checks in addBounty function.Auction.solL126-L138 The attacker can add malicious contract through addBounty and hijack the control flow of settleAuction...

The function addToken does not check if the token was already added

Handle hrkrshnn Vulnerability details addToken does not check if the token was already added The function addToken does not check if the token was already present. function addToken address vault, address token external override notHalted onlyStrategist requireallowedTokenstoken, "!allowedTokens"...

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

Last person to withdraw his tokens might not be able to do this, in Crowdsale (edge case)

Handle gpersoon Vulnerability details Impact Suppose a Crowdsale is successful and enough commitments are made before the marketInfo.endTime. Suppose marketStatus.commitmentsTotal == marketInfo.totalTokens -1 // note this is an edge case, but can be constructed by an attacker Then the function...

Vault.balance() mixes normalized and standard amounts

Handle cmichel Vulnerability details The Vault.balance function uses the balanceOfThis function which scales "normalizes" all balances to 18 decimals. for uint8 i; i tokens.length; i++ address token = tokensi; // everything is padded to 18 decimals balance = balance.addnormalizeDecimalstoken,...

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

ChainLink price data could be stale

Handle cmichel Vulnerability details Vulnerability Details There is no check in ExchangeRate.buildExchangeRate 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 indicat...

Liquidity token value can be manipulated

Handle cmichel Vulnerability details Vulnerability Details The liquidity token value AssetHandler.getLiquidityTokenValue is the sum of the value of the individual claims on cash underlying or rather cTokens and fCash. The amount to redeem on each of these is computed as the LP token to redeem...

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

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

Market whitelist does not work

Handle cmichel Vulnerability details The RCTreasury.marketWhitelistCheck function gets the marketWhitelistmsgSender variable and performs a special check if it's non-zero. However, there's no way to set the whitelist in the first place making this function unnecessary. Impact The market whitelist...

findNewOwner edgecase

Handle gpersoon Vulnerability details Impact In the function findNewOwner of RCOrderbook, as loop is done which included the check loopCounter maxDeletions Afterwards a check is done for "loopCounter != maxDeletions" to determine if the processing is finished. If loopCounter == maxDeletions then...

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

ERC20Rewards returns wrong rewards if no tokens initially exist

Handle cmichel Vulnerability details The ERC20Rewards.updateRewardsPerToken function exits without updating rewardsPerToken.lastUpdated if totalSupply is zero, i.e., if there are no tokens initially. This leads to an error if there is an active rewards period but not tokens have been minted yet...

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

OracleManagerEthVsBtc price data could be stale

Handle cmichel Vulnerability details There is no check in OracleManagerEthVsBtc.getLatestPrice 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."...

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

Token price should not be set manually.

Handle tensors Vulnerability details Impact The Manager.sol file contains many methods to let Watsons manually set the token price. This should never be done, and gives free incentives for malicious users to arbitrage price discrepancies from the pool. Proof of Concept In general, these price...

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. Some tokens charge a certain fee for every transfer or transferFrom. Others types are rebasing tokens that increase in value over time like Aave's aTokens balanceOf changes...

Incorrect internal balance bookkeeping

Handle walker Vulnerability details type: Incorrect Assumptions on External Systems The sherlock smart contract system uses internal bookkeeping of arbitrary ERC20 token balances. It doesn't assert that the ERC20 doesn't implement some non-standard behaviour. For example, deflationary tokens, or...

Flash loan manipulation on getPoolShareWeight of Utils

Handle shw Vulnerability details Impact The getPoolShareWeight function returns a user's pool share weight by calculating how many SPARTAN the user's LP tokens account for. However, this approach is vulnerable to flash loan manipulation since an attacker can swap a large number of TOKEN to SPARTA...

Return values of BEP20.transfer and BEP20.transferFrom are unchecked

Handle shw Vulnerability details Impact The return values of BEP20.transfer and BEP20.transferFrom are not checked to be true in multiple contracts. The return value could be false if the transferred token is not BEP20-compliant, indicating that the transfer fails, while the calling contract will...

Synth: approveAndCall sets unnecessary approval

Handle cmichel Vulnerability details Vulnerability Details The Synth.approveAndCall function approves the recipient contract with the max value instead of only the required amount. Impact For safety, the approval should not be set to the max value, especially if the amount that the contract may u...

Dao has unchecked transfers

Handle heiho1 Vulnerability details Impact Dao.moveBASEBalanceaddress and Dao.handleTransferInaddress,uint256 do not check transfer return values on lines 220 and 266. If the token implementation returns false and does not revert, then potentially the user can deposit for free. Proof of Concept...

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

Router.swapSynthToAsset(uint256,address,address) has unchecked transfer

Handle heiho1 Vulnerability details Impact Router.swapSynthToAssetuint256,address,address on line 249 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 and not revert in which...

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

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

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

distributeStrategyGainLoss can be abused

Handle gpersoon Vulnerability details Impact The function distributeStrategyGainLoss does the following check to allow access to the function: requireindex 0 || index = NCOINS + 1, "!VaultAdaptor"; However the expression index 0 || index = NCOINS + 1 is always TRUE, because the OR || is used shou...

Return values of ERC20 transfer and transferFrom are unchecked

Handle shw Vulnerability details Impact In some contracts e.g., TracerPerpetualSwaps.sol, 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. In that case, the transfer fails without being noticed...

Use of incorrect index leads to incorrect updation of funding rates

Handle 0xRajeev Vulnerability details Impact The updateFundingRate function updates the funding rate and insurance funding rate. While the instant/new funding rates are calculated correctly, the cumulative funding rate calculation is incorrect because it is always adding the instant to 0, not the...

No check transferFrom() return value

Handle s1m0 Vulnerability details Impact The smart contract doesn't check the return value of token.transfer and token.transferFrom, some erc20 token might not revert in case of error but return false. In the TracerPerpetualSwaps:deposit and Insurance:deposit this would allow a user to deposit fo...

Single-step process for critical ownership transfer

Handle 0xRajeev Vulnerability details Impact The Tracer Perpetuals Factory contract is arguably the most critical contract in the project given that it deploys all the markets. The ownership of this contract is transferred to governance address, i.e. TracerDAO, in the constructor. This critical...

Problems with non-standard compliant ERC20 tokens

Handle cmichel Vulnerability details 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...

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

Malicious user can trigger another user’s removal

Handle 0xRajeev Vulnerability details Impact By allowing anyone to call removeUserFromOrderbook instead of only Market::newRental or Treasury::withdrawDeposit or collectRentUser which may result in foreclosures and hence may need to trigger user removal, a malicious user can trigger another's...

Flows can bypass market and global pause

Handle 0xRajeev Vulnerability details Impact Ability to pause all token transfers and all state changes for contracts is a “guarded-launch” best-practice for emergency situations for newly launched projects. The project implements this using a marketsPaused flag per market and a globalPause flag...