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

Erc20.approve() return value is ignored

Handle pants Vulnerability details According to the ERC-20 Token Standard, the function approve returns false on failure. However, the return value of Erc20.approve is ignored 3 times: 1. In Swivel.initiateVaultFillingZcTokenInitiate, line 109. 2. In Swivel.initiateZcTokenFillingVaultInitiate, li...

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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