Merkle Tree criteria can be resolved by wrong tokenIDs

Lines of code Vulnerability details Impact The protocol allows specifying several tokenIds to accept for a single offer. A merkle tree is created out of these tokenIds and the root is stored as the identifierOrCriteria for the item. The fulfiller then submits the actual tokenId and a proof that...

Non standard token transfers will fail in the protocol

Lines of code Vulnerability details Impact In TokenTransferrer.sol the performERC20Transfer function handles the transfer of ERC20 tokens in the protocol. It uses the ERC20transferFromsignature constant inherited from TokenTransferrerConstants.sol. The problem is that tokens that don’t correctly...

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

No Storage Gap for Upgradeable Contract Might Lead to Storage Slot Collision

Lines of code Vulnerability details Impact For upgradeable contracts, there must be storage gap to "allow developers to freely add new state variables in the future without compromising the storage compatibility with existing deployments" quote OpenZeppelin. Otherwise it may be very difficult to...

Avoid payable.transfer

Originally submitted by warden horsefacts in 199, duplicate of 52. Avoid payable.transfer EthPool and EthVault both use payableaddress.transfer to transfer ETH. It's considered a best practice to avoid this pattern for ETH transfers, since it forwards a fixed amount of gas and may revert if futur...

Protocol unusable for some ERC20 tokens (bad tokens)

Lines of code Vulnerability details Impact Protocol unusable for some ERC20 tokens bad tokens PoC Some tokens don't correctly implement the EIP20 standard and their transfer/transferFrom return void The so called bad tokens. More info This makes the transaction revert when calling. Recommended Us...

In MerkleVesting.sol::withdraw check return value of ERC20 transfer or use safeTransfer of OZ

Lines of code Vulnerability details Impact MerkleVesting.solL173 tree.tokenBalance -= currentWithdrawal; IERC20tree.tokenAddress.transferdestination, currentWithdrawal; In case of failed transfer here it do not check return value of transfer. it updates the tree balance without transfering the...

Transfers Will Revert On Non-Standard ERC20 Token Contracts

Lines of code Vulnerability details Impact There are several contracts which do not utilise OpenZeppelin's SafeERC20 library when performing token transfers. The FactoryDAO suite of smart contracts intends to support transfers by all tokens, including non-standard tokens such as USDT which does n...

Deprecated safeApprove() function

Originally submitted by warden Dravee in 146, duplicate of 178 related to the use of safeApprove. This is upgraded from a QA report to standalone issue because it correctly described the revert when trying to call safeApprove on non-zero allowance. QA report that only describe safeApprove as...

Unhandled return value of ERC20 transfer

Lines of code Vulnerability details Impact For reference, see similar Medium-severity finding from Consensys Diligence Audit of Aave Protocol V2: As stated in the above finding: “ERC20 implementations are not always consistent. Some implementations of transfer and transferFrom could return ‘false...

ERC20 tokens with no return value will fail to transfer

Lines of code Vulnerability details Although the ERC20 standard suggests that a transfer should return true on success, many tokens are non-compliant in this regard including high profile, like USDT . In that case, the .transfer call here will revert even if the transfer is successful, because...

Missing Re-entrancy Guard

Judge @GalloDaSballo has assessed item C4-009 in QA Report 198 as Medium risk. The relevant finding follows: … Impact - Non-Critical Consider using ReentrancyGuard to protect functions that have external calls and do not follow Checks Effects Interactions pattern. An example of a function that...

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

Update initializer modifier to prevent reentrancy during initialization

Lines of code Vulnerability details Impact The solution uses: "@openzeppelin/contracts": "^4.0.0", "@openzeppelin/contracts-upgradeable": "^4.3.2", These dependencies have a known high severity vulnerability: Which makes these contracts vulnerable: contracts/helpers/CryptoPunksHelper.sol: 19:...

Collection Owners Can Reinitialize CoreCollection.sol and Mint Duplicate NFTs

Lines of code Vulnerability details Impact The initialize function is called by CoreFactory.sol when creating projects or adding collections to an existing project. When ownership of the CoreCollection.sol contract is transferred to the project owner, it gives the owner access to a subset of...

approve and safeApprove Should Approve the Zero Amount First

Lines of code Vulnerability details Impact The PooledCreditLine.sol and LenderPool.sol contracts approve the strategy contract on the collateral or borrow assets before depositing funds. This allows the strategy contracts to transfer on behalf of these contracts, pulling assets out from the pool...

Constructor cannot be used in upgradeable contracts

Lines of code Vulnerability details Details As per OpenZeppelin’s documentation, “Due to a requirement of the proxy-based upgradeability system, no constructors can be used in upgradeable contracts.” reason being “the code within a logic contract’s constructor will never be executed in the contex...

[WP-H0] Wrong implementation of EIP712MetaTransaction

Lines of code Vulnerability details 1. EIP712MetaTransaction is a utils contract that intended to be inherited by concrete actual contracts, therefore. it's initializer function should not use the initializer modifier, instead, it should use onlyInitializing modifier. See the implementation of...

[WP-M3] OperateProxy.callFunction() should check if the callee is a contract

Lines of code Vulnerability details /// @notice Allows a sender/signer to make external calls to any other contract. /// @dev A separate OperateProxy contract is used to make the external calls so /// that the Controller, which holds funds and has special privileges in the Quant /// Protocol, is...

@hifi/amm (>=1.1.2 <=1.4.0), @hifi/flash-swap (>=1.0.1 <=1.3.1) +6 more potentially affected by unknown CVE via @openzeppelin/contracts-upgradeable (=4.2.0)

@openzeppelin/contracts-upgradeable NPM version =4.2.0 is affected by a known vulnerability. The following packages have a transitive dependency on @openzeppelin/contracts-upgradeable and may be impacted: - @hifi/amm =1.1.2, =1.0.1, =1.1.2, =1.0.0, =0.8.1-pr-brioux-1333.92b26c3a.36, =1.0.5, =2.3....