Unsafe casts to uint120

Handle pauliax Vulnerability details Impact contract IndexPool does unsafe casts to uint120, e.g.: uint120 ratio = uint120divtoMint, totalSupply; It trusts the input from the user and converts without assurance that the value will fit in this narrow type. Depending on the pool state and parameter...

_depositToBentoBox sometimes uses both ETH and WETH

Handle cmichel Vulnerability details The TridentRouter.depositToBentoBox function only uses the ETH in the contract if it's higher then the desired underlyingAmount addressthis.balance = underlyingAmount. Otherwise, the ETH is ignored and the function uses WETH from the user. Impact Note that the...

IndexPool performs several unsafe type casts

Handle cmichel Vulnerability details The IndexPool.burn/burnSingle/swap/flashSwap functions all perform unsafe type casts to uint120 without checking if the values actually fit into 120 bits. Impact Unlike mint, these functions have other require statements and checks that don't seem to directly...

Initialize can be called multiple times!!!!

Handle pants Vulnerability details Initialize can be called multiple times. Initialize function must be called only by the contract owner and only once for safety. line 74 --- The text was updated successfully, but these errors were encountered: All reactions...

HybridPool's flashSwap sends entire fee to barFeeTo

Handle cmichel Vulnerability details The HybridPool.flashSwap function sends the entire trade fees fee to the barFeeTo. It should only send barFee fee to the barFeeTo address. Impact LPs are not getting paid at all when this function is used. There is no incentive to provide liquidity. Recommende...

IndexPool does not apply barFee

Handle pauliax Vulnerability details Impact IndexPool does not mint fees barFee and barFeeTo. It initializes these variables but never uses them in a meaningful way. Recommended Mitigation Steps Consider handling the fees in this contract similarly to other pools. --- The text was updated...

No bar fees for IndexPools?

Handle 0xsanson Vulnerability details Impact IndexPool doesn't collect fees for barFeeTo. Since this Pool contains also a method updateBarFee, probably this is an unintended behavior. Also without a fee, liquidity providers would probably ditch ConstantProductPool in favor of IndexPool using the...

IndexPool.INIT_POOL_SUPPLY is too large, which may cause a significant amount of fund loos to the first liquidity provider

Handle WatchPug Vulnerability details uint256 internal constant BASE = 1018; ... uint256 internal constant INITPOOLSUPPLY = BASE 100; /// @dev Mints LP tokens - should be called via the router after transferring bento tokens. /// The router must ensure that sufficient LP tokens are minted by usin...

IndexPool's flashswap trasfer before callback

Handle 0xsanson Vulnerability details Impact The flashswap function in IndexPool.sol doesn't fulfill its function. Indeed it should transfer tokens to the users before they need to pay back, but the transfer happens at the end: ... ITridentCalleemsg.sender.tridentSwapCallbackcontext; // @dev Chec...

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

Missing timelock for critical contract setters of privileged roles

Handle 0xRajeev Vulnerability details Impact Setter functions for critical protocol parameters accessible only by privileged roles e.g. onlyOwner should consider adding timelocks so that users and other privileged roles in the case of a multiSig can detect upcoming changes and have the time to...

First lp provider in Index Pool is vulnerable to MEV

Handle broccoli Vulnerability details Impact In the IndexPool contract, the first lp providers have to deposit the same amount of tokens. This creates arbitrage space. If the deployer tries to deploy a BTC/DAI pool. S/He has to initialize the BTC pool with BTC price = 1. I consider this is a...

absolute difference is not calculated properly when a > b in MathUtils

Handle hack3r-0m Vulnerability details the difference is computed incorrectly when a b. As it only used in within1 function, scope narrows down to where differencea, b It is possible to decrease the denominator and increase the value of the numerator when calculating y using constants and input t...

User’s Ether may get left behind in the Router

Handle 0xRajeev Vulnerability details Impact If the user deposits less ETH than claimed by the params.tokenIn for native functions, then wETH is transferred from the msg.sender to pool, while the user's ETH is left behind in the Router. Proof of Concept Tools Used Manual Analysis Recommended...

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

UniswapV3Oracle.sol Does Not Use latestRoundData() Instead of latestAnswer()

Handle leastwood Vulnerability details Impact This issue was submitted in WildCredit's previous audit but has not been fixed, so raising the issue again. latestAnswer is used in UniswapV3Oracle.sol over latestRoundData to get the latest price data for WETH. There are no checks to ensure the data...

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

DDoS of rebalance

Handle goatbug Vulnerability details Impact Given a bondDiv of say 400, it requires 0.25% of a portfolios capital to perform a Ddos attack. Portfolio rebalancing is often a time sensitive task. New weights need to be put into place to take advantage of shifts in capital market expectations etc...

If newRatio can differ drastically from the old ibRatio attacks could take place

Handle tensors Vulnerability details Impact Consider L91 in Auction.sol. I'm not sure what the logic for updating the ibRatio in this case is. If the newRatio is larger/smaller than the old ibRatio than a user can mint/burn tokens in the basket contract, update the ratio, then burn/mint the...

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

Incorrect data location specifier can be abused to cause DoS and fund loss

Handle 0xRajeev Vulnerability details Impact The withdrawBounty loops through the bounties array looking for active bounties and transferring amounts from active ones. However, the data location specifier used for bounty is memory which makes a copy of the bounties array member instead of a...

burn and mintTo in Basket.sol vulnerable to reentrancy

Handle 0xalpharush Vulnerability details Impact The functions mintTo and burn make external calls prior to updating the state. If a basket contains an ERC777 token, attackers can mint free basket tokens. Proof of Concept An attacker could reenter the mintTo function when the contract pulls an...

Fee calculation is potentially incorrect

Handle itsmeSTYJ Vulnerability details Impact More fees are actually charged than intended Mitigation Steps Basket.sol line 118 Assume that license fee is 10% i.e. 1e17 and time diff = half a year. When you calculate feePct, you expect to get 5e16 since that's 5% and the actual amount of fee to b...

Re-entrancy in burn()

Handle hack3r-0m Vulnerability details anyone can propose basket and hence one can create basket with his/her choice of tokens, out of which some can be malicious. attacker can create sequence of tokens as WETH1, DAI2, USDC3 and Malicious contract4 and can drain all funds. After calling burn, at...

withdrawBounty the same bounty multiple times

Handle pauliax Vulnerability details Impact The same bounty can be claimed multiple times as it remains active. 'bounty' points to the memory variable, so its state is not persisted: Bounty memory bounty = bountiesbountyIdsi; requirebounty.active; IERC20bounty.token.transfermsg.sender,...

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

Basket.sol#auctionBurn() A failed auction will freeze part of the funds

Handle WatchPug Vulnerability details Given the auctionBurn function will burn the auction bond without updating the ibRatio. Once the bond of a failed auction is burned, the proportional underlying tokens won't be able to be withdrawn, in other words, being frozen in the contract. Proof of Conce...

Incorrect timing

Handle goatbug Vulnerability details Impact uint256 public constant TIMELOCKDURATION = 4 60 24; // 1 day This is not one day. 4 60 24 != 86400 where 86400 is seconds in a day. Proof of Concept Incorrect timelocks in both the basket and factory can allow parameters to be modified quicker than...

onlyOwner Role Can Unintentionally Influence settleAuction()

Handle leastwood Vulnerability details Impact The onlyOwner role is able to make changes to the protocol with an immediate affect, while other changes made in Basket.sol and Auction.sol incur a one day timelock. As a result, an onlyOwner role may unintentionally frontrun a settleAuction transacti...

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

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

Fee on transfer tokens can lead to incorrect approval

Handle hrkrshnn Vulnerability details Fee on transfer tokens can lead to incorrect approval The createBasket function does not account for tokens with fee on transfer. function createBasketuint256 idNumber external override returns IBasket // ... for uint256 i = 0; i bProposal.weights.length; i++...

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

Unhandled return value of transfer() could cause bounty payment failure

Handle 0xRajeev Vulnerability details Impact ERC20 implementations are not always consistent. Some implementations of transfer and transferFrom could return ‘false’ on failure instead of reverting. It is safer to wrap such calls into require statements or use safe wrapper functions implementing...

Publisher May Accidentally Publish New Index With Incorrect Token Decimals

Handle leastwood Vulnerability details Impact The defiProtocol platform expects all tokens to be 18 decimals, however, nowhere in the contracts is this behaviour enforced. As a result, if a publisher mistakenly publishes a new index with new tokens that don't conform to this standard, settleAucti...

Basket.handleFees() (contracts/Basket.sol#110-129) performs a multiplication on the result of a division

Handle 0xalpharush Vulnerability details Impact Users can burn tokens and evade fees by backrunning other transactions that result in handleFee being called. Proof of Concept Following another transaction that results in lastFee being updated, an attacker can call burn and withdraw their tokens...

Front running basket Creation

Handle goatbug Vulnerability details Impact Anyone can front run and create your basket, circumventing paying any initial fees. Proof of Concept Once proposeBasketLicense is called, anyone can subsequently call createBasket using that ID. Factory contract By calling it first you will avoid paying...

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

Re-entrancy in settleAuction allow stealing all funds

Handle cmichel Vulnerability details Note that the Basket contract approved the Auction contract with all tokens and the settleAuction function allows the auction bonder to transfer all funds out of the basket to themselves. The only limiting factor is the check afterwards that needs to be abided...

No restrictions on owner setting params

Handle goatbug Vulnerability details Impact function setMinLicenseFeeuint256 newMinLicenseFee public override onlyOwner minLicenseFee = newMinLicenseFee; function setAuctionDecrementuint256 newAuctionDecrement public override onlyOwner auctionDecrement = newAuctionDecrement; Proof of Concept Both...

ERC20 return values not checked

Handle cmichel Vulnerability details The ERC20.approve function returns a boolean value indicating success. This parameter needs to be checked for success. The Basket.approveUnderlying function does not check the return value of the IERC20tokensi.approve call. Neither does it work with tokens tha...

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

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

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

Use safeTransfer instead of transfer

Handle hack3r-0m Vulnerability details transfer might return false instead of reverting, in this case, ignoring return value leads to considering it successful. use safeTransfer or check the return value if length of returned data is 0. --- The text was updated successfully, but these errors were...

Withdrawing bounties after checking balances can lead to the entire basket being drained.

Handle itsmeSTYJ Vulnerability details Impact All funds in the basket can be stolen. Proof of Concept 1. Malicious user creates a smart contract that can interact w/ the auction can call bondForRebalance and settleAuction. 2. Auction is started by the publisher 3. Malicious user uses smart contra...

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

Reentrancy in settleAuction(): malicious publisher can bypass index timelock mechanism, inject malicious index, and rug the basket

Handle kenzo Vulnerability details The settleAuction function calls withdrawBounty before setting auctionOngoing = false, thereby allowing reentrancy. Impact A malicious publisher can bypass the index timelock mechanism and publish new index which the basket's users won't have time to respond to...

Bounties payout will get stuck on publishing new index

Handle csanuragjain Vulnerability details Impact User funds will get stuck in case Auction is restarted via publishNewIndex Proof of Concept 1. Publisher publishes new index using publishNewIndex function of Basket.sol function publishNewIndexaddress memory tokens, uint256 memory weights...