Summarize your findings for the bug or vulnerability

Handle AC0611 Vulnerability details Impact Detailed description of the impact of this finding. Proof of Concept Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept. Tools Used Recommended Mitigation Steps --- The...

Deposits don't work with fee-on transfer tokens

Handle cmichel Vulnerability details There are ERC20 tokens that may make certain customizations to their ERC20 contracts. One type of these tokens is deflationary tokens that charge a certain fee for every transfer or transferFrom. Others are rebasing tokens that increase in value over time like...

PoolCommitter.sol#commit() Calculation of amountOut is wrong

Handle WatchPug Vulnerability details else if commitType == CommitType.LongBurn // long burning: pull in long pool tokens from committer // A theoretical amount based on current ratio. Used to get same units as minimumCommitSize uint256 amountOut = PoolSwapLibrary.getWithdrawAmountOnBurn...

Deposits don't work with fee-on transfer tokens

Handle cmichel Vulnerability details There are ERC20 tokens that may make certain customizations to their ERC20 contracts. One type of these tokens is deflationary tokens that charge a certain fee for every transfer or transferFrom. Others are rebasing tokens that increase in value over time like...

uncommit sends tokens to the wrong user

Handle cmichel Vulnerability details The PoolCommitter.uncommit function calls the ILeveragedPoolleveragedPool.quoteTokenTransfer/mintTokens function with msg.sender. But in uncommit's case that's the pool, not the commit owner, see onlyPool modifier on executeAllCommitments which calls uncommit...

commit burn yields wrong amountOut computation

Handle cmichel Vulnerability details The PoolCommitter.commit function first adds the amount to the shadow pool shadowPoolscommitType = shadowPoolscommitType + amount and then computes the amountOut with this updated value already: PoolSwapLibrary.getWithdrawAmountOnBurn IERC20tokens1.totalSupply...

The formula of number of prizes for a degree is wrong

Handle WatchPug Vulnerability details The formula of the number of prizes for a degree per the document: is: Number of prizes for a degree = 2^bit range^degree - 2^bit range^degree-1 - 2^bit range^degree-2 - ... Should be changed to: Number of prizes for a degree = 2^bit range^degree - 2^bit...

Miners Can Re-Roll the VRF Output to Game the Protocol

Handle leastwood Vulnerability details Impact Miners are able to rewrite a chain's history if they dislike the VRF output used by the protocol. Consider the following example: A miner or well-funded user is participating in the PoolTogether protocol. A VRF request is made and fulfilled in the sam...

Wrong keeper reward computation

Handle cmichel Vulnerability details The PoolKeeper.keeperReward computation mixes WADs and Quads which leads to issues. 1. Note that keeperTip returns values where 1 = 1%, and 100 = 100%, the same way BASETIP = 5 = 5%. Thus tipPercent = ABDKMathQuad.fromUIntkeeperTip is a Quad value of this keep...

Number of prizes math is wrong

Handle cmichel Vulnerability details The math described in Splitting the prizes and implemented in DrawCalculator.numberOfPrizesForIndex seems to be wrong. Assuming a bit range of 4 16 possibilities per position and cardinality of 8. Note that degree is determined by the first position where it...

PoolCommitter.sol#._uncommit() Tokens are not returned to the commit owner

Handle WatchPug Vulnerability details if commit.commitType == CommitType.LongMint || commit.commitType == CommitType.ShortMint // minting: return quote tokens to the commit owner ILeveragedPoolleveragedPool.quoteTokenTransfermsg.sender, commit.amount; else if commit.commitType ==...

Basket becomes unusable if everybody burns their shares

Handle kenzo Vulnerability details While handling the fees, the contract calculates the new ibRatio by dividing by totalSupply. This can be 0 leading to a division by 0. Impact If everybody burns their shares, in the next mint, totalSupply will be 0, handleFees will revert, and so nobody will be...

mint will always revert

Handle pauliax Vulnerability details Impact Function mint will always fail as it calls mintTo and both functions are nonReentrant. Recommended Mitigation Steps Remove nonReentrant modifier from the mint function as it will be applied in mintTo anyway. --- The text was updated successfully, but...

Auction bonder can steal user funds if bond block is high enough

Handle kenzo Vulnerability details After an auction has started, as time passes and according to the bondBlock, newRatio which starts at 2ibRatio gets smaller and smaller and therefore less and less tokens need to remain in the basket. This is not capped, and after a while, newRatio can become...

Unnecessary nonReentrant at mint breaks protocol

Handle kenzo Vulnerability details Basket's mint function has nonReentrant modifier. Mint function is only calling mintTo which also has nonReentrant modifier. Impact Nobody can use mint function. Proof of Concept Tools Used Recommended Mitigation Steps Remove nonReentrant from mint. --- The text...

Bonding mechanism allows malicious user to DOS auctions

Handle kenzo Vulnerability details A malicious user can listen to the mempool and immediately bond when an auction starts, without aim of settling the auction. As no one can cancel his bond in less than 24h, this will freeze user funds and auction settlement for 24h until his bond is burned and t...

Validations

Handle pauliax Vulnerability details Impact function setBondPercentDiv should validate that newBondPercentDiv is not 0, or bondForRebalance will experience division by zero error otherwise. If you want to allow 0 values, then bondForRebalance should accommodate for such a possibility. function...

Basket.sol#mint() Malfunction due to extra nonReentrant modifier

Handle WatchPug Vulnerability details function mintuint256 amount public nonReentrant override mintToamount, msg.sender; function mintTouint256 amount, address to public nonReentrant override requireauction.auctionOngoing == false; The mint method is malfunction because of the extra nonReentrant...

Fee on transfer tokens do not work within the protocol

Handle tensors Vulnerability details Fee on transfer tokens transfer less tokens in than what would be expect. This means that the protocol request incorrect amounts when dealing with these tokens. The protocol should use stored token balances instead of transfer for calculating amounts. --- The...

If newRatio > ibRatio after a settlement the protocol could lose its funds.

Handle tensors Vulnerability details Suppose that after a certain settleAuction call we have that newRatio ibRatio. I don't see any reason why this couldn't be possible, going through the math and solving for this condition we can see that: if b ibRatio then newRatio ibRatio assuming...

createBasket re-entrancy

Handle pauliax Vulnerability details Impact function createBasket in Factory should also be nonReentrant as it interacts with various tokens inside the loop and these tokens may contain callback hooks. Recommended Mitigation Steps Add nonReentrant modifier to the declaration of createBasket. ---...

Setting Factory.auctionDecrement to zero causes Denial of Service in Auction.settleAuction()

Handle pants Vulnerability details The function Factory.setAuctionDecrement allows the owner to set the state variable Factory.auctionDecrement to zero. Impact If Factory.auctionDecrement equals zero then the function Auction.settleAuction will always revert due to a division by zero: uint256 b =...

Denial of Service in Basket.mint()

Handle pants Vulnerability details The function Basket.mint calls Basket.mintTo, but both Basket.mint and Basket.mintTo use the nonReentrant modifier. Impact The function Basket.mint will always revert when it calls Basket.mintTo, due to reentrancy. Tool Used Manual code review. Recommended...

Setting Factory.bondPercentDiv to zero cause Denial of Service in Auction.bondForRebalance()

Handle pants Vulnerability details The function Factory.setBondPercentDiv allows the owner to set the state variable Factory.bondPercentDiv to zero. Impact If Factory.bondPercentDiv equals zero then the function Auction.bondForRebalance will always revert due to a division by zero: bondAmount =...

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

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

fee-on-transfer underlying can cause problems

Handle 0xsanson Vulnerability details Impact The current implementation doesn't work with fee-on-transfer underlying tokens. Considering that Compound can have these kind of tokens ex. USDT can activate fees, this issue can affect the protocol. The problem arise when transferring tokens, basicall...

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

ConcentratedLiquidityPoolManager.sol#claimReward() and reclaimIncentive() will fail when incentive.token is token0 or token1

Handle WatchPug Vulnerability details In ConcentratedLiquidityPosition.collect, balances of token0 and token1 in bento will be used to pay the fees. uint256 balance0 = bento.balanceOftoken0, addressthis; uint256 balance1 = bento.balanceOftoken1, addressthis; if balance0 newBalance0 token0amount =...

Can reclaim the same incentive multiple times

Handle pauliax Vulnerability details Impact function reclaimIncentive can be invoked more than once for the same incentiveId. If there were multiple incentives for the same token it would allow to drain these tokens by repeatedly calling reclaimIncentive. Recommended Mitigation Steps Simple...

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

DDOS by cancelling orders or removing approvals

Handle cmichel Vulnerability details The market assumes that the maker of an order approved the swivel contract as there are many calls that move funds from the maker to the contract, see Swivel.initiateVaultFillingZcTokenInitiate/initiateZcTokenFillingVaultInitiate: uToken.transferFromo.maker,...

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

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

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

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

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

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

unchecked return value from approve()

Handle JMukesh Vulnerability details Impact contract uses an ERC20.approve call but does not check the success return value. Some tokens do not revert if the approval failed, returning false instead. The impact is that, tokens that don’t actually perform the approve and return false are still...

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

## ConcentratedLiquidityPoolManager.sol Wrong implementation of claimReward() and getReward()

Handle WatchPug Vulnerability details ConcentratedLiquidityPoolManager.sol Wrong implementation of claimReward and getReward Given incentive.secondsClaimed can usually be larger than 128, both claimReward and getReward will revert at: uint256 secondsUnclaimed = maxTime - incentive.startTime --- T...

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

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

Wrong usage of positionId in ConcentratedLiquidityPoolManager

Handle broccoli Vulnerability details Impact In the subscribe function of ConcentratedLiquidityPoolManager, the incentive to subscribed is determined as follows: Incentive memory incentive = incentivespoolpositionId; However, positionId should be incentiveId, a counter that increases by one...

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

Two-step change of a swivel address

Handle pauliax Vulnerability details Impact MarketPlace allows an admin to change swivel to a different address. This function has no validations, even a simple check for zero-address is missing, and there is no validation of the new address being correct. If the admin accidentally uses an invali...

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

fee possibly send in the wrong way in exitVaultFillingVaultInitiate

Handle gpersoon Vulnerability details Impact The functions exitZcTokenFillingZcTokenInitiate and exitVaultFillingVaultInitiate are kind of similar, however the processing of the fee is different. exitZcTokenFillingZcTokenInitiate send the fee from o.maker exitVaultFillingVaultInitiate sends the f...

Unhandled return value of ERC20 transfer/transferFrom

Handle 0xRajeev Vulnerability details Impact For reference, see similar Medium-severity finding from Consensys Diligence Audit of Aave Protocol V2: As stated in the above finding: “ERC20 implementations are not always consistent. Some implementations of transfer and transferFrom could return...