TwabRewards: fee on transfer token as promotion token can make claimRewards() fail

Handle GiveMeTestEther Vulnerability details Impact If the rewards creator sets a token that applies transfer fees for the promotion token, users won't be able to claim some of their rewards because the claimRewards uses calculateRewardAmount that assumes for each epoch there are...

If a promoter cancels a promotion, unclaimed rewards of ticket holders are lost

Handle hubble Vulnerability details Impact Loss of rewards for the ticket holders who have not yet claimed the rewards accrued until the previous epocs, if the promoter cancels the promotion after any epoch. Proof of Concept contract : TwabRewards function : cancelPromotion line 132 delete...

Rewards not worth to claim will accumulate in the contract

Handle 0x0x0x Vulnerability details Let's say there is a Ticket called A. Ticket A has many users and assume there are some whales and they control most of the supplies. In this case most users rewards can be less than gas cost to claim rewards. Therefore, users with rewards less than gas cost...

TwabRewards: fee on transfer token as promotion token will block at least one epoch reward claim

Handle GiveMeTestEther Vulnerability details Impact If the promotion token applies transfer fees, the total amount to claim will be less than "tokensPerEpoch numberOfEpochs" bcs a part of this amount is the fee = funds + fee, but only the "funds" can be withdrawn but the calculation in...

TwabRewards: fee on transfer token as promotion token can make claimRewards() & cancelPromotion() fail

Handle GiveMeTestEther Vulnerability details Vulnerability details Impact If the rewards creator sets a token that applies transfer fees for the promotion token, users won't be able to claim some of their rewards because the claimRewards uses calculateRewardAmount that assumes for each epoch ther...

cancelPromotion will revert if the promotion is not started yet

Handle certora Vulnerability details cancelPromotion might fail unexpectedly. cancelPromotion calls getRemainingRewards which calls getCurrentEpochId: function getCurrentEpochIdPromotion memory promotion internal view returns uint256 // elapsedTimestamp / epochDurationTimestamp return...

Missing Check When Transferring Tokens Out For A Given Promotion

Handle leastwood Vulnerability details Impact The claimRewards function is called upon by ticket holders who parse a set of epochIds they wish to claim rewards on. An internal call is made to calculateRewardAmount to calculate the correct reward amount owed to the user. Subsequently, the...

After a promotion is cancelled, not claimed rewards are stuck

Handle 0x0x0x Vulnerability details After a promotion is cancelled using cancelPromotion, not awarded rewards for remaining epochs are sent to an address given by promotion creator. Awarded rewards not claimed by users stay in the contract. Since the promotion is deleted from promotions, users...

Large _epochId value impacts rewards calculation

Handle sirhashalot Vulnerability details Impact The epochId value is a uint256 that can be provided by the user in the epochIds array in the important claimRewards and getRewardsAmount functions. The epochId value should be between 0 and 255, as evidenced by the bit shifting of a uint256 type in...

Missing Validation Of createPromotion Parameters

Handle leastwood Vulnerability details Impact The createPromotion function is called by a creator account denoted as msg.sender to fund a promotion with tokens allocated on a per epoch basis across a set epochs. However, the function does not perform the necessary checks on function inputs to...

Reward stuck if promotion cancel before all past reward claimed

Handle gzeon Vulnerability details Impact When owner call cancelPromotion, the contract 1. Delete the promotion struct L132 2. Return all token reserved for future epochs L133 If there are token left for previous epochs, they will be stuck in the contract as the promotion struct is gone. Proof of...

Can drain any promotion rewards

Handle gzeon Vulnerability details Impact There are no checks to make sure epochId const promotionId = 1; const wallet2Amount = toWei'750'; const wallet3Amount = toWei'250'; await ticket.mintwallet2.address, wallet2Amount; await ticket.connectwallet2.delegatewallet2.address; await...

Tokens with fee on transfer are not supported

Handle WatchPug Vulnerability details There are ERC20 tokens that charge fee for every transfer or transferFrom. In the current implementation, createPromotion assumes that the received amount is the same as the transfer amount, and uses it to calculate reward amounts. As a result, in claimReward...

Non compatile with all tokens

Handle 0x1f8b Vulnerability details Impact It could not work with all tokens. Proof of Concept The method fromTokenAmount inside the contract Token18 can't work with tokens with 0 decimals, the result of UFixed18Lib.ratio will throw and error because it will divide by zero. It's mandatory to chec...

getRemainingRewards might fail unexpectedly

Handle certora Vulnerability details getRemainingRewards might fail unexpectedly. getRemainingRewards calls getRemainingRewards which calls getCurrentEpochId: function getCurrentEpochIdPromotion memory promotion internal view returns uint256 // elapsedTimestamp / epochDurationTimestamp return...

Can claim epoch > 255 repeatedly due to bitshift truncation

Handle gzeon Vulnerability details Can claim epoch 255 repeatedly due to bitshift truncation Impact TwabRewards contract store user claimed reward in a claimedEpochs bitmap. function isClaimedEpochuint256 userClaimedEpochs, uint256 epochId internal pure returns bool return userClaimedEpochs epoch...

StartTimestamp is not checked

Handle defsec Vulnerability details Impact When the promotion is created, the promotion.startTimestamp variable can be so old date. That will cause to affect to epoch calculation. Proof of Concept 1. Navigate to the following contract code. promotionsnextPromotionId = Promotion msg.sender, ticket...

Possibility to drain SavingsAccount contract assets

Handle kemmio Vulnerability details Impact A malicious actor can manipulate switchStrategy function in a way to withdraw tokens that are locked in SavingsAccount contract the risk severity should be reviewed Proof of Concept Firstly an attacker need to deploy a rogue strategy contract implementin...

AaveYield: Misspelled external function name making functions fail

Handle 0xngndev Vulnerability details Impact In AaveYield.sol the functions: liquidityToken withdrawETH depositETH Make a conditional call to IWETHGatewaywethGateway.getAWETHAddress This function does not exist in the wethGateway contract, causing these function to fail with the error "Fallback n...

getRewardsAmount doesn't check epochs haven't been claimed

Handle harleythedog Vulnerability details Impact In ITwabRewards.sol, it is claimed that getRewardsAmount should account for epochs that have already been claimed, and not include these epochs in the total amount indeed, there is a line that says @dev Will be 0 if user has already claimed rewards...

divide-before-multiply

Handle 0v3rf10w Vulnerability details Impact divide-before-multiply can lead to miscalculation of fees in below function Proof of Concept Vulnerable Function : Basket.handleFeesuint256 Basket.sol133-153 : uint256 feePct = timeDiff licenseFee / ONEYEAR; uint256 fee = startSupply feePct / BASE -...

Basket:handleFees(): fees are overcharged

Handle GiveMeTestEther Vulnerability details Impact The fee calculation is based on the totalSupply of the basket token. But some amount of the totalSupply represents the fees paid to the publisher/ protocol owner. Therefore the fees are "overcharged": because the fee amount is calculated on a pa...

Auction.sol#settleAuction() Bonder may not be able to settle a bonded auction, leading to loss of funds

Handle WatchPug Vulnerability details uint256 a = factory.auctionMultiplier basket.ibRatio; uint256 b = bondBlock - auctionStart BASE / factory.auctionDecrement; uint256 newRatio = a - b; address memory pendingTokens, uint256 memory pendingWeights, uint256 minIbRatio = basket.getPendingWeights;...

Publisher can scam users by changing license fees and using auction to freeze user funds

Handle 0x0x0x Vulnerability details Publisher can call changeLicenseFee to change fees. There is no maximum cap for this parameter. At first glance, because there is a timelock period of 1 day, the users have enough time to react. But if the publisher can start an auction and bondForRebalance. Th...

Fee calculation is slightly off

Handle gzeon Vulnerability details Impact The fee calculation uint256 timeDiff = block.timestamp - lastFee; uint256 feePct = timeDiff licenseFee / ONEYEAR; uint256 fee = startSupply feePct / BASE - feePct; tries to calculate a fee such that fee/supply+fee = %fee using a simple interest formula i....

Change in auctionMultiplier/auctionDecrement change profitability of auctions and factory can steal all tokens from a basket abusing it

Handle 0x0x0x Vulnerability details When factory changes auctionMultiplier or auctionDecrement profitability of bonded auctions change. There is no protection against this behaviour. Furthermore, factory owners can decide to get all tokens from baskets where they are bonded for the auction. Proof...

Missing cap on LicenseFee

Handle gzeon Vulnerability details Impact There is no cap on LicenseFee. While change of LicenseFee is under 1 day timelock, introducing a maxLicenseFee can improve credibility by removing the "rug" vector. There is a minLicenseFee in the contracts, while imo make little sense to have minLicenseF...

Reentrancy vulnerability in Basket contract's initialize() method.

Handle broccolirob Vulnerability details A malicious "publisher" can create a basket proposal that mixes real ERC20 tokens with a malicious ERC20 token containing a reentrancy callback in it's approve method. When the initialize method is called on the newly cloned Basket contract, a method calle...

Promotion creator can't cancel promotion before it has begun

Handle kenzo Vulnerability details If a promotion creator has created a promotion for the future, and decides to cancel it, he can not do so due to an underflow in a calculation. Impact Promotion creator funds will be locked until promotion begins. Proof of Concept Promotion's startTimestamp is...

Basket.sol#publishNewIndex() Lack of input validation may cause fund loss to anyone who bonds an auction

Handle WatchPug Vulnerability details According to the newRatio formula in settleAuction, the maximum value of newRatio is factory.auctionMultiplier basket.ibRatio. However, since there is no validation for the value of minIbRatio when setting it, if the publisher publishes a newIndex with...

Multiple initialization of Collateral contract

Handle 0x1f8b Vulnerability details Impact The attacker can initialize the contract, take malicious actions, and allow it to be re-initialized by the project without any error being noticed.. Proof of Concept The initialize method of the Collateral contract does not contain the initializer...

Basket.sol#auctionBurn calculates ibRatio wrong

Handle 0x0x0x Vulnerability details The function is implemented as follows: function auctionBurnuint256 amount onlyAuction nonReentrant external override uint256 startSupply = totalSupply; handleFeesstartSupply; burnmsg.sender, amount; uint256 newIbRatio = ibRatio startSupply / startSupply -...

Basket can be fully drained if the auction is settled within a specific block

Handle Ruhum Vulnerability details Impact The settleAuction function allows someone to settle the auction by transferring funds in a way that the new pending index is fulfilled. As a reward, they are able to take out as many tokens as they want as long as the pending index is fulfilled after that...

Lost fees due to precision loss in fees calculation

Handle kenzo Vulnerability details In fees calculation, division is being used in the midst of the calculation, not at the end of it. This leads to lost precision in fee amount as solidity doesn't save remainder of division. Division should happen at the end to maintain precision. Impact Lost fee...

Wrong emergencyWithdraw logic

Handle 0x1f8b Vulnerability details Impact The contract doesn't work as expected. Proof of Concept The method, emergencyWithdraw inside the contract yield/NoYield doesn't work as expected, the transfer was done with received value, and it should be done with amount, so the emergencyWithdraw never...

Publisher can lock all user funds in the Basket in order to force a user to have their bond burned

Handle TomFrenchBlockchain Vulnerability details Impact All user funds in a basket being held hostage by the publisher Proof of Concept The Basket publisher can propose an auction in order to set new tokens and weights with a 1 day timelock. As part of this call they can set the minIbRatio variab...

Wrong fee calculation after totalSupply was 0

Handle kenzo Vulnerability details handleFees does not update lastFee if startSupply == 0. This means that wrongly, extra fee tokens would be minted once the basket is resupplied and handleFees is called again. Impact Loss of user funds. The extra minting of fee tokens comes on the expense of the...

Missing SafeApprove(0)

Handle sirhashalot Vulnerability details There are instances where the SafeApprove function is called only once without setting the allowance to zero. Some tokens, like USDT, require first reducing the address' allowance to zero by calling approvespender, 0. Additionally, the comment before the...

UniswapV3's path issue for swapExactOutput

Handle cmichel Vulnerability details UniswapV3 expects a path object like tokenA, feeAB, tokenB, feeBC, tokenC. The UniV3Trader.swapExactOutput code tries to reverse this path to get to tokenC, feeBC, tokenB, feeAB, tokenA but that's not what the reverseBytes function does. Note that it reverts t...

Basket:handleFees fee calculation is wrong

Handle GiveMeTestEther Vulnerability details Impact The fee calculation on L141 is wrong. It should only get divided by BASE and not BASE - feePct Proof of Concept This shows dividing only by BASE is correct: Assumptions: BASE is 1e18 accordign to the code timeDiff is exactly ONEYEAR for easier...

Basket publisher can raise licenseFee in an unbounded fashion, stealing other users tokens

Handle TomFrenchBlockchain Vulnerability details Impact Publisher can make licenseFee arbitrarily large and then steal any funds remaining in the basket after 1 day. Proof of Concept On minting or burning basket tokens the handleFees function is called. This mints a number of basket tokens to the...

Bad redirects can make it impossible to deposit & withdraw

Handle cmichel Vulnerability details The GatewayVault.push function gets redirects from the strategyParams. If redirectsi = j, vault index i's deposits are redirected to vault index j. Note that the deposits for vault index i are cleared, as they are redirected: for uint256 j = 0; j The same is...

Unsafe implementation of fundLoan() allows attacker to steal collateral from an unfunded loan

Handle WatchPug Vulnerability details uint256 treasuryFee = fundsLent ILenderLikelender.treasuryFee paymentInterval paymentsRemaining / uint256365 days 10000; // Transfer delegate fee, if any, to the pool delegate, and decrement drawable funds. uint256 delegateFee = fundsLent...

Anyone can call closeLoan() to close the loan

Handle WatchPug Vulnerability details function closeLoanuint256 amount external override returns uint256 principal, uint256 interest // The amount specified is an optional amount to be transfer from the caller, as a convenience for EOAs. requireamount == uint2560 ||...

Unlimited allowances let anyone move funds to child vaults

Handle 0x0x0x Vulnerability details A malicious attacker can move someones funds between their vaults, since max allowances are given. As a consequence, a malicious user can block activities such as reclaimTokens. Although, funds are not directly stolen, it is possible to revert transactions of...

UniV3Vault.sol#collectEarnings() can be front run

Handle WatchPug Vulnerability details For UniV3Vault, it seems that lp fees are collected through collectEarnings callable by the strategy and reinvested rebalanced. However, in the current implementation, unharvested yields are not included in tvl, making it vulnerable to front-run attacks that...

ChiefTrader.sol Wrong implementation of swapExactInput() and swapExactOutput()

Handle WatchPug Vulnerability details When a caller calls ChiefTrader.solswapExactInput, it will call ITradertraderAddress.swapExactInput. return ITradertraderAddress.swapExactInput0, amount, recipient, path, options; However, in the current implementation, inputToken is not approved to the...

Governor can steal funds from vaults

Handle 0x0x0x Vulnerability details reclaimTokens function is as follows function reclaimTokensaddress to, address memory tokens external nonReentrant requirenft 0, ExceptionsLibrary.INITIALIZATION; IProtocolGovernance governance = vaultGovernance.internalParams.protocolGovernance; bool...

IsContract Function Usage

Handle defsec Vulnerability details Impact the isContract function that uses EXTCODESIZE was discovered to be hackable. The function will return false if it is invoked from a contract's constructor because the contract has not been deployed yet. The code should be used very carefully, if at all, ...

tokenWhitelist might break if removeFromTokenWhitelist is called multiple times with the same token

Handle gzeon Vulnerability details Impact In removeFromTokenWhitelist of ProtocolGovernance, numberOfValidTokens is decreased by 1 if addr is in tokenEverAdded. There are no check if the addr is still in the whitelist and therefore numberOfValidTokens can be repeatedly decreased to 0 by calling...