Swivel: implementation for initiateZcTokenFillingZcTokenExit is incorrect

Handle itsmeSTYJ Vulnerability details Impact In initiateZcTokenFillingZcTokenExit , this comment // transfer underlying tokens - the premium paid + fee in underlying to swivel from sender is incorrect because you are actually transferring the underlying tokens - premium paid to the maker from...

Erc20.approve() doesn't work on non-standard compliant tokens like USDT

Handle pants Vulnerability details Non-standard compliant tokens like USDT don't return a return value on approve. The function Erc20.approve reverts if the token doesn't return a return value, as it is defined to always return a boolean. Impact When using any non-standard compliant token like...

Incentives for different pools should differ by a large factor

Handle tensors Vulnerability details I'm adding this as an issue because I didn't see it mentioned anywhere in the codebase, and I think its a fair point that relates to how the protocol gives out rewards to users. As I understand , the point of staking is to provide users with additional...

User can get more/less interest than required

Handle csanuragjain Vulnerability details Impact It seems that owner must not get interest for transfer fee which is collected using transferNotionalFee No interest is calculated on owner side before reducing the owner notional amount But this can simply be bypassed if owner adds new notional whi...

Incorrect implementation of chainlink oracle

Handle tensors Vulnerability details The protocol doesn't implement the chainlink ETH oracle correctly. Many user functions in LendingPair.sol use currentTokenValues which computes data based off of the chainlink eth oracle via tokenPrice which uses EthPrice. In a correct implementation 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...

Incorrect multiplication in _computeSingleOutGivenPoolIn of IndexPool

Handle broccoli Vulnerability details Impact The computeSingleOutGivenPoolIn function of IndexPool uses the raw multiplication i.e., to calculate the zaz variable. However, since both BASE - normalizedWeight and swapFee are in WAD, the mul function should be used instead to calculate the correct...

Revert the transaction if convergence does not happen in HybridPool

Handle broccoli Vulnerability details Title: Revert the transaction if convergence does not happen in HybridPool Impact The getY and computeLiquidityFromAdjustedBalances functions use iterative approximations to calculate the output token amount or the liquidity invariant D. However, if the...

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

Supply part of the accrued debt can be stolen

Handle cmichel Vulnerability details The LendingPair.uniClaimDeposit function allows the user to "collect fees" and mint new supply shares with the collected amounts. uniClaimDeposit does not accrue tokens However, the current total supply is not accrued in the function. This means an attacker ca...

Router's complexPath percentagePaths don't work as expected

Handle cmichel Vulnerability details The TridentRouter.complexPath function allows splitting a trade result into several buckets and trade them in a different pool each. The distribution is defined by the params.percentagePathi.balancePercentage values: for uint256 i; i...

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

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

Funds in the pool could be stolen by exploiting flashSwap in HybridPool

Handle broccoli Vulnerability details Impact An attacker can call the bento.harvest function during the callback function of a flash swap of the HybridPool to reduce the number of input tokens that he has to pay to the pool, as long as there is any unrealized profit in the strategy contract of th...

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

IndexPool.mint() Unchecked arithmetic can overflow that allows stealing of almost all the funds in the pool

Handle WatchPug Vulnerability details /// @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 using the return value. function mintbytes calldata data public override lock returns uint256...

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

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

Approximations may finish with inaccurate values

Handle 0xsanson Vulnerability details Impact In HybridPool.sol, functions computeLiquidityFromAdjustedBalances, getY and getYD may finish before approximation converge, since it's limited by MAXLOOPLIMIT iterations. In this situation the final estimated value will still be treated as correct, eve...

Use of tokenB’s price instead of tokenA in determining account health will lead to protocol mis-accounting and insolvency

Handle 0xRajeev Vulnerability details Impact In supplyCreditUni, the last argument of convertTokenValues on L674 being priceB instead of priceA in the calculation of supplyB is a typo should be priceA and therefore miscalculates supplyB, creditB, creditUni and therefore totalAccountSupply in...

Reentrancy in withdraw. Reentrancy guard is missing,

Handle pants Vulnerability details Reentrancy problem in withdraw for any token with callback in transfer. Thereare multiple standards that allow that and therefore allows reentrancy attacks on your contract. line 115 --- The text was updated successfully, but these errors were encountered: All...

_pow is mathematically wrong

Handle 0xsanson Vulnerability details Impact In IndexPool.sol, the function pow is called during the computation of the output amount when swapping. 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 !...

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

IndexPool.INIT_POOL_SUPPLY is too large, which may cause a significant amount of fund loss 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...

Incorrect implementation of difference in MathUtils

Handle broccoli Vulnerability details Impact The difference function of MathUtils is incorrect. Without a return statement in the if bracket, the function always returns diff = b - a, causing differencex + 1, x to be uint-1, and thus withinx + 1, x is false. The within function is used to in the...

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

Liquidation can be escaped by depositing a Uni v3 position with 0 liquidity

Handle WatchPug Vulnerability details When the liquidator is trying to liquidate a undercolldarezed loan by calling liquidateAccount, it calls unwrapUniPosition - uniV3Helper.removeLiquidity - positionManager.decreaseLiquidity. However, when the Uni v3 position has 0 liquidity,...

TridentRouter.addLiquidity() Add liquidity to IndexPool through TridentRouter may casue loss of a small portion of users funds

Handle WatchPug Vulnerability details The mint function in IndexPool requires the liquidity provider to transfer in amounts no less than the amounts of tokens' reserve proportionally to the toMint amount. However, the TridentRouter won't calculate the toMint amount and amountsIn for the liquidity...

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

Overflow in the mint function of IndexPool causes LPs' funds to be stolen

Handle broccoli Vulnerability details Impact It is possible to overflow the addition in the balance check i.e., balancetokenIn = amountIn + reserve in the mint function by setting the amountIn to a large amount. As a result, the attacker could gain a large number of LP tokens by not even providin...

ChainLink price data could be stale

Handle cmichel Vulnerability details There is no check in UniswapV3Oracle.ethPrice 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." under...

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

Users are susceptible to back-running when depositing ETH to TridenRouter

Handle broccoli Vulnerability details Impact The depositToBentoBox and depositFromUserToBentoBox allow users to provide ETH to the router, which is later deposited to the bento contract for swapping other assets or providing liquidity. However, in these two functions, the input parameter does not...

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._updateReserves Wrong implementation

Handle WatchPug Vulnerability details HybridPool.updateReserves is supposed to update the reserves to the latest bento share amounts. Instead, it uses underlying token amounts. In the getReserves function, reserves are treated as bento share amounts and get converted to underlying token amounts...

hybrid pool uses wrong non_optimal_mint_fee

Handle broccoli Vulnerability details Impact When an lp provider deposits an imbalance amount of token, a swap fee is applied. HybridPool uses the same nonOptimalMintFee as constantProductPool; however, since two pools use different AMM curve, the ideal balance is not the same. ref:...

ConstantProductPool.burnSingle swap amount computations should use balance

Handle cmichel Vulnerability details The ConstantProductPool.burnSingle function is basically a burn followed by a swap and must therefore act the same way as calling these two functions sequentially. The token amounts to redeem amount0, amount1 are computed on the balance not the reserve. Howeve...

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

HybridPool._updateReserves Wrong implementation

Handle WatchPug Vulnerability details HybridPool.updateReserves is supposed to update the reserves to the latest bento share amounts. Instead, it uses underlying token amounts. In the getReserves function, reserves are treated as bento share amounts and get converted to underlying token amounts...

IndexPool.mint() Unchecked arithmetic can overflow that allows stealing of almost all the funds in the pool

Handle WatchPug Vulnerability details /// @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 using the return value. function mintbytes calldata data public override lock returns uint256...

Use of deprecated Chainlink API

Handle 0xRajeev Vulnerability details Impact The 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 contracts have to...

IndexPool initial LP supply computation is wrong

Handle cmichel Vulnerability details The IndexPool.constructor function already mints INITPOOLSUPPLY = 100 1e18 = 1e20 LP tokens to the zero address. When trying to use the pool, someone has to provide the actual initial reserve tokens in mint. On the first mint, the pool reserves are zero and th...

Miscalculation of _supplyCreditUni allows undercollateralized loan

Handle WatchPug Vulnerability details In supplyCreditUni, the calculation of the collateral value of tokenB supply is using priceB instead of priceA, which can lead to undercollateralized loans. function supplyCreditUni address account, address returnToken, uint priceA, uint priceB, uint...

Transfer method doesn't consider gained interest correctly,

Handle pants Vulnerability details Impact transfer method doesn't consider gained interest correctly. For example, a user that gained 10% of interest and moves the LP tokens to another user might lose the gained interest. This is a severe bug and all other LP tokens out there manages the interest...

Router would fail when adding liquidity to index Pool

Handle broccoli Vulnerability details Impact TridentRouter is easy to fail when trying to provide liquidity to an index pool. Users would not get extra lp if they are not providing lp at the pool's spot price. It's the same design as uniswap v2. However, uniswap's v2 handle's the dirty part...

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

Rounding errors will occur for tokens without decimals

Handle tensors Vulnerability details Some rare tokens have 0 decimals: For these tokens, small losses of precision will be amplified by the lack of decimals. Consider a constant product pool with 1000 of token0 with no decimals, and 1000 of token1 also with no decimals. Suppose I swap n= 1,2,3,4 ...

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

ConstantProductPool & HybridPool: Adding and removing unbalanced liquidity yields slightly more tokens than swap

Handle GreyArt Vulnerability details Impact A mint fee is applied whenever unbalanced liquidity is added, because it is akin to swapping the excess token amount for the other token. However, the current implementation distributes the minted fee to the minter as well when he should be excluded. It...

LendingPair.withdrawUniPosition should accrue debt first

Handle cmichel Vulnerability details The LendingPair.withdrawUniPosition function allows the user to withdraw their UniswapV3 pool position NFT again. As the Uniswap position acts as collateral in the protocol, a health check is performed afterwards. However, it does not check the current debt of...