Creating the same market shouldn't be possible

Handle 0xsanson Vulnerability details Impact Only an admin can create a market by calling MarketPlace.createMarket. With the current implementation, it's possible to create another market with the same underlying u and maturity m. Doing so would rewrite marketsum with a new ZcToken and...

functions permit and permitAll can be tricked by an invalid signature

Handle pauliax Vulnerability details Impact When the signature is not valid, ecrecover returns empty 0x0 address. There is a potential check against that: require recoveredAddress != address0 && recoveredAddress == owner || isApprovedForAllownerrecoveredAddress, "INVALIDPERMITSIGNATURE" ; However...

return value of 0 from ecrecover not checked

Handle gpersoon Vulnerability details Impact The solidity function ecrecover is used, however the error result of 0 is not checked for. See documentation: "recover the address associated with the public key from elliptic curve signature or return zero on error. " Now you can supply invalid input...

Incorrect usage of typecasting in burn lets an attacker corrupt the pool state

Handle broccoli Vulnerability details Impact In the burn function of ConcentratedLiquidityPool, when calling updatePosition, the amount of liquidity to burn is explicitly converted from uint128 to int128, which could result in a positive integer if amount is larger than 1 127 and less than 1 128...

Admin is a single-point of failure without any mitigations

Handle 0xRajeev Vulnerability details Impact Admin role has absolute power across Swivel, Marketplace and VaultTracker contracts with several onlyOwner functions. There is no ability to change admin to a new address or renounce it which is helpful for lost/compromised admin keys or to delegate...

Users cannot receive rewards from ConcentratedLiquidityPoolManager if their liquidity is too large

Handle broccoli Vulnerability details Impact There could be an integer underflow error when the reward of an incentive is claimed, forcing users to wait for a sufficient period or reduce their liquidity to claim the rewards. Proof of Concept The unclaimed reward that a user could claim is...

Override existing market

Handle pauliax Vulnerability details Impact When calling function createMarket an admin can override an existing market by specifying the same underlying and maturity: marketsum = Marketc, zctAddr, vAddr; it does not check if the market for these parameters already exists, so technically it is...

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

Market can be overwritten

Handle cmichel Vulnerability details The MarketPlace.createMarket function does not check if a market already exists for the underlying, maturity pair. Impact Existing markets can be overwritten and the old zt and n tokens will lose their association to the market. Recommended Mitigation Steps Ad...

ConcentratedLiquidityPoolHelper: getTickState() might run out of gas

Handle hickuphh3 Vulnerability details Impact getTickState attempts to fetch the state of all inserted ticks including MINTICK and MAXTICK of a pool. Depending on the tick spacing, this function may run out of gas. Recommended Mitigation Steps Have a starting index parameter to start the iteratio...

Unsafe handling of underlying tokens

Handle 0xsanson Vulnerability details Impact Not every ERC20 token follows OpenZeppelin's recommendation. It's possible inside ERC20 standard that a transferFrom doesn't revert upon failure but returns false. The code doesn't check these return values. For example uToken.transferFrommsg.sender,...

Safe transfers of tokens

Handle pauliax Vulnerability details Impact Not every ERC20 token returns true on transfer success. To support different tokens, the current best practice is to use SafeERC20 safeTransfer, safeTransferFrom, etc: Similarly, some ERC20 tokens like USDT require resetting the approval to 0 first befo...

unchecked return value from Transfer/TransferFrom()

Handle JMukesh Vulnerability details Impact The ERC20.transfer and ERC20.transferFrom functions return a boolean value indicating success. This parameter needs to be checked for success. Some tokens do not revert if the transfer failed but return false instead. Proof of Concept Tools Used manual...

Missing event & timelock for critical onlyAdmin functions

Handle 0xRajeev Vulnerability details Impact onlyAdmin functions that change critical contract parameters/addresses/state should emit events and consider adding timelocks so that users and other privileged roles can detect upcoming changes by offchain monitoring of events and have the time to rea...

Wrong reserve decrease in burn

Handle 0xsanson Vulnerability details Impact When burning a liquidity position the reserves should be decreased by the tokens' amount that leaves the contract. However in ConcentratedLiquidityPool's burn they are decreased only by the fees. Proof of Concept Tools Used editor Recommended Mitigatio...

ConcentratedLiquidityPoolManager: incentive is incorrectly accessed

Handle hickuphh3 Vulnerability details Impact The positionId is used to retrieve the incentive info instead of incentiveId. Incentive storage / memory incentive = incentivesposition.poolpositionId; Recommended Mitigation Steps Incentive storage / memory incentive =...

ConcentratedLiquidityPool: secondsPerLiquidity should be modified whenever pool liquidity changes

Handle hickuphh3 Vulnerability details Impact secondsPerLiquidity is updated as such: secondsPerLiquidity += uint160diff 128 / liquidity; where diff = timestamp - uint256lastObservation. Hence, whenever liquidity changes, secondsPerLiquidity should be updated prior to the change. In particular,...

Wrong index when accessing incentives

Handle pauliax Vulnerability details Impact Should be incentiveId, not positionId here: Incentive memory incentive = incentivespoolpositionId; Recommended Mitigation Steps Incentive memory incentive = incentivespoolincentiveId; --- The text was updated successfully, but these errors were...

ConcentratedLiquidityPosition.sol#burn() Wrong implementation allows attackers to steal yield

Handle WatchPug Vulnerability details When a user calls ConcentratedLiquidityPosition.solburn to burn their liquidity, it calls ConcentratedLiquidityPool.solburn - updatePosition: The updatePosition function will return amount0fees and amount1fees of the whole position with the lower and upper ti...

Infinite mint by transferring nTokens to self

Handle cmichel Vulnerability details The VaultTracker.transferNotionalFrom function first stores the vaults data for f and t in a memory variable not using storage pointers. An attacker can transfer tokens to themself using f = t and mint free tokens for themself. The vaultsf storage is set first...

ConcentratedLiquidityPosition.sol#collect() Users may get double the amount of yield when they call collect() before burn()

Handle WatchPug Vulnerability details When a user calls ConcentratedLiquidityPosition.solcollect to collect their yield, it calcuates the yield based on position.pool.rangeFeeGrowth and position.feeGrowthInside0, position.feeGrowthInside1: When there are enough tokens in bento.balanceOf, it will...

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

Handle broccoli Vulnerability details Impact Similar to a previous finding in the IndexPool contract, the mint function of ConcentratedLiquidityPool allows integer overflows when checking the balance is larger or equal to the received amount of token plus the reserve. As a result, the attacker...

Incentive should check that it hasn't started yet

Handle cmichel Vulnerability details The ConcentratedLiquidityPoolManager.addIncentive function can add an incentive that already has a non-zero incentive.secondsClaimed. Impact Rewards will be wrong. Recommended Mitigation Steps Add a check: requireincentive.secondsClaimed == 0, "!secondsClaimed...

Burning does not update reserves

Handle cmichel Vulnerability details The ConcentratedLiquidityPool.burn function sends out amount0/amount1 tokens but only updates the reserves by decreasing it by the fees of these amounts. unchecked // @audit decreases by fees only, not by amount0/amount1 reserve0 -= uint128amount0fees; reserve...

ConcentratedLiquidityPool: incorrect feeGrowthGlobal accounting when crossing ticks

Handle hickuphh3 Vulnerability details Impact Swap fees are taken from the output. Hence, if swapping token0 for token1 zeroForOne is true, then fees are taken in token1. We see this to be the case in the initialization of feeGrowthGlobal in the swap cache feeGrowthGlobal = zeroForOne ?...

ERC20 return values not checked

Handle cmichel Vulnerability details The ERC20.transfer and ERC20.transferFrom functions return a boolean value indicating success. This parameter needs to be checked for success. Some tokens do not revert if the transfer failed but return false instead. This value is not checked throughout the...

TridentNFT.permit should always check recoveredAddress != 0

Handle cmichel Vulnerability details The TridentNFT.permit function ignores the recoveredAddress != 0 check if isApprovedForAllownerrecoveredAddress is true. Impact If a user accidentally set the zero address as the operator, tokens can be stolen by anyone as a wrong signature yield...

Missing zero address check for ecrecover will validate invalid signatures

Handle 0xRajeev Vulnerability details Impact Solidity’s ecrecover returns 0 if signature is invalid. The Sig.sol library does not perform zero address check on ecrecover’s return value and returns it as-is. The validOrderHash function in Swivel which uses Sig.recover compares its return value...

Missing events/timelocks for owner/admin only functions that change critical parameters

Handle defsec Vulnerability details Impact Owner/admin only functions that change critical parameters should emit events and have timelocks. Events allow capturing the changed parameters so that off-chain tools/interfaces can register such changes with timelocks that allow users to evaluate them...

Manipulation of secondsClaimed

Handle pauliax Vulnerability details Impact function addIncentive does not verify secondsClaimed so any arbitrary value can be set initially by the creator and it may break calculations. Recommended Mitigation Steps Consider either requiring that incentive.secondsClaimed is 0 or manually resettin...

Swivel.sol transfer of tokens in initiateZcTokenFillingZcTokenExit and initiateVaultFillingVaultExit can fail without reverting causing account problems

Handle GalloDaSballo Vulnerability details Impact initiateZcTokenFillingZcTokenExit and initiateVaultFillingVaultExit in Swivel.sol are using transferFrom This function call can fail meaning the tokens are not transferred without causing a revert This would break the accounting in the protocol I...

Signature check passes for maker=0 orders

Handle cmichel Vulnerability details It's possible to choose the zero address for the maker of an order. Then any signature will pass the validOrderHash check. Impact This doesn't lead to any serious issues as the zero address does not have any approvals set of its own. The transactions will fail...

Wrong indexes: positionId vs incentiveId

Handle 0xsanson Vulnerability details Impact In multiple functions in ConcentratedLiquidityPoolManager, the index positionId is used instead of the correct incentiveId when dealing with the incentives mapping. Of course the issue is that incentives cannot be used, or in some cases only by lucky...

uToken ERC20 approve method missing return value check #L109

Handle defsec Vulnerability details Impact The initiateVaultFillingZcTokenInitiate function performs an ERC20.approve call but does not check the success return value. Some tokens do not revert if the approval failed but return false instead. Proof of Concept 1. Navigate to "" 2...

Integer underflow error in claimReward of ConcentratedLiquidityPoolManager

Handle broccoli Vulnerability details Impact The claimReward function of ConcentratedLiquidityPoolManager calculates the secondsUnclaimed variable using a formula with an unclear intention: uint256 secondsUnclaimed = maxTime - incentive.startTime 128 - incentive.secondsClaimed; This formula cause...

ConcentratedLiquidityPoolManager's incentives can be stolen

Handle cmichel Vulnerability details The ConcentratedLiquidityPoolManager keeps all tokens for all incentives in the same contract. The reclaimIncentive function does not reduce the incentive.rewardsUnclaimed field and thus one can reclaim tokens several times. This allows anyone to steal all...

Wrong subtraction from reserve in burn

Handle pauliax Vulnerability details Impact function burn should subtract amount0 and amount1, not only fees from reserve0 and reserve1 here as whole amounts are withdrawn: reserve0 -= uint128amount0fees; reserve1 -= uint128amount1fees; Recommended Mitigation Steps reserve0 -= uint128amount0;...

Unsafe cast in ConcentratedLiquidityPool burn leads to attack

Handle cmichel Vulnerability details The ConcentratedLiquidityPool.burn function performs an unsafe cast of a uint128 type to a signed integer. uint256 amount0fees, uint256 amount1fees = updatePositionmsg.sender, lower, upper, -int128amount; Note that amount is chosen by the caller and when...

uToken's approve() Function is Susceptible to Frontrunning Attacks

Handle leastwood Vulnerability details Impact Order makers will call the approve function to facilitate transactions between order takers and makers. If a taker sees that a maker is seeking to reduce its order exposure by reducing Swivel.sol's allowance, a taker can frontrun this transaction to...

ConcentratedLiquidityPoolManager: incorrect calculation of secondsUnclaimed

Handle hickuphh3 Vulnerability details Impact The subtraction of secondsClaimed should be performed after the left shifting of bits in uint256 secondsUnclaimed = maxTime - incentive.startTime 128 - incentive.secondsClaimed; Recommended Mitigation Steps uint256 secondsUnclaimed = maxTime -...

Prevent creating the same market twice

Handle gpersoon Vulnerability details Impact The function createMarket of MarketPlace.sol doesn't check if the market already exists. So it could accidentally deploy a market with has the same maturity timestamp twice and overwrite the previous values of the market. The previously deployed market...

onlyAdmin Role May Unintentionally Cause Issues in the Underlying Protocol

Handle leastwood Vulnerability details Impact The onlyAdmin role points to an EOA account managed by the Swivel team. Withdrawals from the Swivel protocol have to be scheduled by the onlyAdmin role, ensuring that users have sufficient time to withdraw, if required. The setFee function in Swivel.s...

lack of access modifier in mintReturn()

Handle JMukesh Vulnerability details Impact Due to lack of access modifier in mintReturn , any malicious actor can set its value to false due which function which will call mintaddress f, uint256 a will always fail which include: custodialInitiate mintZcTokenAddingNotional Proof of Concept Tools...

Erc20.transferFrom() 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 transferFrom. The function Erc20.transferFrom 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 tok...

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

Erc20.transfer() 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 transfer. The function Erc20.transfer 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...

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.transfer() return value is ignored

Handle pants Vulnerability details According to the ERC-20 Token Standard, the function transfer returns false on failure. However, the return value of Erc20.transfer is ignored 8 times: 1. In Swivel.exitVaultFillingZcTokenExit, line 313. 2. In Swivel.exitVaultFillingZcTokenExit, line 315. 3. In...

Swivel: Taker is charged fees twice in exitVaultFillingVaultInitiate

Handle itsmeSTYJ Vulnerability details Impact Taker is charged fees twice in exitVaultFillingVaultInitiate . Maker is transferring less than premiumFilled to taker and then taker is expected to pay fees i.e. taker's net balance is premiumFilled - 2fee Recommended Mitigation Steps function...

Erc20.transferFrom() return value is ignored

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