Upgraded Q -> M from 225 [1655654402923]

Judge has assessed an item in Issue 225 as Medium risk. The relevant finding follows: C4-011 : Centralization Risk On The teamSummon Function - LOW Impact - LOW With the teamSummon function, owner can mint unlimited warriors. This poses a security risk. The max/min limit should be implemented at...

_transferNFTs() functions doesn't reverts if the transfer item doesn't supports interface for both ERC721 and ERC1155

Lines of code Vulnerability details Impact The transferNFTs function use ERC165 to check if the itemnft supports ERC721 interface or ERC1155 interface and execute transfer accordingly. But if it doesn't supports either, it just exits the functionno revert. Proof of Concept in...

Vault can never fully be emptied

Lines of code Vulnerability details Impact Vault cannot be fully emptied Proof of Concept Whenever rewards are earned they are automatically locked into the the Aura Locker. Since that reward will then earn more rewards while locked, there will be more rewards to be collected when that lock is...

Missing slippage protection for autocompounding auraBAL rewards into AURA

Lines of code Vulnerability details Impact Autocompounding auraBAL rewards into AURA requires multiple swaps auraBAL - BAL/ETH BPT - WETH - AURA within MyStrategy.harvest. The swaps are at risk of being front-run / sandwiched, resulting in a loss of funds. Since MEV is very prominent I think the...

Upgraded Q -> M from 143 [1655579880057]

Judge has assessed an item in Issue 143 as Medium risk. The relevant finding follows: L-04 safeTransferETH should perform simple ETH transfers and don’t forward 30k gas Link: Being a simple funds transfer, having a fallback of a WETH deposit, there should be no extra gas involved when potentially...

prepareWithdrawAll should not be external

Lines of code Vulnerability details Impact Despite commented as Internal Core Implementations, prepareWithdrawAll is external. There also lack guarantee that prepareWithdrawAll is called before withdrawAll. Proof of Concept /// ===== Internal Core Implementations ===== function prepareWithdrawAll...

Upgraded Q -> M from 270 [1655579826704]

Judge has assessed an item in Issue 270 as Medium risk. The relevant finding follows: Gas stipend for payable.send may be too low for contract wallets ETH withdrawals in both the minter and token contracts use payableaddress.send to transfer ether to the vault address. If the configured vault is ...

_harvest has no slippage protection when swapping auraBAL for AURA

Lines of code Vulnerability details Impact Single swaps of harvest contains no slippage or deadline, which makes it vulnerable to sandwich attacks, MEV exploits and may lead to significant loss of yield. Proof of Concept When using BALANCERVAULT.swap here and here, there is no slippage protection...

Upgraded Q -> M from 91 [1655579814367]

Judge has assessed an item in Issue 91 as Medium risk. The relevant finding follows: use call instead of send affected code: Use of transfer might render ETH impossible to withdraw because after istanbul hardfork , there is increases in the gas cost of the SLOAD operation and therefore breaks som...

Upgraded Q -> M from 47 [1655572104366]

Judge has assessed an item in Issue 47 as Medium risk. The relevant finding follows: 1. Unsafe use of transfer/transferFrom with IERC20 Some tokens do not implement the ERC20 standard properly but are still accepted by most code that accepts ERC20 tokens. For example Tether USDT's transfer and...

Reward token (auraBal) can be locked in the strategy

Lines of code Vulnerability details Impact Detailed description of the impact of this finding. getRewardaddress account function of Aura Locker is an external function therefore can be called by anyone by passing in the address of strategy and transferring the rewards to the strategy. harvest...

Attacker can grief users from withdrawing their tokens and causing users to loose money

Lines of code Vulnerability details Impact balance of this contract of vault tokens in the require statement can stop users from withdrawing their tokens 2 issues: 1. vaultbalancebefore= balance of this contract from a vault tokens vaultamount=vaultbalancebefore- balance of vault right then when...

Missing reentrancy protections

Lines of code Vulnerability details Impact The files below contain both deposit and withdraw functions which seem re-entrable at the point of calls that transfer tokens. The functions do not fully follow a checks-effects-interactions pattern, thus they can be re-entered multiple times. Depending ...

Residual Allowance Might Allow Tokens In SetToken To Be Stolen

Lines of code Vulnerability details Proof-of-Concept Whenever mintFCashPosition function is called to mint new fCash position, the contract will call the approve function to set the allowance to maxSendAmount so that the fCash Wrapper contact can pull the payment tokens from the SetToken contract...

Upgraded Q -> M from 307 [1655245513660]

Judge has assessed an item in Issue 307 as Medium risk. The relevant finding follows: Low feeRate can be modified for existing vaults feeRate is a parameter that controls the fee applied on exercise. It can be set by the function: function setFeeuint256 feeRate external onlyOwner feeRate = feeRat...

No Slippage Protection During Redeeming Matured Positions

Lines of code Vulnerability details Proof-of-Concept The lack of slippage checks can cause redemption of matured positions to happen at a bad rate/prices, resulting the trade to receive fewer tokens than the fair market rate/price. The NotionalTradeModule.redeemMaturedPositions function calls the...

Upgraded Q -> M from 294 [1654474576938]

Judge has assessed an item in Issue 294 as Medium risk. The relevant finding follows: Owner can frontrun exercise to increase fees A malicious owner account can observe and frontrun calls to exercise and extract 100% of the strike price as a protocol fee. Scenario: A malicious owner observes a ca...

Upgraded Q -> M from 124 [1654442937681]

Judge has assessed an item in Issue 124 as Medium risk. The relevant finding follows: C4-006 : The Contract Should Approve0 first Impact Some tokens like USDT L199 do not work when changing the allowance from an existing non-zero allowance value. They must first be approved by zero and then the...

More than 1 zero address token in burnToTarget may lead to draining of FeeBurner.sol

Lines of code FeeBurner.solL43-L88 Vulnerability details Impact The burnToTarget function in FeeBurner.sol allows any array of tokens to be used. If the tokens array contains more than 1 zero address, the swapAll function for the swapperRouter will be called more than once with the same msg.value...

Usage of deprecated transfer to send ETH

Lines of code Vulnerability details Impact Usage of deprecated transfer Swap can revert. Proof of Concept The original transfer used to send eth uses a fixed stipend 2300 gas. This was used to prevent reentrancy. However this limit your protocol to interact with others contracts that need more th...

Attacker can get drain ETH for targetLpToken_

Lines of code Vulnerability details Impact Attacker can drain all ETH from FeeBurner.sol. Technically msg.value gets sent to swapperRouter, but since this contract is out of scope FeeBurner.sol will be treated as the victim Proof of Concept FeeBurner.solL56-L65 ... for uint256 i; i tokens.length;...

RewardHandler.soland FeeBurner.sol will fail due to inexistent AddressProvider.sol getter functions

Lines of code Vulnerability details Impact FeeBurner.burnToTarget and RewardHandler.burnFees will fail every time because addressProvider.getSwapperRouter, addressProvider.getBKDLocker and addressProvider.getFeeBurner are not implemented in AddressProvider.sol. Proof of Concept getSwapperRouter,...

Approving from non-zero to non-zero allowance will revert with OZ's safeApprove()

Lines of code Vulnerability details Impact Transaction reverting. Proof of Concept OZ's implementation of safeApprove would throw an error if an approve is attempted from a non-zero value "SafeERC20: approve from non-zero to non-zero allowance": require value == 0 || token.allowanceaddressthis,...

Minter.sol#startInflation() can be bypassed

Lines of code Vulnerability details function startInflation external override onlyGovernance requirelastEvent == 0, "Inflation has already started."; lastEvent = block.timestamp; lastInflationDecay = block.timestamp; As lastEvent and lastInflationDecay are not initialized in the constructor, they...

Bad use of safeApprove

Lines of code Vulnerability details Impact Function deposit in booster could revert when stake option is true if the contract has an allowance of the token for rewarcontract 0. The same in many contracts but in VE3DRewardPool.sol a user can get his rewards blocked. Proof of Concept If the contrac...

Owner's delegates should be decreased in _burn()

Lines of code Vulnerability details function burnuint tokenId internal requireisApprovedOrOwnermsg.sender, tokenId, "caller is not owner nor approved"; address owner = ownerOftokenId; // Clear approval approveaddress0, tokenId; // TODO add delegates // Remove token removeTokenFrommsg.sender,...

QA Report

Impact RubiconRouter.swapEntireBalance is supposed to buy at least buyamtmin of Y output tokens by using the caller's entire X input token balance. The problem is that instead of passing the buyamtmin minus the fee to swap, the code uses the caller's X token balance, maxAmount. This means the cal...

Steal user's first deposit

Lines of code Vulnerability details Impact Steal user's first deposit Proof of Concept The attack's flow is as following: 1. The first depositor wants to deposit X in bathToken.sol. 2. We detect it and frontrun 2 operations: 1. We deposit 1 of the underlying to the system. In exchange we receive ...

DoS on deposit(), incrementLock() and createLock() because of a safeApprove(0)

Lines of code Vulnerability details Impact Since SafeERC20 is the one implemented by OpenZeppelin, if one checks the code at this address: You will see that the safeApprove executes this validation: require value == 0 || token.allowanceaddressthis, spender == 0, "SafeERC20: approve from non-zero ...

Reentrancy attack

Lines of code Vulnerability details Impact High impact. Reentrancy Attack may lead in lost of funds. Proof of Concept Reentrancy in RubiconMarket.matchouint256,ERC20,uint256,ERC20,uint256,bool c ontracts/RubiconMarket.sol1049-1108: External calls: - buybestmakerid,minmpayamt,t...

BathPair.sol#rebalancePair() can be front run to steal the pending rebalancing amount

Lines of code Vulnerability details function underlyingBalance public view returns uint256 uint256 pool = IERC20underlyingToken.balanceOfaddressthis; return pool.addoutstandingAmount; function removeFilledTradeAmountuint256 amt external onlyPair outstandingAmount = outstandingAmount.subamt; emit...

ExtraRewardsDistributor deposits don’t work with fee-on transfer tokens

Lines of code Vulnerability details Impact 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 safetransfer or safetransferFrom . Proof of Concept The ExtraRewardsDistributor’s...

ConvexMasterChef's deposit and withdraw can be reentered drawing all reward funds from the contract if reward token allows for transfer flow control

Lines of code Vulnerability details Reward token accounting update in deposit and withdraw happens after reward transfer. If reward token allows for the control of transfer call flow or can be upgraded to allow it in the future i.e. have or can introduce the beforetokentransfer, afterTokenTransfe...

Aura.sol mint() function has a bug in the logic

Lines of code Vulnerability details Impact Intention is that during inflation period operator will or at least will be able to min EMISSIONSMAXSUPPLY token amount. Due to a logic bug amount of operator mintable tokens directly depends on how many tokens are minted by function minterMint and init...

massUpdatePools() is susceptible to DoS with block gas limit

Lines of code Vulnerability details Impact massUpdatePools is a public function and it calls the updatePool function for the length of poolInfo. Hence, it is an unbounded loop, depending on the length of poolInfo. If poolInfo.length is big enough, block gas limit may be hit. Proof of Concept Tool...

Users can grief reward distribution

Lines of code Vulnerability details Impact Users can grief reward distributions by spending dust Proof of Concept If a reward is targeted for an epoch in the past, a user can front-run the txn in the mempool and call addRewardToEpoch with a dust amount at an epoch after the one in question. This...

LidoVault: Premature return after sending ETH

Lines of code Vulnerability details Details & Impact The require check is performed after exiting the function, meaning that the ETH transfer’s validity check is skipped. This would thus cause invalid withdrawals to be erroneously processed as valid. Recommended Mitigation Steps Swap the require...

[WP-H12] Wrong implementation of ConvexCurveLPVault#withdrawOnLiquidation() may cause the positions collateralized with ConvexCurveLP unbale to be liquidated as withdrawOnLiquidation() will always fail

Lines of code Vulnerability details function withdrawuint256 amount, address to internal returns uint256 // Withdraw from Convex address baseRewardPool = getBaseRewardPool; IConvexBaseRewardPoolbaseRewardPool.withdrawAndUnwrapamount, true; // Deliver Curve LP Token...

call to harvest() can be manipulated to steal funds from the contract

Lines of code Vulnerability details Impact When calling harvest, attacker can device a cross-contract reentrancy to steal NFT owned by the contract Cally. Proof of Concept 1. While creating a vault, the owner of NFT transfers the NFT to the contract i.e. ownerOfnftID is Cally. 2. Attacker contrac...

Fee-on-transfer / rebasing ERC20 cannot be used as underlying

Lines of code Vulnerability details Some ERC20 tokens may have fee-on-transfer or change balance without owner intervention. If these tokens are used as underlying in the protocol they can be lost. Proof of concept Alice creates a vault with a token that has a 1% fee on transfer. She sends...

Inefficiency in the Dutch Auction due to lower duration

Lines of code Vulnerability details The vulnerability or bug is in the implementation of the function getDutchAuctionStrike The AUCTIONDURATION is defined as 24 hours, and consider that the dutchAuctionReserveStrike or reserveStrike will never be set to 0 by user. Now if a vault is created with...

no-revert-on-transfer ERC20 tokens can be drained

Lines of code Vulnerability details Impact Some ERC20 tokens don't throw but just return false when a transfer fails. This can be abused to trick the createVault function to initialize the vault without providing any tokens. A good example of such a token is ZRX: Etherscan code When such a vault ...

Missing check in the updateValset function

Lines of code Vulnerability details Impact The updateValset function don't check that the sum of the powers of the new validators in the new valset is greater than the threshold, which can lead to unwanted behavior. There are 2 main problems that can occur in that situation: 1. The sum of the new...

Admin has ability to rugpull all tokens

Lines of code Vulnerability details Impact Currently it is possible for the admin to pull all tokens belonging to the Gravity bridge. In normal circumstances this is probably fine, but if the admin account were compromised this would lead to the bridge being drained of locked funds. Furthermore, ...

Missing powerThreshold validation in function updateValset(in Gravity.sol) could cause the contract non-functional

Lines of code Vulnerability details Impact When the cumulative power of validators in newValset is less than or equal to statepowerThreshold, the checkValidatorSignatures function would fail. Eventually, submitBatch, submitLogicCall & updateValset would fail for the new set of validators. This wi...

it's possible that validators don't have enough power

Lines of code Vulnerability details newValset parameter in updateValset can have validators that don't have enough power combined for passing the threshold. Recomendation mitigation steps check that the validators power combined is bigger than the threshold --- The text was updated successfully,...

Not checking returned bool by transfer can lead to loss of funds

Lines of code Vulnerability details Impact Loss of funds Proof of Concept The ERC20 interface ensures a token transfer will return false on failure. In merkleVesting there is no requirement for this to be true. The contract doesn't ensure all the funds to cover the MerkleTree are present since it...

Missing a storage slot

Lines of code Vulnerability details Impact By using a pre-increment in our instantiations, we are potentially missing out on using the first 0th slot. Tools Used Manual Inspection Recommended Mitigation Steps Use post-incement --- The text was updated successfully, but these errors were...

Re-entrancy attack on the main functions

Lines of code Vulnerability details Impact A malicious token, or one that implemented transfer hooks, could re-enter the public calling function such as withdraw before proper internal accounting was completed. Because the earned reward function looks up the pool.totalDepositsWei and...

Users can not initialize and withdraw tokens if coinsPerSecond is 0

Lines of code Vulnerability details Impact If a user tries to claim a few totalCoins with a long vestingTime, this user will call the initialize function failed, and can not withdraw funds. Proof of Concept In MerkleResistor.sol L259: uint coinsPerSecond = totalCoins uint100 - tree.pctUpFront /...