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

Executors need to be trusted

62 comment Warden: cmichel Executors need to be trusted, otherwise, they can just call sendFundsToUser to transfer out any funds as they please. Consider adding threshold signatures to prevent all funds from being lost when a single executor is compromised. --- The text was updated successfully,...

no-revert-on-transfer ERC20 tokens can be drained

Lines of code Vulnerability details Impact Some ERC20 tokens don't throw but just return false when a transfer fails. This can be abused to trick the createVault function to initialize the vault without providing any tokens. A good example of such a token is ZRX: Etherscan code When such a vault ...

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

PermissionlessBasicPoolFactory.sol Does Not Support Reward Tokens With Decimals Other Than 18

Lines of code Vulnerability details Impact The PermissionlessBasicPoolFactory.sol contract allows anyone to add staking pools which users can participate in to earn reward tokens. Pools are segregated to ensure malicious pools cannot siphon tokens from honest pools. Upon the addition of a new poo...

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

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

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

if user call addMerkleTree() of MerkleIdentity with priceIndex==0 by mistake or other uninitialized gates, then attacker can steal all the NFTs with 0 payment

Lines of code Vulnerability details Impact If treeAdder calls addMerkleTree with priceIndex==0 by mistake, then attackers can buy NFT's with price of 0 because gate Indexs in FixedPricePassThruGate or SpeedBumpPriceGate start from 1 and gates0 will be uninitialized and getCost0 will return 0. Thi...

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

setGlobalTax() Can Be Manipulated By The Global Beneficiary To Steal Reward Tokens Or Censor Pool Creators

Lines of code Vulnerability details Impact Upon pool creation, the pool configures a taxPerCapita value which is controlled by the global beneficiary. This global beneficiary account can effectively sandwich calls to addPool by increasing the fee to 100% before the addition of a new pool and...

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

Pool Creators Can Reject Taxes From Being Withdrawn

Lines of code Vulnerability details Impact Upon pool creation, stakers can opt to earn rewards by staking their deposit tokens. When these deposit tokens are withdrawn, reward tokens are sent to the owner of the receipt along with their deposit tokens. A tax is charged on any rewards accrued by t...

Users will pay more than required for NFT Minting

Lines of code Vulnerability details Impact NFTs should be sold for their best price without descending into a gas race: Due to SpeedBumpPriceGate.sol function passThruGate code users will pay more than required for NFT Minting. User will pay msg.value and not the NFT "price". It is very likely th...

SpeedBumpPriceGate.sol and FixedPricePassThruGate.sol should check whether gate.beneficiary is address(0)

Lines of code Vulnerability details Impact In SpeedBumpPriceGate\addGate and FixedPricePassThruGate\addGate, it doesn’t check whether gate.beneficiary is address0. Therefore, when doing passThruGate. ETH will be sent to address0. These ETH can never be taken back. Proof of Concept beneficiary can...