Yearn token <> shares conversion decimal issue

Handle cmichel Vulnerability details The yearn strategy YearnYield converts shares to tokens by doing pricePerFullShare shares / 1e18: function getTokensForSharesuint256 shares, address asset public view override returns uint256 amount if shares == 0 return 0; // @audit should divided by...

No slippage tolerance checks during swap functions opens up flashloan attacks and price manipulation

Handle jayjonah8 Vulnerability details Impact With functions like joinTokenSingle in SingleTokenJoin.sol that handle swapping with uniswapV2 like exchanges there are no slippage tolerance checks in place to prevent flashloan attacks and price manipulation. Proof of Concept Tools Used Manual code...

withdrawTo Does Not Sync Before Checking A Position's Margin Requirements

Handle leastwood Vulnerability details Impact The maintenanceInvariant modifier in Collateral aims to check if a user meets the margin requirements to withdraw collateral by checking its current and next maintenance. maintenanceInvariant inevitably calls AccountPosition.maintenance which uses the...

Wrong implementation of NoYield.sol#emergencyWithdraw()

Handle WatchPug Vulnerability details function emergencyWithdrawaddress asset, address payable wallet external onlyOwner returns uint256 received requirewallet != address0, 'cant burn'; uint256 amount = IERC20asset.balanceOfaddressthis; IERC20asset.safeTransferwallet, received; received = amount;...

Collateral can be deposited in a finished pool

Handle pedroais Vulnerability details Proof of Concept The depositCollateral function doesn't check the status of the pool so collateral can be deposited in a finished loan. This can happen by mistake and all funds will be lost. Recommended Mitigation Steps Require loan status to be collection or...

Wrong implementation of AaveYield.sol causing users to lose yields

Handle WatchPug Vulnerability details The current implementation of AaveYield.sol is taking AAVE aToken as a share token eg, cToken and yToken. However, AAVE's aTokens are quite different from cToken and yToken as it's always 1:1 to the underlying token, and the holder's balance will keep changin...

Extension voting threshold check needs to rerun on each transfer

Handle cmichel Vulnerability details The Extension contract correctly reduces votes from the from address of a transfer and adds it to the to address of the transfer in case both of them voted on it before, but it does not rerun the voting logic in voteOnExtension that actually grants the...

Failed transfer with low level call could be overlooked

Handle harleythedog Vulnerability details Impact There are several places where low level call is used within the contract. In particular, in SavingsAccount.sol, the external withdraw function has the to argument, which is eventually used in this code in transfer: bool success, = to.callvalue:...

It is possible to liquidate not existing account

Handle 0x0x0x Vulnerability details It is possible to liquidate an address for any product when collateral = maintenance = 0. So in other words, if a user have never used a product and deposited collateral, the user can get liquidated blacklisted from the product by anyone, since a liquidated...

Incentivizer rewards are unclear and can be more than allotted?

Handle cmichel Vulnerability details Note that the Incentivizer.create allocates programInfo.amount.sum tokens to be rewarded. It's unclear how the rewards that are actually paid out see Program.unsettled over the duration relate to this value. The ProgramInfo.amountPerShare returns the...

In CreditLine#liquidate, lender doesn't receive _borrowTokens, when it is ETH

Handle 0x0x0x Vulnerability details When borrowAsset == address0, the liquidator sends ethereum via transaction value msg.value, but borrowTokens amount is not sent to the lender. Therefore, lender losses funds. As seen in: Mitigation step: Add bool success, = lender.callvalue: borrowTokens''; at...

DoS when depositing collateral

Handle cmichel Vulnerability details The Pool.depositCollateral and Pool.addCollateralInMarginCall functions require the caller to have a pool token balance of zero. function depositCollateraluint256 amount, bool transferFromSavingsAccount external payable override requireamount != 0, 'DC1'; //...

NoYield.sol 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, NoYield.sollockTokens assumes that the received amount is the same as the transfer amount, and uses it to calculate sharesReceived amounts. As a result, ...

No checks if given product is created by the factory

Handle 0x0x0x Vulnerability details An attacker can create a fake product. Collateral contract does not check whether the given product is created by the factory. A malicious product can return arbitrary maintenance amounts, therefore they can make any deposit to fake product stuck simply return...

ChainLink price data could be stale

Handle cmichel Vulnerability details There is no check in ChainlinkOracle.sync if the oracle 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...

CreditLine.liquidate doesn't transfer borrowed ETH to a lender

Handle hyh Vulnerability details Impact Funds that are acquired from a liquidator and should be sent to a lender are left with the contract instead. The funds aren't lost, but after the fact mitigation will require manual accounting and fund transfer for each CreditLine.liquidate usage. Proof of...

outputBasket address not checked in EthSingleTokenJoin.sol

Handle jayjonah8 Vulnerability details Impact In the joinToKenEth function in EthSingleTokenJoin.sol, the outputBasket within the passed in data is not checked and could be an attacker controlled address returning any thing it wants in fake versions of calcTokensForAmount and joinPool functions...

no protection from sandwich attacks

Handle certora Vulnerability details the function joinTokenSingle has no protection from slippage and sandwich attacks. the balance after the swaps is not checked to have a minimum and the min amount in the swaps is set to zero: Impact malicious actors can front-run joinTokenSingle and perform a...

Chainlink's latestRoundData might return stale or incorrect results

Handle WatchPug Vulnerability details function sync public , int256 feedPrice, , uint256 timestamp, = feed.latestRoundData; Fixed18 price = Fixed18Lib.ratiofeedPrice, SafeCast.toInt256decimalOffset; if priceAtVersion.length == 0 || timestamp timestampAtVersioncurrentVersion + minDelay...

SHOULD CHECK RETURN DATA FROM CHAINLINK AGGREGATORS

Handle defsec Vulnerability details Impact The sync function in the contract ChainlinkOracle.sol fetches the asset price from a Chainlink aggregator using the latestRoundData function. However, there are no checks on roundID, resulting in stale prices. The oracle wrapper calls out to a chainlink...

SavingsAccount withdrawAll and switchStrategy can freeze user funds by ignoring possible strategy liquidity issues

Handle hyh Vulnerability details Impact Full withdrawal and moving funds between strategies can lead to wrong accounting if the corresponding market has tight liquidity, which can be the case at least for AaveYield. That is, as the whole amount is required to be moved at once from Aave, both...

In SingleTokenJoinV2.sol, a user can provide malicious input

Handle jayjonah8 Vulnerability details Impact In SingleTokenJoinV2.sol, a user can provide malicious input to the joinTokenSingle function providing an attacker controlled inputToken and an attacker controlled outputBasket. This fake outputBasket can return anything it wants from a fake joinPool...

Approved by zero first

Handle Jujic Vulnerability details Some tokens like USDT do not work when changing the allowance from an existing non-zero allowance value. They must first be approved by zero and then the actual allowance must be approved. Impact When using one of these unsupported tokens, all transactions rever...

JoinTokenSingle() function does not validate against evil struct inputs

Handle jayjonah8 Vulnerability details Impact In SingleTokenJoin.sol, the joinTokenSingle function allows a user to add any token address as an inputToken and any address as the outputBasket address without validating the struct data to guard against malicious input Proof of Concept An attacker...

transfer return value is ignored

Handle robee Vulnerability details Need to use safeTransfer instead of transfer. As there are popular tokens, such as USDT that transfer/trasnferFrom method doesn’t return anything. The transfer return value has to be checked as there are some other tokens that returns false instead revert, that...

Function sync in ChainlinkOracle.sol does not check the price returned from chainlink aggregators

Handle ye0lde Vulnerability details Impact The sync function in the contract ChainlinkOracle.sol fetches the feedPrice' from a Chainlink aggregator using the latestRoundDatafunction. There are checks on thetimeStampversus previous versions of the feed protecting against stale prices. But there is...

Unlinked address can link immediately again

Handle gpersoon Vulnerability details Impact After a master calls unlinkAddress to unlink an address, the address that has just been unlinked can directly link again without permission. The address that is just unlinked can call linkAddressmasterAddress which will execute because...

Incorrect maintenance invariant check

Handle kenzo Vulnerability details In Collateral, maintenanceInvariant is used to check whether after withdrawal the user will have enough funds to cover the maintenance for his current position and for his next position. However, the modifier is wrongly checking this via a max function: if...

PriceOracle Does Not Filter Price Feed Outliers

Handle leastwood Vulnerability details Impact If for whatever reason the Chainlink oracle returns a malformed price due to oracle manipulation or a malfunctioned price, the result will be passed onto users, causing unintended consequences as a result. In the same time it's possible to construct...

setLock() function has no validation on the _lock number

Handle jayjonah8 Vulnerability details Impact In BasketFacet.sol, the setLock function allows the privileged caller to change the lockBlock number in storage without validating the lock arg number passed into it. This number is extremely important since if getLock returns true, it means the pool ...

Unable To Call emergencyWithdraw ETH in NoYield Contract

Handle leastwood Vulnerability details Impact The emergencyWithdraw function is implemented in all yield sources to allow the onlyOwner role to drain the contract's balance in case of emergency. The contract considers ETH as a zero address asset. However, there is a call made on asset which will...

Wrong shortfall calculation

Handle kenzo Vulnerability details Every time an account is settled, if shortfall is created, due to a wrong calculation shortfall will double in size and add the new shortfall. Impact Loss of funds: users won't be able to withdraw the correct amount of funds. Somebody would have to donate funds ...

Allowing more than 256 epochs leads to loss of funds

Handle cmichel Vulnerability details The TwabRewards contract has an implicit restriction of 256 epochs per promotion as it uses a bitmask in a uint256 to mark claimed epochs 0-255, see isClaimedEpoch. "/// @dev We pack epochs claimed by a user into a uint256. So we can't store more than 255...

Unsafe uint64 casting may overflow

Handle sirhashalot Vulnerability details Impact The calculateRewardAmount function casts epoch timestamps from uint256 to uint64 and these may overflow. The epochStartTimestamp value is a function of the user-supplied epochId value, which could be extremely large up to 2255 – 1. While Solidity...

Suggestion : To add explicit check for no of epochs to be 255 in createPromotion and extendPromotion

Handle 0x421f Vulnerability details So we are using uint256 to store 1/0 if that epoch is being claimed or not uint2561 is our base, so 255 is max limit we have now if someone creates a promotion with 255 epochs in epochs 255 , it will revert with "TypeError : Invalid Rational Number" Copied from...

Attacker can empty all the funds by creating fake promotions

Handle WatchPug Vulnerability details The current implementation of calculateRewardAmount allows a arbitrary epochId, which can even be a epochId numberOfEpochs. A malicious user can call claimRewards with epochIds larger than numberOfEpochs and claim other users' rewards. Furthermore, since...

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

Can drain any promotion rewards with a evil ticket

Handle gzeon Vulnerability details Impact TwabRewards check legitimacy of ticket by checking if the ticket have a controller method. function requireTicketaddress ticket internal view requireticket != address0, "TwabRewards/ticket-not-zero-address"; bool succeeded, bytes memory data =...

Support of deflationary / rebasing tokens

Handle pauliax Vulnerability details Impact Deflationary fee on transfer / rebasing tokens are not supported. Because anyone can createPromotion with an arbitrary token, such tokens may be lost forever. Recommended Mitigation Steps Consider checking the actual amounts transferred balance...

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

cancelPromotion() Unable to cancel unstarted promotions

Handle WatchPug Vulnerability details For unstarted promotions, cancelPromotion will revert at block.timestamp - promotion.startTimestamp in getCurrentEpochId. Call stack: cancelPromotion - getRemainingRewards - getCurrentEpochId. function getRemainingRewardsPromotion memory promotion internal vi...

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

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

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

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

Drain the award pool by feeding outrange epoch into function claimRewards

Handle 0xabc Vulnerability details Impact in function claimRewards, there is no checking on value in epochIds. Exploiter can claim more than the promotion award by calling the function with outrange epochs. Proof of Concept Consider a promotion with promitionid X and numberOfEpochs =Y. Exploiter...

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

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

Dust Token Balances Cannot Be Claimed By An admin Account

Handle leastwood Vulnerability details Impact Users who have a small claim on rewards for various promotions, may not feasibly be able to claim these rewards as gas costs could outweigh the sum they receive in return. Hence, it is likely that a dust balance accrues overtime for tokens allocated f...

unsafe cast can lead to theft

Handle certora Vulnerability details claimRewards gets epochs ids as uint256. However, it should be uint8. If a user provides an epoch Id that's larger than 256, isClaimedEpoch will return false: function isClaimedEpochuint256 userClaimedEpochs, uint256 epochId internal pure returns bool return...