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

IndexPool.mint The first liquidity provider is forced to supply assets in the same amount, which may cause a significant amount of fund loss

Handle WatchPug Vulnerability details When reserve == 0, amountIn for all the tokens will be set to the same amount: ratio, regardless of the weights, decimals and market prices of the assets. The first liquidity provider may not be aware of this so that it may create an arbitrage opportunity for...

IndexPool._pow wrong loop and does not normalize values

Handle cmichel Vulnerability details The IndexPool.compute function is indented as if the if n % 2 != 0 output = output a; is inside the loop but there are actually not braces around it. It must be in the loop for the exponentiation by repeated squaring algorithm to work: function powuint256 a,...

HybridPool's flashSwap gives the total fee to barFeeTo

Handle 0xsanson Vulnerability details Impact In HybridPool's flashSwap function there's a transfer to barFeeTo transfertokenIn, fee, barFeeTo, false; Here fee = amountIn swapFee / MAXFEE is the total swap fee. However it should transfer out only a fraction of it barFee/MAXFEE otherwise liquidity...

HybridPool's reserve is converted to "amount" twice

Handle cmichel Vulnerability details The HybridPool's reserves are stored as Bento "amounts" not Bento shares in updateReserves because balance converts the current share balance to amount balances. However, when retrieving the reserve0/1 storage fields in getReserves, they are converted to amoun...

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

Wrong implementation of _sqrt function

Handle pants Vulnerability details Use a library for that. Don't implement yourself. line 187. --- The text was updated successfully, but these errors were encountered: All reactions...

withdrawFromWETH always reverts

Handle cmichel Vulnerability details The TridentHelper.withdrawFromWETH used in TridentRouter.unwrapWETH function performs a low-level call to WETH.withdrawamount. It then checks if the return data length is more or equal to 32 bytes, however WETH.withdraw returns void and has a return value of 0...

Incorrect usage of _pow in _computeSingleOutGivenPoolIn of IndexPool

Handle broccoli Vulnerability details Impact The computeSingleOutGivenPoolIn function of IndexPool uses the pow function to calculate tokenOutRatio with the exponent in WAD i.e., in 18 decimals of precision. However, the pow function assumes that the given exponent n is not in WAD. for example,...

Adding imbalanced liquidity earns extra rewards

Handle broccoli Vulnerability details Adding imbalanced liquidity earns extra rewards Impact When a user provides liquidity with unbalanced balance. It should be the same as swapping tokens and adding lp. However, the liquidity the users get is calculated as follow: uint256 computed =...

Wrong balance to reserve casting in _updateReserves()

Handle 0xsanson Vulnerability details Impact In HybridPool's updateReserves function, the reserves are calculated as: uint256 reserve0, uint256 reserve1 = balance; This is incorrect, since reserves are a bento-share quantity, while balance outputs a bento-amount quantity. This basically inflates...

IndexPool: Poor conversion from Balancer V1's corresponding functions

Handle GreyArt Vulnerability details Impact A number of functions suffer from the erroneous conversion of Balancer V1's implementation. compute equivalent to Balancer's bpow if remain == 0 output = wholePow; when a return statement should be used instead. computeSingleOutGivenPoolIn equivalent to...

IndexPool's flashSwap does not transfer tokens before the callback

Handle cmichel Vulnerability details The IndexPool.flashSwap function calls ITridentCalleemsg.sender.tridentSwapCallbackcontext before transferring the tokens to the recipient via tranfer. Impact It's very important that the tokens are transferred to the caller before the callback. The use-case o...

IndexPool.sol#_pow() Wrong implementation

Handle WatchPug Vulnerability details function powuint256 a, uint256 n internal pure returns uint256 output output = n % 2 != 0 ? a : BASE; for n /= 2; n != 0; n /= 2 a = a a; if n % 2 != 0 output = output a; 1. a a without div by BASE will accumulate decimals unexpectedly and leads to overflow...

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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