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

Claiming BADGER bribes and sweeping BADGER rewards tokens is broken

Lines of code Vulnerability details Impact Twice the amount of BADGER tokens are sent to badgerTree within the MyStrategy.sendBadgerToTree function. Due to insufficient BADGER tokens, this will break the following functionalities: MyStrategy.sweepRewardToken L107-L113 function...

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

Wrong Deadline

Lines of code Vulnerability details the deadline is the timestamp after which the transaction will revert. the goal of this field is that the caller can set a deadline for the transaction so the transaction will not succeed in any arbitrary time in the future, and after this deadline, they can...

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

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

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

There is no check that in setBribesProcessor() the value of newBribesProcessor is not 0x0, fund will be lost or locked if by mistake value set to 0x0

Lines of code Vulnerability details Impact Fund can be lost if the value of bribesProcessor set to 0x0 address and there is no check in the setBribesProcessor to prevent it. sendTokenToBribesProcessor sends bribes to bribesProcessor and there is no check there too. so if by mistake the value of...

attacker can burn anyones tokens and steal everyones money

Lines of code Vulnerability details Impact attacker can burn tokens and balance of contract is an amount of less or greater in sendtokenreciver function and if receiver is me then tokentransfer can be more than i lended or borrwing,withdrawing and gain me extra tokens. burn function is called...

Did Not Enforce fCash To Be A Component Of SetToken Before Minting

Lines of code Vulnerability details Proof-of-Concept Assume that the manager decided to add a fCash position called "Wrapped fDAI @ 10 October 2022", which will mature at 10 October 2022, to the SetToken. To do so, the manager will call the NotionalTradeModule.mintFCashPosition function. The...

Round down in previewWithdraw() may result in withdrawing asset using zero share.

Lines of code Vulnerability details Impact In wfCashERC4626.previewWithdraw function, when fCash has matured, shares is calculated using convertToShares. But convertToShares function rounded down in division. This may lead to the case that user can use zero share to withdraw asset. It has been...

safeMath function being used without importing the safeMath library preventing contract compilation

Lines of code Vulnerability details Impact Contract NotionalTradeModule.sol will not compile due to an error caused by missing import of safeMath and the directive using for. Since safeMath is not imported and no using for directive, the contract would not even compile as it would throw an error ...

Upgraded Q -> H from 63 [1654488227536]

Judge has assessed an item in Issue 63 as High risk. The relevant finding follows: --- The text was updated successfully, but these errors were encountered: All reactions...

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 119 [1654475092615]

Judge has assessed an item in Issue 119 as Medium risk. The relevant finding follows: Checking whether the receiver is capable of holding ERC721 The contract usessafeTransfer for ERC20 but uses transferFrom for ERC721 in both exercise and withdraw which may lead to the loss of ERC721 if the...

_verifyProof allows empty proofs (allows malleable transactions)

Lines of code Vulnerability details Impact verifyProof allows empty proofs and in that case it expects the leaf to equal the root, because no hashing and iteration is taking place. The purpose of the tree is to hold multiple accepted tokenIds, where the consideration contains one and proving its...

Governance Token limit can be massively increased due to uninitialised lastEvent variable

Lines of code Vulnerability details Impact Due to insufficient checks in the executeInflationRateUpdate in the Minter contract, the limit on tokens that can be minted is massively increased. As lastEvent is uninitialised and equal to 0 by default, in the first line of executeInflationRateUpdate,...

Users will lose all of their money during pool migration

Lines of code Vulnerability details Impact Users will lose all of their money when they migrate by calling PoolMigrationZap.migrate Proof of Concept File: protocol/contracts/zaps/PoolMigrationZap.sol 1 52 function migrateaddress oldPoolAddress public override 53 ILiquidityPool oldPool =...

Fulfill transactions that are not protected with a deadline may lead to unfavorable trade.

Lines of code Vulnerability details Impact A fulfill transaction of order with descending/ascending amount should be protected by the deadline. The price of an order with a descnding amount is sensitive to the time. Letting users make such a trade without providing the deadline would lead to...

BurnToTarget can be exploited to receive more LP tokens

Lines of code Vulnerability details Issue: depositInPool uses addressthis.balance Consequences: exploiter will get more lp tokens File: FeeBurner.sol 81: // Depositing target underlying into target pool 82: uint256 targetLpTokenBalance = depositInPooltargetUnderlying, targetPool; 83: 84: //...

DoS of an order without fully fulfilling it

Lines of code Vulnerability details Impact A malicious attacker can DoS an order by making its n and d invalid, without fully fulfilling the order. Proof of Concept In this example the attacker fulfills only 2/3 of the order, and makes the rest of it unfulfillable. 1. The attacker calls the...

Restricted Trades Vulnerable to Stolen Items Being Traded

Lines of code Vulnerability details Impact The current contract does not check for stolen items. Traditionally, Opensea has frozen items that are stolen based on its stolen item policy. It was possible to do so on Opensea at the UI level. However, with Seaport, anyone can list a restricted trade...

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

wrong reward distribution and user fund lose if migrate() is called with current rewardToken by mistake or intentionally

Lines of code Vulnerability details Impact It's possible to call migrate function of BkdLocker with newRewardToken value equal to current rewardToken and there is no check to prevent this. and if this happens then userCheckpoint will calculated reward double times for rewardToken, one time becaus...

Lack of safeApprove(0) may prevent deposits and swapping of token

Lines of code Vulnerability details Impact OpenZeppelin’s safeApprove will revert if the account already is approved and the new safeApprove is done with a non-zero value. Users may be prevented from swapping tokens to Backd LPTokens a second time Proof of Concept There are multiple places...

Able to get LP tokens without spending any funds in FeeBurner.sol

Lines of code FeeBurner.solL43-L88 Vulnerability details Impact In the burnToTarget function, if the tokens array has only a zero address token and msg.value is 0 then FeeBurner.sol would still call the swapAll function and use all WETH held in the contract. If the contract has any WETH, the call...

RewardHandler.burnFees() could fail depending on number of pools with underlying = address(0)

Lines of code Vulnerability details Impact If more than one pool has underlying = address0 then RewardHandler.burnFees will fail or use ETH balance from FeeBurner.sol. Proof of Concept RewardHandler.solL40-L50 uint256 ethBalance = addressthis.balance; address memory tokens = new address; for...

Fulfilling an order more than once

Lines of code Vulnerability details Impact Any non basic or FULL order can be fulfilled more than once, assuming the offerer has approved the contract the right amounts. This is a valid assumption as users may max-approve the contract/conduits, or have multiple orders with the same items open so...

Buyer can take from a seller an item they had not offered

Lines of code Vulnerability details Impact Orders in Seaport can involve "criteria-based items", in which case it isn't one specific item that can fulfill the order but any one of a set of items. This set of items is the "criteria", and it is specified in the order as the merkle root of a merkle...

Does not set the offerer as the recipient if execution amount is nonzero

Lines of code Vulnerability details Impact Order structures will be corrupted through invalid fulfillment application logic. Proof of Concept Tools Used Manual inspection Recommended Mitigation Steps Change to // Set the offerer as the receipient if execution amount is nonzero. if...

balanceAtEpochOf will revert

Lines of code Vulnerability details Impact The line for uint256 i = locks.length - 1; i + 1 != 0; i-- relies on uint256 underflow and overflow, which would revert in solidity ^0.8.0 Proof of Concept function balanceAtEpochOfuint256 epoch, address user public view returns uint256 amount...

Add a tme lock to VoterProxy.sol setter function

Lines of code%20external%20%7B,%7D,-function%20deposit Vulnerability details Impact More trust for user functions that are set key/critical variables should be behind a timelock. Proof of Concept%20external%20%7B,%7D,-function%20deposit Tools Used Manuel review Recommended Mitigation Steps Add a...

_aggregateValidFulfillmentOfferItems() can be tricked to accept invalid inputs

Lines of code Vulnerability details Impact The aggregateValidFulfillmentOfferItems function aims to revert on orders with zero value or where a total consideration amount overflows. Internally this is accomplished by having a temporary variable errorBuffer, accumulating issues found, and only...

Using ExtraRewardStashV2 In Angle Deployment Will Cause Gauge Rewards To Be Frozen

Lines of code Vulnerability details Note: This report aims to discuss the issue encountered when ExtraRewardStashV2 is used within Angle Deployment. There is also another issue when ExtraRewardStashV3 is used within Angle Deployment, but I will raise it in a separate report since ExtraRewardStash...

Gauge Rewards Stuck In VoterProxy Contract When ExtraRewardStashV3 Is Used Within Angle Deployment

Lines of code Vulnerability details Note: This report aims to discuss the issue encountered when ExtraRewardStashV3 is used within Angle Deployment. There is also another issue when ExtraRewardStashV2 is used within Angle Deployment, but I will raise it in a separate report since ExtraRewardStash...

pendingLockAtEpochOf will revert

Lines of code Vulnerability details Impact The line for uint256 i = locks.length - 1; i + 1 != 0; i-- relies on uint256 underflow and overflow, which would revert in solidity ^0.8.0 Proof of Concept function pendingLockAtEpochOfuint256 epoch, address user external view returns uint256 amount...

VoterProxy.claimVeAsset() and VeAssetDepositor._lockVeAsset() both spend veAsset.balanceOf(staker) for different purposes

Lines of code Vulnerability details Impact Each of the functions VeAssetDepositor.lockVeAsset and VoterProxy.claimVeAsset will use the entire balance of the VoterProxy for different purposes. In VeAssetDepositor.lockVeAsset the balance is locked into the VoterEscrow contract and cannot be...

Upgraded Q -> M from 294 [1654058024144]

Judge has assessed an item in Issue 294 as Medium risk. The relevant finding follows: Beneficiary is credited additional ETH above premium The CallybuyOption function ensures that the caller sends an ETH amount equal to or greater than the calculated premium: buyOptionL224 requiremsg.value =...

ERC777 getReward Reentrancy

Lines of code Vulnerability details Impact Attacker can drain BaseRewardPool.sol if rewardToken is an ERC777 token. Proof of Concept Attacker calls getReward. rewardToken.safeTransfer triggers ERC777 callback and attacker take control of execution flow. Attacker reenters getReward. Attacker repea...

_cancel function should check order status

Lines of code Vulnerability details Impact In the cancel function of the OrderValidator contract, orderStatusorderHash.isValidated is not checked. This is to allow users to cancel unverified private orders. This is not a good solution. For unverified private orders, the user can verify and cancel...

Users can get unlimited votes

Lines of code Vulnerability details Impact Users can get unlimited votes which leads to them: 1. gaining control over governance 2. getting undeserved rewards 3. having their pools favored due to gauge values Proof of Concept mint calls moveTokenDelegates to set up delegation... File:...

Bribe.sol is not meant to handle fee-on-transfer tokens

Lines of code Vulnerability details Impact Should a fee-on-transfer token be added as a reward token and deposited, the tokens will be locked in the Bribe contract. Voters will be unable to withdraw their rewards. Proof of Concept Tokens are deposited into the Bribe contract using...

Not calling _safeApprove(0) before setting a new approval causes the call to revert when used with Tether (USDT)

Lines of code Vulnerability details Impact Some tokens like the very popular USDT do not work when changing the allowance from an existing non-zero allowance value it will revert if the current approval is not zero to protect against front-running changes of approvals. These tokens must first be...

notifyRewardAmount() griefing attack by adding MAX_REWARD_TOKENS

Lines of code Vulnerability details function notifyRewardAmountaddress token, uint amount external lock requireamount 0; if !isRewardtoken requirerewards.length Recommendation Consid...

GettersAndDerivers: Offer and consideration should be sorted when calculating orderHash

Lines of code Vulnerability details Impact When calculating the orderHash in the deriveOrderHash function of the GettersAndDerivers contract, the orderHash will be different due to the order of the elements in the offer and consideration, and the user is likely to get the wrong orderHash due to t...

BathToken.sol#_deposit() attacker can mint more shares with re-entrancy from hookable tokens

Lines of code Vulnerability details BathToken.soldeposit calculates the actual transferred amount by comparing the before and after balance, however, since there is no reentrancy guard on this function, there is a risk of re-entrancy attack to mint more shares. Some token standards, such as ERC77...

Multi-hop routes will leave a dust trail

Lines of code Vulnerability details Impact By calling RubiconMarket.buyid, quantity as a consequence of L239 and L241 with quantity = currentAmount - currentAmount expectedMarketFeeBPS / 10000 ...the fee calculated by the buy function amounts to: currentAmount - currentAmount expectedMarketFeeBPS...

ConvexMasterChef: safeRewardTransfer can cause loss of funds

Lines of code Vulnerability details Impact Same as code-423n4/2022-02-concur-findings244 All calculations are rounded down, since a lack of tokens in the contracts cannot be rounding errors' fault. So the function is redundant. On the other hand, if the contract is undersupplied with cvx tokens,...

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