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

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

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

GHSA-7J52-6FJP-58GR Inconsistent storage layout for ERC2771ContextUpgradeable

Impact The storage layout of the ERC2771ContextUpgradeable is not constant between versions. - versions 4.0.0, 4.1.0 and 4.2.0, the contract has a length of 51 slots. - since 4.3.0, the contract has a length of 50 slots - future versions will continue using 50 slots. This difference in layout cou...

Inconsistent storage layout for ERC2771ContextUpgradeable

Impact The storage layout of the ERC2771ContextUpgradeable is not constant between versions. - versions 4.0.0, 4.1.0 and 4.2.0, the contract has a length of 51 slots. - since 4.3.0, the contract has a length of 50 slots - future versions will continue using 50 slots. This difference in layout cou...

PT-2022-28211

Name of the Vulnerable Software and Affected Versions OpenZeppelin Upgradeable Contracts library versions 4.0.0 through 4.2.0 Description The storage layout of the ERC2771ContextUpgradeable contract is not constant between versions, which could result in breaking upgrades if someone upgrades from...

Forcing ERC20Upgradeable when calling transfer() reverts when used with some ERC20 tokens

Lines of code Vulnerability details 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 transferFrom functions do not return booleans as the specification requires, and instead have no retur...

Update initializer modifier to prevent reentrancy during initialization

Lines of code Vulnerability details Impact While Governable.sol is out of scope, I figured this issue would still be fair game. The solution uses: "@openzeppelin/contracts": "4.2.0". This dependency has a known high severity vulnerability: Which makes this contract vulnerable: File: Governable.so...

Use of IERC20.transfer() instead of SafeERC20.safeTransfer()

Lines of code Vulnerability details Impact There are multiple external calls to IERC20.transfer that suppose to transfer out tokens from the system. However, there are tokens like USDT that don't return any return value in their implementation of the transfer function, so calling IERC20.transfer ...