Upgraded Q -> H from 135 [1654503748167]

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

Upgraded Q -> M from 135 [1654503715861]

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

Upgraded Q -> M from 190 [1654475279061]

Judge has assessed an item in Issue 190 as Medium risk. The relevant finding follows: L-02 Fee in Cally.setFee should be upper-bounded A malicious owner or owner account compromise can set the fee very high, up to 100%, siphoning all profits from the protocol. Consider implementing a reasonable...

Upgraded Q -> H from 69 [1654495749150]

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

Upgraded Q -> M from 98 [1654475216526]

Judge has assessed an item in Issue 98 as Medium risk. The relevant finding follows: Set Limits on setFee A Malicious owner could set feeRate to = 100 1e18 / 100; which would give the entire value of an exercise transaction to the protocol, create a limit on the fees the owner can set. --- The te...

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

Upgraded Q -> M from 37 [1654474354289]

Judge has assessed an item in Issue 37 as Medium risk. The relevant finding follows: Missing sanity check in setFeeRate There is no input validation in setFeeRate. A faulty payload could set the feeRate to a very high amount, which would cause problems when options are exercised: Loss of fund for...

Upgraded Q -> M from 268 [1654474507101]

Judge has assessed an item in Issue 268 as Medium risk. The relevant finding follows: High feeRate can break core protocol function PROBLEM There is no maximum input value on setFee in Cally.sol. But if the owner sets it to a uint greater than 1e18, the users will not be able to call exercice as...

Upgraded Q -> M from 41 [1654474648312]

Judge has assessed an item in Issue 41 as Medium risk. The relevant finding follows: L02: Lack of upper bound for feeRate Line References Description Fees can be set above 1e18, preventing options from being exercised. Recommended Mitigation Steps Consider having a hard cap of x% 100%. // Eg. cap...

Upgraded Q -> M from 103 [1654497041670]

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

Upgraded Q -> H from 103 [1654497033247]

Judge has assessed an item in Issue 103 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 135 [1654503723748]

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

Upgraded Q -> M from 131 [1654475029272]

Judge has assessed an item in Issue 131 as Medium risk. The relevant finding follows: Recommend using safeTransferFrom instead of transferFrom for NFTs The EIP-721 standard states: /// @notice Transfer ownership of an NFT -- THE CALLER IS RESPONSIBLE /// TO CONFIRM THAT to IS CAPABLE OF RECEIVING...

Upgraded Q -> M from 174 [1654475154140]

Judge has assessed an item in Issue 174 as Medium risk. The relevant finding follows: feeRate Can Be Set to 100% Permalinks Description There is no maximum limit on how high the feeRate can be, which might result in a fee rate of 100%, meaning the protocol will collect every ether sent in...

Upgraded Q -> M from 286 [1654474962848]

Judge has assessed an item in Issue 286 as Medium risk. The relevant finding follows: No min/max fee rate Line Refrences Cally.solL119-121 Description Limits for the fee rate should be set to avoid mistakes when setting the fee. A fee rate that is over 100% could result in the exercise function n...

Upgraded Q -> M from 95 [1654474439349]

Judge has assessed an item in Issue 95 as Medium risk. The relevant finding follows: Incompatability with deflationary / fee-on-transfer tokens Function Cally.createVault function takes a tokenIdOrAmount parameter but this parameter is not the actual transferred amount for fee-on-transfer /...

Upgraded Q -> M from 74 [1654474884998]

Judge has assessed an item in Issue 74 as Medium risk. The relevant finding follows: Protocol does not support fee-on-transfer tokens The tokenIdOrAmount established in createVault prevents buyers from exercise their option because addressthis holds less than tokenIdOrAmount due to the transfer...

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

Upgraded Q -> M from 104 [1654442859269]

Judge has assessed an item in Issue 104 as Medium risk. The relevant finding follows: Check transfer receiver is not 0 to avoid burned money Transferring tokens to the zero address is usually prohibited to accidentally avoid "burning" tokens by sending them to an unrecoverable zero address. Code...

Upgraded Q -> M from 114 [1654442763376]

Judge has assessed an item in Issue 114 as Medium risk. The relevant finding follows: PROBLEM All external functions that can be called by users should have comments SEVERITY Non-Critical PROOF OF CONCEPT Instances include: SuperVaultFactory.sol SuperVaultFactory.sol:23: function clonebytes...

Upgraded Q -> M from 137 [1654449389324]

Judge has assessed an item in Issue 137 as Medium risk. The relevant finding follows: It does not even check the return value and a low-level call succeeds if the address is empty or non-existent. dexMapping is a manually operated config so it may not contain info for all collateral tokens, and i...

Upgraded Q -> M from 104 [1654443194486]

Judge has assessed an item in Issue 104 as Medium risk. The relevant finding follows: transfer return value of a general ERC20 is ignored 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...

Upgraded Q -> M from 87 [1654442977298]

Judge has assessed an item in Issue 87 as Medium risk. The relevant finding follows: Issue 3 Low - Some tokens do not allow for Non-Zero to Non-Zero value approvals USDT, for example, will not allow approving a non-zero amount and then approving another non-zero amount. The approval must be reset...

Upgraded Q -> M from 124 [1654443300023]

Judge has assessed an item in Issue 124 as Medium risk. The relevant finding follows: C4-007 : USE SAFEERC20.SAFEAPPROVE Impact This is probably an oversight since SafeERC20 was imported and safeTransfer was used for ERC20 token transfers. Nevertheless, note that approve will fail for certain tok...

Proof for NFT with token id = 0 won't be checked

Lines of code Vulnerability details Impact The proof criteria for NFTs with a zero token id won't be checked, because of a zero token id represents an item with no criteria. Proof of Concept This if check skips the check for the proof of NFT with token id = 0. if identifierOrCriteria != uint2560 ...

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

_transfer#Executor.sol not checking all enum values

Lines of code Vulnerability details Impact Transaction can revert or have unexpected behaviour Poc In transferexecutor.sol you are checking Itemtype values Itemtype.NATIVE, Itemtype.ERC20 and Itemtype.ERC721 after then the last else clause assumes that the Itemtype is a ERC1155. else @audit assum...

THE FIRST AMM STAKER WILL HAVE CONTROL OVER HOW THE SHARES ARE CALCULATED

Lines of code Vulnerability details Impact The first staker can take control of how the subsequent shares are going to be distributed by simply staking 1wei amount of the token and frontrunning future stakers. The reasons of this are related on how the variables are updated and with the amounts...

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

Any user can start inflation for Minter.sol

Lines of code Vulnerability details Submitting as a high risk bug because it is equivalent to calling startInflation which is a function reserved for only governance to call and it drastically alters token distribution Impact Bypass governance only call restriction and cause immediate inflation...

QA Report

Reinitalisation in BkdLocker.sol In the initialize function, the only check is made that the current startBoost value is zero, if zero is passed then this allows the function to be reinitialised and potentially modifying other variables which should remain constant. Consider making a check that t...

Inconsistency in view functions can lead to users believing they’re due for more BKD rewards

Lines of code Vulnerability details Impact The view functions used for a user to check their claimable rewards vary in their implementation. This can cause users to believe they are due X amount but will receive Y. Proof of Concept If the inflationRecipient is set, then poolStakedIntegral will be...

uint120 overflow for partially fillable orders in OrderValidator.sol

Lines of code Vulnerability details Repo commit referenced: 49799ce156d979132c9924a739ae45a38b39ecdd Impact In the lines OrderValidator.solL223-L239 where the orderStatus for an orderHash gets updated: orderStatusorderHash.numerator = uint120 filledNumerator + numerator ;...

Wrong items length assertion in basic order

Lines of code Vulnerability details When fulfilling a basic order we need to assert that the parameter totalOriginalAdditionalRecipients is less or equal than the length of additionalRecipients written in calldata. However in prepareBasicFulfillmentFromCalldata this assertion is incorrect L346: /...

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

StakerVault.unstake(), StakerVault.unstakeFor() would revert with a uint underflow error of StakerVault.strategiesTotalStaked, StakerVault._poolTotalStaked.

Lines of code Vulnerability details Impact StakerVault.unstake, StakerVault.unstakeFor would revert with a uint underflow error of StakerVault.strategiesTotalStaked, StakerVault.poolTotalStaked. Proof of Concept Currently it saves totalStaked for strategies and non-strategies separately. uint...

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

Minter.sol#_executeInflationRateUpdate() inflationManager().checkpointAllGauges() is called after InflationRate is updated, causing users to lose rewards

Lines of code Vulnerability details When Minter.solexecuteInflationRateUpdate is called, if an INFLATIONDECAYPERIOD has past since lastInflationDecay, it will update the InflationRate for all of the gauges. However, in the current implementation, the rates will be updated first, followed by the...

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

A malicious zone operator can piggyback other transactions, when a caller tries to fulfill the restricted order

Lines of code Vulnerability details If an order has a restricted order type and, if the caller is other than the offerer or zone, then a staticcall to isValidOrder on the zone is made. This call outside the Seaport contract opens up a possible misuse by the zone contract implementation. The zone...

Can overfill orders

Lines of code Vulnerability details Impact There's a bug in the partial order filling that allows an attacker to overfill any order fill more than the total size of the order. This bug happens because the fractions are stored as uint120s in orderStatus but intermediate computations to scale the...

[WP-H1] OrderFulfiller.sol#_applyFractionsAndTransferEach() Orders with offerItem.itemType == ItemType.NATIVE are not processed properly

Lines of code Vulnerability details // Reduce available value if offer spent ETH or a native token. if offerItem.itemType == ItemType.NATIVE // Ensure that sufficient native tokens are still available. if amount etherRemaining revert InsufficientEtherSupplied; // Skip underflow check as a...

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

A malicious filler can fill a partial order in such a way that the rest cannot be filled by anyone

Lines of code Vulnerability details Impact For filling a partial order, because Seaport accepts any representation of a fraction, a malicious filler can provide a giant fraction unreduced. This would severely limit what orders can be filled afterwards and in some cases making any more fills...

Upgraded G -> H from 10 [1654286526568]

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

call() should be used instead or transfer() on an address payable

Lines of code Vulnerability details This is a classic Code4rena issue: code-423n4/2021-04-meebits-findings2 code-423n4/2021-10-tally-findings20 code-423n4/2022-01-openleverage-findings75 Impact The use of the deprecated transfer function for an address will inevitably make the transaction fail...

Vote locking should block wrapper contracts

Lines of code Vulnerability details The reason that users are given boosted rewards for locking their governance tokens is that by making them illiquid for a set amount of time, the supply available to be sold is restricted, and users buying the token are more able to push the price up. Impact By...

There are multiple ways for admins/governance to rug users

Lines of code Vulnerability details Impact A malicious admin can steal user funds or lock their balances forever Even if the user is benevolent the fact that there is a rug vector available may negatively impact the protocol's reputation. Proof of Concept Unlike the original Convex code that goes...