missing input validation for _liquidityPool

189 comment Warden: kenta missing input validation for liquidityPool. The owner can change always liquidityPool but this liquidityPool will be used to execute low-level calls. To avoid errors with an empty address this must be checked always. requireliquidityPool != address0, “liquidityPool canno...

EthAssetManager and ThreePoolAssetManager don't control Meta tokens decimals

Lines of code Vulnerability details Both contracts treat meta assets as if they have fixed decimals of 18. Minting logic breaks when it's not the case. However, meta tokens decimals aren't controlled. If actual meta assets have any other decimals, minting slippage control logic of both contracts...

Consider remove setLiquidityProviders function

143 comment Warden: gzeon Owner can call setLpToken to change the value of lpToken in WhitelistPeriodManager, which will make all onlyLiquidityPool function revert function setLiquidityProvidersaddress liquidityProviders internal liquidityProviders = ILiquidityProvidersliquidityProviders; functio...

Re-entance steal token

Lines of code Vulnerability details Impact An attacker can steal tokens from the protocol after it sent ETH to him. Proof of Concept 1.Alice create vault of some ERC20/NFT. and buy the option 2.Alice pay for itself, so ethBalanceAlice is greater than 1 3. Alice call harvest, and get eth. Alice...

YearnTokenAdapter allows a maximum loss of 100% when withdrawing

Lines of code Vulnerability details Impact YearnTokenAdapter allows slippage of 100% when withdrawing from the vault which will cause a loss of funds. Here's the documentation straight from the vault contract: It allows the user to specify the maxLoss as the last parameter. It determines how many...

Options can be sold for tokens that have been destructed

Lines of code Vulnerability details Unlike OpenZeppelin's safeTransfer, solmate's version of the function does not check for contract existence before its low level calls are made. The foot-gun associated with this difference is that it's up to the caller to verify that the contract exists before...

Arbitrage Bots Could Extract Value From Buyer of Call Options

Lines of code Vulnerability details Impact Arbitrage bots could monitor the Cally contract and the price of the underlying asset being auctioned for all vaults that have outstanding call options, and whenever the underlying asset price rises above the strike price of the outstanding call option,...

A malicious user can create a vault that is actually empty

Lines of code Vulnerability details description A malicious user can call the createVault function to create a vault with an ERC20 token that returns false rather than revert on failed transfer. By specifying the tokenType parameter to be ERC721 the transferFrom function will be called rather tha...

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

Calls inside loops that may address DoS.

Lines of code Vulnerability details Impact Calls inside loops that may address DoS. Proof of Concept Calls to external contracts inside a loop are dangerous especially if the loop index can be user-controlled because it could lead to DoS if one of the calls reverts or execution runs out of gas...

Gravity does not support ERC20 tokens with built-in fee

Lines of code Vulnerability details Impact Contract Gravity.sol does not properly handle ERC20 tokens that charge fee on their transfers. Implementation of such a tokens does not transfer exact amount provided to transfer but part of it is charged as a fee, burned or used in some other way. This...

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

Nft.upaidRewards reset upon withdrawal

129 comment Warden: 0xDjango Nft.upaidRewards reset upon withdrawal. This feels a bit unfair if a user means to collect rewards while withdrawing NFT. If there are not enough rewards in the contract balance, the unpaidRewards will be deleted upon withdrawal with the line delete nftInfonftId;. I'm...

Fees in TokenManager.sol:function changeFee() should be upper-bounded

193 comment Warden: Dravee --- The text was updated successfully, but these errors were encountered: All reactions...

Administrators can rug users

Lines of code Vulnerability details The Cudos Network is a special-purpose blockchain designed to provide high-performance, trustless, and permissionless cloud computing for all. To be considered trustless, both the incentives and the code must be aligned to prevent the possibility of...

Admin can rug users, withdraw all the locked funds

Lines of code Vulnerability details function withdrawERC20 address tokenAddress external requirecudosAccessControls.hasAdminRolemsg.sender, "Recipient is not an admin"; uint256 totalBalance = IERC20tokenAddress.balanceOfaddressthis; IERC20tokenAddress.safeTransfermsg.sender , totalBalance; The...

Incorrect accounting on transfer-on-fee/deflationary tokens in Gravity contract

Lines of code Vulnerability details Impact The sendToCosmos function of Gravity transfers amount of tokenContract from the sender using the function transferFrom. If the transferred token is a transfer-on-fee/deflationary token, the actually received amount could be less than amount. However, sin...

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

Lack of check that _tokenContract is a supported token in sendToCosmos can use users to lose money.

Lines of code Vulnerability details Impact Lack of check that tokenContract is a supported token in sendToCosmos can use users to lose money. Proof of Concept If a user calls sendToCosmos with a non supported token , then the transferFrom transaction in sendToCosoms would happened but the user...

Wrong amount will be passed with fee on transfer tokens

Lines of code Vulnerability details Impact Loss of 1 to 1 ratio with fee on transfer tokens Proof of Concept Some tokens like USDT have a fee on transfer that can be activated. If such a token is used then wrong amounts will be minted on the other side. As we can see in the sendToCosmos function ...

The sendToCosmos function does not check for transfer-on-fee/deflationary tokens

Lines of code Gravity.solL595-L609 Vulnerability details Impact The documentation describes the use of tokens other than CUDOS in the Gravity.sol contract; so it is assumed that the CUDOS token will not be the only supported token. The documentation also states that validators on the Tendermint...

ERC20 tokens with before/afterTokenTransfer hooks can be manipulated to cause submitBatch to fail

Lines of code Vulnerability details Impact Currently submitBatch processes a batch of Cosmos to Ethereum transactions. As part of this method ERC20 tokens are transferred to the desired destination from the Gravity bridge, as approved by the current validator set. For ERC20 tokens without a...

Centralisation Risk: Admin can withdraw all ERC20 held in the bridge and all deployed CosmosERC20 tokens

Lines of code Vulnerability details Impact The function withdrawERC20 allows an admin to withdraw any ERC20 tokens help in the bridge. The impact of this is significant as the function deployERC20 will create a new CosmosERC20 token with MAXUINT256 supply minted to the Gravity smart contract. The...

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

Consistently check account balance before and after transfers for Fee-On-Transfer discrepancies

Lines of code Vulnerability details Impact Wrong amount emitted in SendToCosmosEvent event Proof of Concept File: Gravity.sol 595: function sendToCosmos 596: address tokenContract, 597: bytes32 destination, 598: uint256 amount 599: public nonReentrant 600:...

sendToCosmos doesn't consider the ERC20 transfer fee, resulting in incorrect amount of ERC20 token

Lines of code Vulnerability details Impact Some ERC tokens have a fee on each transfer. The protocol doesn’t handle the fee when transferring this kind of ERC20 tokens, leading to the inconsistent amount of token actually received in the contract. Validators on the Cudos will mint more tokens to...

Tokens with fee on transfer are not supported

Lines of code Vulnerability details There are ERC20 tokens that charge fee for every transfer or transferFrom. In the current implementation, sendToCosmos assumes that the received amount is the same as the transfer amount, and uses it to emit SendToCosmosEvent event. As a result, when bridging t...

updateValset() Insufficient validation of new validator set may brick the contract

Lines of code Vulnerability details In Gravity.solupdateValset, while the signatures of the current validators are verified and = powerThreshold is checked, there is one important validation should be done: check the cumulative power of the new validator set to ensure the contract has sufficient...

Attackers can prevent the transfer of the highest-value Cosmos to Ethereum transactions

Lines of code Vulnerability details In order to ensure that profitable batches are eventually created we must avoid locking up the high fee 'good transactions' into obviously bad batches. To add to the difficulty we don't actually know what any token in this process is worth or what ETH gas costs...

The Gravity.sol should have pause/unpause functionality

Lines of code Vulnerability details Impact In case a hack is occuring or an exploit is discovered, the team or validators in this case should be able to pause functionality until the necessary changes are made to the system. Additionally, the gravity.sol contract should be manged by proxy so that...

in depositErc20 - no check that token address != NATIVE

103 comment Warden: CertoraInc The depositErc20 doesn't have any check that the given token address doesn't equal to the NATIVE address in this case the user should use the depositNative function. This is a needed check, it can be seen also in the withdrawErc20GasFee, so it needs to be applied in...

Missing input validation for upper bound of values in TokenManager.sol functions

165 comment Warden: hubble Details Issue3 Title : Missing input validation for upper bound of values in TokenManager.sol functions Impact The below two state variables are defined as uint256, but they should never be more than 100% or a defined max value. uint256 equilibriumFee, uint256 maxFee...

Batch Transfer will likely fail on ERC777 transfers, either maliciously or accidentally

Lines of code Vulnerability details Impact If users attempt to bridge ERC777s from Cosmos to Ethereum, it is highly likely that the transfers will revert. The issue lies in the combination of using safeTransfer and the idea of batching. safeTransfer will revert for any failed transfer meaning tha...

Voting power of new validators not checked in updateValset function, Gravity.sol

Lines of code Gravity.solL276-L358 Vulnerability details Impact While the voting power of the current valset is checked when the checkValidatorSignatures function is called in updateValset, the power of the new valset is not. This could cause some functions to not work since whenever...

Rugpull vector : a single admin address can withdraw all funds

Lines of code Vulnerability details Impact Someone with access to admin keys could rug pull all funds Proof of Concept The gravity.sol contract should work as an escrow to mint equivalent tokens in the cosmos chain. This is maintained by a system of validators. The possible decentralization of th...

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

No cumulative power check when updating valset

Lines of code Vulnerability details Impact When batches are submitted by validators in the current valset, they are checked for validity based on signatures and cumulative powers. Each validator in the valset has an associated power which can give certain validators more voting power than others...

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

ERC20 transfers does not work on non-standard compliant tokens like USDT

Lines of code Vulnerability details Impact PermissionlessBasicPoolFactory.sol Consider this function: function deposituint poolId, uint amount external Pool storage pool = poolspoolId; requirepool.id == poolId, 'Uninitialized pool'; requireblock.timestamp pool.startTime, 'Cannot deposit before po...

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

Bogus deposits in Pools possible

Lines of code Vulnerability details Impact Bogus deposits in the Pools is possible by calling the function deposit with a malicious ERC20 token that always returns true whenever transferFrom is called. Tools Used Manual Inspection Recommended Mitigation Steps Create a whitelist of allowed ERC20...

Multiple vestings for the same user will fail

Lines of code Vulnerability details Impact Loss of funds from multiple vestings for a single user Proof of Concept In MerkleVesting and MerkleResistor vestings are distributed using merkle trees. Creators of the vesting submit the Merkle root of the tree and deposit the funds to be distributed. A...

steal user funds with front-running when he calls depositTokens() of MerkleVesting and MerkleResistor with wrong treeIndex (uninitiated)

Lines of code Vulnerability details Impact This nature of this bug is similar in MerkleVesting and MerkleResistor and MerkleDropFactory, so I only write MerkleDropFactory version: If a user calls depositTokens with wrong treeIndex value by mistake, attacker can perform front-running attack and...

If A User Mistakenly Provides Too Much Ether To The passThruGate() Function, This Additional Amount Will Be Forever Locked Within The Contract

Lines of code Vulnerability details Impact The passThruGate function acts as a proxy function to the beneficiary address by attaching Ether to the call. If an excess of Ether is provided to the call, only gate.ethCost will be sent to the beneficiary. Excess Ether will be forever be locked in the...

Creator of the contract could front run tax per capita to 100% of Yield rewards

Lines of code Vulnerability details Impact Contract creator could steal all rewards using frontrunning Proof of Concept When a yield pool is created pool tax is set equal to global tax and funds are sent into the contract to pay for rewards. The contract creator could set tax to 100% in a...

Fee-On-Transfer Tokens Are Not Supported

Lines of code Vulnerability details Impact The FactoryDAO suite of contracts interact with any arbitrary ERC20 token. Because of this, there is a specific instance and likely several others where a fee-on-transfer token will not be correctly handled. PermissionlessBasicPoolFactory.fundPool will...

transfer is used for transfering ether

Originally submitted by warden pauliax in 173, duplicate of 52. .transfer is used for transfering ether, e.g.: payableto.transferamount; payablemsg.sender.transferamount; It is currently not recommended as recipients with custom fallback functions smart contracts will not be able to handle that...

Fund loss in passThruGate() of FixedPricePassThruGate becasue only some portion of user payed amount has been used and the rest of it don't returned to user

Lines of code Vulnerability details Impact If user pay extra ether for minting NFT, then those extra ethers will be locked in FixedPricePassThruGate forever. because passThruGate of FixedPricePassThruGate transfer only NFT cost to gate.beneficiary and don't return extra amount in msg.value to buy...

if user send uninitialized poolId to function deposit() of PermissionlessBasicPoolFactory, then attacker can cause user fund to be locked forever, and only unlock it if user pays ransom

Lines of code Vulnerability details Impact Function deposit of PermissionlessBasicPoolFactory supposed to revert if user send uninitialized poolId by mistake, but if user does this, attacker can perform front-running attack and create multiple pools with his smart contract and be owner of that...

Reward Token Transfer Failure Can Lead to Loss of Deposit in PermissionlessBasicPoolFactory

Lines of code Vulnerability details Impact If transfer of any reward token returns False or reverts for whatever reason, users who deposited will not be able to withdraw their deposit. A malicious pool creator could abuse this to lock tokens from victims by using two reward tokens, one...