Potential reentrance in claimRewards

Judge @GalloDaSballo has assessed the 1st item in QA Report 36 as Medium risk. The relevant finding follows: … POC IERC20tokensi.safeTransfermsg.sender, getting; rewardmsg.sendertokensi = 0; Considering there are exterTokens, it is possible that some token will provide reentry opportunities...

CvxCrvRewardsLocker implements a swap without a slippage check that can result in a loss of funds through MEV

Lines of code Vulnerability details Impact The CvxCrvRewardsLocker contract swaps tokens through the CRV cvxCRV pool. But, it doesn't use any slippage checks. The swap is at risk of being frontrun / sandwiched which will result in a loss of funds. Since MEV is very prominent I think the chance of...

AmmGauge stake allows for reentrancy that can lead to stealing the contract balance

Lines of code Vulnerability details Impact Some ERC20 do allow for user's control of execution. For example, ERC777 has tokensReceived hook. This way, an ability to reenter can be executed with the usage of any such tokens. AmmGauge stake do not control for reentrancy and uses balance difference ...

Lack of safeApprove(0) prevents some registrations, and the changing of stakers and LP tokens

Lines of code Vulnerability details OpenZeppelin's safeApprove will revert if the account already is approved and the new safeApprove is done with a non-zero value function safeApprove IERC20 token, address spender, uint256 value internal // safeApprove should only be called when setting an initi...

RoleManager.sol: _revokeRole doesn't remove from the _roleMembers[role] set

Lines of code Vulnerability details Impact In the file RoleManager.sol the function revokeRole doesn't remove the account from the roleMembersrole set. This makes getRoleMemberCount wrong for the else part therefore the renounceGovernance require, on the number of governor, useless and risky as...

Incorrect accounting of free weight in _decrementWeightUntilFree

Lines of code Vulnerability details Impact In decrementWeightUntilFree, the free weight is calculated by balanceOfuser - getUserWeightuser plus weight freed from non-deprecated gauges. The non-deprecated criteria is unnecessary and lead to incorrect accounting of free weight. Proof of Concept...

First xERC4626 deposit exploit can break share calculation

Lines of code Vulnerability details Solmate convertToShares function follow the formula: assetDepositAmount totalShareSupply / assetBalanceBeforeDeposit. The share price always return 1:1 with asset token. If everything work normally, share price will slowly increase with time to 1:2 or 1:10 as...

CompoundHandler and other contracts call native payable.transfer

Title CompoundHandler topUp, EthPool doTransferOut and VaultReserve withdraw call native payable.transfer Impact Multiple operations are handled by using a payable.transfer call. transfer has a gas budget limit which is unsafe because gas costs can and may change and can fail when the user is a...

If currentMonth in init is 0, then CPI update will revert, zero div

This is a manual upgrade of the sixth item in QA report 86 , per judge @jack-the-pug's assessment of it as a Medium risk issue. If currentMonth in init is 0, then CPI update will revert, zero div --- The text was updated successfully, but these errors were encountered: All reactions...

Oracle data feeds are insufficiently validated

Lines of code Vulnerability details Impact If the oracle price feeds are insufficiently validated, there will be pricing errors leading to the miss-pricing of assets/risk Proof of Concept The code does not verify that answeredInRound = roundID for both cases where an oracle is used, and the...

Swapper3Crv.sol use the wrong address for sushiswap

Lines of code Vulnerability details Impact This seems to be out of scope, but still worth mentioning. This line should be address sushiSwap = SUSHISWAP; --- The text was updated successfully, but these errors were encountered: All reactions...

first depositor can drain other depositors

Lines of code Vulnerability details in deposit, when the ratio totalSupply / balance is very high, the amount of the minted shares can round down to zero. Proof of Concept Alice is the first one to deposit in LiquidityPool. she deposits 1 basic unit of the token, therefore minting one lp token...

Missing validations for return value of oracle data feed.

Lines of code Vulnerability details Impact In ChainlinkUsdWrapper there are no validations for answerthe price if the price is 0 or not. I checked ethOracle0x5f4eC3Df9cbd43714FE2740f5E3616155c5b8419..latestRoundData. However, this contract has no validation for the price too. In addition to that,...

setFlywheelRewards can take any rewardToken

Lines of code Vulnerability details Impact Though setFlywheelRewards has requiresAuth, it still has rug risk that a privileged user can move all rewardToken of flywheelRewards to new malicious newFlywheelRewards unconditionally. Proof of Concept A malicious user or a compromised admin can call...

BufferStored could be larger than BufferCap after _setBufferCap()

Judge @jack-the-pug has assessed the second item in QA Report 64 as Medium risk. The relevant finding follows: Impact In RateLimited.sol BufferCap should be the upper bound of BufferStored, However in setBufferCap it calls updateBufferStored before replacing the old BufferCap. If old BufferCap is...

ChainlinkOracleProvider can provide zero and stale prices

Lines of code Vulnerability details Impact As stale price is determined by time since last timestamp, the price that is most recent, but wasn't updated for more than 2 hours say there were no trades on the market will be rejected, which makes system functionality unavailable in such a case. This...

_incrementGaugeWeight allows user to add weight to nonexistent gauges

Lines of code Vulnerability details Impact User adds weight to a gauge that hasn't been added In addition to adding to a nonexistent gauge it also increments totalWeight which only contains weight for live gauges. This value then results in returning values for reward distribution that account fo...

Customers cannot be topUp()ed a second time

Lines of code Vulnerability details OpenZeppelin's safeApprove will revert if the account already is approved and the new safeApprove is done with a non-zero value function safeApprove IERC20 token, address spender, uint256 value internal // safeApprove should only be called when setting an initi...

EthPool use payable.transfer with an arbitrary receiver

Lines of code Vulnerability details Impact EthPool sends out native tokens via payable.transfer call. This is unsafe as transfer has hard coded gas budget and can fail when the to is a smart contract. Such transactions will fail for smart contract users which don't fit to 2300 gas stipend transfe...

Customers cannot redeem() LP tokens to non-EOA accounts

Lines of code Vulnerability details The use of payable.transfer is heavily frowned upon because it can lead to the locking of funds. The transfer call requires that the recipient has a payable callback, only provides 2300 gas for its operation. This means the following cases can cause the transfe...

_revokeRole doesn't remove account from roleMember set

Lines of code Vulnerability details Impact The function doesn't remove the address from roleMembersrole set, which will mess up with the roleCount Proof of Concept Tools Used Recommended Mitigation Steps rolesrole.membersaccount = false; roleMembersrole.removeaccount; --- The text was updated...

Division by zero in isWithinDeviationThreshold

Judge @jack-the-pug is upgrading the following issue from a QA report issue 30 to Medium risk: Division by zero in isWithinDeviationThreshold if a is zero. This only seems to be the case if the oracle would return 0 for CPI and in this case, something is wrong anyway. Should still handle this err...

[WP-H0] xERC4626.sol Some users may not be able to withdraw until rewardsCycleEnd the due to underflow in beforeWithdraw()

Lines of code Vulnerability details function beforeWithdrawuint256 amount, uint256 shares internal virtual override super.beforeWithdrawamount, shares; storedTotalAssets -= amount; function syncRewards public virtual uint192 lastRewardAmount = lastRewardAmount; uint32 timestamp =...

Malicious Stakers can grief Keepers

Lines of code Vulnerability details Impact A Staker -- that has their top-up position removed after execute is called by a Keeper -- can always cause the transaction to revert. They can do this by deploying a smart contract to the payer address that has implemented a receive function that calls...

Swapper3Crv's swapping path can be suboptimal

Lines of code Vulnerability details Impact Swapper3Crv.swap result can be suboptimal as only paths with ETH are evaluated. Setting severity to medium as despite function availability not affected there can be some fund losses as a result. Proof of Concept tokenAmountOut uses fixed tokenIn, ETH,...

[WP-H15] AmmConvexGauge.sol#poolCheckpoint() cvxStakedIntegral can be manipulated by the attacker

Lines of code Vulnerability details function poolCheckpoint public virtual override returns bool if killed return false; uint256 timeElapsed = block.timestamp - uint256ammLastUpdated; uint256 currentRate = IControllercontroller.inflationManager.getAmmRateForToken ammToken ; uint256 crvEarned =...

ETHVault, BkdEthCvx and VaultReserve use payable.transfer for inter-system transfers

Lines of code Vulnerability details Impact These contracts use payable.transfer for internal fund transfer, where the recipients are vaults, pools, strategies. This is generally unsafe as transfer has hard coded gas budget and can fail when the to is a smart contract. Such transactions will fail...

Function deposit can receive both ETH and tokens, but only compute tokens

Lines of code Vulnerability details Impact ETH can be transfered to the contract without being computed as a deposit. Proof of Concept The function depositVaultReserve.sol can accept both tokens and ETH. Suppose that Vault accidentally transfer eth and an amount of tokens . The contract will...

Unsafe use of .transfer

Lines of code Vulnerability details Impact User funds can be locked by using a smart contract wallet with inefficient callback, or by future hardforks that change the gas consumption. address.transfer has been suggested to deprecate by most auditors, because of the potential OOG error. However,...

FlywheelCore's setFlywheelRewards can remove access to reward funds from current users

Lines of code Vulnerability details Impact FlywheelCore.setFlywheelRewards can remove current reward funds from the current users' reach as it doesn't check that newFlywheelRewards' FlywheelCore is this contract. If it's not, by mistake or with a malicious intent, the users will lose the access t...

function lockFunds in TopUpActionLibrary can cause serious fund lose. fee and Capped bypass. It's not calling stakerVault.increaseActionLockedBalance when transfers stakes.

Lines of code Vulnerability details Impact In function TopUpActionLibrary.lockFunds when transfers stakes from payer it doesn't call stakerVault.increaseActionLockedBalance for that payer so stakerVault.actionLockedBalancespayer is not get updated for payer and...

Unlimited reward minting with Function Transfer in StakerVault (updates balances before calling userCheckpoint)

Lines of code Vulnerability details Impact The bug in "StakerVault.transfer" function which is externally callable is that first it is updating the balance of sender and receiver then it calls ILpGaugelpGauge.userCheckpoint for those addresses. Function userCheckpoint use balance of address to...

User can steal all rewards due to checkpoint after transfer

Lines of code Vulnerability details Impact I believe this to be a high severity vulnerability that is potentially included in the currently deployed StakerVault.sol contract also. The team will be contacted immediately following the submission of this report. In StakerVault.sol, the user...

FlywheelCore.setBooster() can be used to steal unclaimed rewards

Lines of code Vulnerability details Impact A malicious authorized user can steal all unclaimed rewards and break the reward accounting Even if the authorized user is benevolent the fact that there is a rug vector available may negatively impact the protocol's reputation. Furthermore since this...

In ERC20Gauges, contribution to total weight is double-counted when incrementGauge is called before addGauge for a given gauge.

Lines of code Vulnerability details Impact The impact depends really on how gauges are used by other contracts. The most obvious consequence I can imagine is that some other contract distributes rewards based on calculateGaugeAllocation. However, because getStoredWeighttotalWeight, currentCycle i...

Unsafe ERC20 transfer Operations

Findings Unsafe ERC20 transfer Operations The transfer and transferFrom functions return a Boolean value which should be checked for successful transfer, Some tokens do not revert if the transfer did fail but return false. the protocol Joyn do have some lines for code that make a use of these...

Transfer return value is ignored

Impact Some ERC20 tokens, such as USDT, don't revert when transfer/transferFrom fails. The transfer return value has to be checked as there are some other tokens that returns false instead revert. safeTransfer should be used instead of transfer Proof of Concept safeTransferFrom should be used...

Don't ignore ERC20 transfer return values

You're ignoring the return value of an ERC20 transfer twice: Either use SafeERC20 or check the return value as you do in other places in the code base. --- The text was updated successfully, but these errors were encountered: All reactions...

transfer or transferFrom without checking the boolean result

It was found some transfer, approve or transferFrom without checking the boolean result, ERC20 standard specify that the token can return false if this call was not made, so it's mandatory to check the result of approve methods. CoreCollection.solL175 ERC721Payable.solL54 --- The text was updated...

use safe erc20 functions

ERC721Payable.handlePayment core-contracts/contracts/ERC721Payable.sol50-56 ignores return value by payableToken.transferFrom core-contracts/contracts/ERC721Payable.sol54 consider safeTransferFrom to prevent any locks or loss of funds --- The text was updated successfully, but these errors were...

Attacker can cause minter to lose his funds by calling sync() between deposit and mint()

Lines of code Vulnerability details Impact All funds will be lost Proof of Concept To mint new index tokens underlying tokens need to be deposited in Vtoken.sol and then call the mint function. The mint function does balance-previousBalance to calculate tokens that were deposited. The previous...

Index managers can rug user funds

Lines of code Vulnerability details Impact The ORDERERROLE role has the ability to arbitrarily transfer user funds, and this role is shared between both the orderer and people who can rebalance the index. Even if the owner is benevolent the fact that there is a rug vector available may negatively...

Unchecked parameters in Chainlink latestRounddata() could lead to retrieve stale data

Lines of code Vulnerability details Impact Oracle can retrieve unchecked stale data Proof of Concept In ChainlinkpriceOracle.sol the function function refreshedAssetPerBaseInUQaddress asset public override returns uint AssetInfo storage assetInfo = assetInfoOfasset; , int basePrice, , , =...

Index Minting and Redemption Can Be Frontrun

Lines of code Vulnerability details Impact According to the provided source code, the user must transfer the underlying asset to the contract prior to calling mint or the index token before to calling burn. If these two actions are performed on the difference block, it introduces the risk that...

Wrong requirement in reweight function (ManagedIndexReweightingLogic.sol)

Lines of code Vulnerability details Impact The list of assets won't be changed after reweight because of reverted tx Proof of Concept requireupdatedAssets.length = IIndexRegistryregistry.maxComponents when reweight is not true, because as in the doc, maxComponent is the maximum assets for an inde...

basePrice and quotePrice values are insufficiently validated

Lines of code Vulnerability details Impact The oracle data feed basePrice and quotePrice in refreshedAssetPerBaseInUQ of ChainlinkPriceOracle.sol will be stale which results in wrong basePrice value and quotePrice value. Proof of Concept In refreshedAssetPerBaseInUQ function, it calls...

UniswapV2PriceOracle.sol currentCumulativePrices() will revert when priceCumulative addition overflow

Lines of code Vulnerability details uint price0Cumulative, uint price1Cumulative, uint32 blockTimestamp = addresspair.currentCumulativePrices; Because the Solidity version used by the current implementation of UniswapV2OracleLibrary.sol is =0.8.7, and there are some breaking changes in Solidity...

Missing Validations for the return values of Chainlink Price feeds

Lines of code Vulnerability details Impact You check only the answerThe price after calling the chainlink Chainlink Price feeds in the following lines. In addition, you need to check whether the data is really updated. Proof of Concept Tools Used code review Recommended Mitigation Steps Please ad...

Insufficient oracle data feed validation

Lines of code Vulnerability details Impact Stale prices can lead to the incorrect valuation of assets Proof of Concept The code does not check the other data returned from latestRoundData which must be used to ensure that the data is not stale and that the price is valid File:...

Chainlink's latestRoundData might return stale or incorrect results

Lines of code Vulnerability details Impact In ChainlinkPriceOracle.sol, latestRoundData is used but there is no check if the return value indicates stale data. This could lead to stale prices according to the Chainlink documentation: Proof of Concept ChainlinkPriceOracle.solL83...