Protocol does not support fee-on-transfer tokens

62 comment Warden: cmichel Protocol does not support fee-on-transfer tokens, see addLiquidity and LiquidityPool.depositErc20. The amount value is stored and available to be withdrawn later but the contract receives amount - fees. Note that there are tokenCheckstokenAddress and the protocol would...

createVault() does not verify ERC20 token contract exists before transferring

Lines of code Vulnerability details Impact There are no checks to ensure the the vault.token contract exists when creating a vault. Token address are deterministic in the EVM and can be known ahead of time. As a result it is possible for a user to call createVault with token set to a ERC20 contra...

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

Unchecked return value of transferFrom() leading to zero cost RewardFunding

Lines of code Vulnerability details Impact Due to unchecked return value of transferFrom in https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.solL144 , it is possible to fund a pool during creation without transferring tokens to use as rewardToken...

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

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

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

Lines of code Vulnerability details Impact If a user calls depositTokens with wrong treeIndex value by mistake, attacker can perform front-running attack and steal user funds before user transaction reach the contract, attacker will create multiple trees in MerkleDropFactory until he reach tree...

Missing a storage slot

Lines of code Vulnerability details Impact By using a pre-increment in our instantiations, we are potentially missing out on using the first 0th slot. Tools Used Manual Inspection Recommended Mitigation Steps Use post-incement --- The text was updated successfully, but these errors were...

PermissionlessBasicPoolFactory's pools with fee on transfer tokens can be emptied by repetitive deposit-withdraws

Lines of code Vulnerability details Griefing attack is possible if pool deposit token is a fee on transfer ERC20 as deposit, withdraw atomic call sequence is allowed with pre-fee token quantity being accounted as deposit amount. Suppose F is a fee on transfer token and the pool with F as deposit...

Users may send more ETH in the passThruGate()

Lines of code Vulnerability details Impact When a user send ETH via the withdraw of MerkleIdentity, It will call IPriceGatetree.priceGateAddress.passThruGatevalue: msg.valuetree.priceIndex, msg.sender;. passThruGate checks whether msg.value = gate.ethCost holds. If a user accidentally sends more...

FixedPricePassThruGate.sol All the msg.value should be pass thru to gate.beneficiary instead of gate.ethCost

Lines of code Vulnerability details In FixedPricePassThruGate.solpassThruGate, at L48 the msg.value is checked to be = gate.ethCost instead of == gate.ethCost, which makes it possible for the caller to send more than gate.ethCost. However, at L53 only the amount of gate.ethCost is passed thru 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...

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

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

If treeAdder call addMerkleTree() of MerkleIdentity with wrong values for eligibilityIndex or priceIndex (uninitialized) attacker can steal NFTs

Lines of code Vulnerability details Impact If treeAdder call addMerkleTree of MerkleIdentity with wrong values for eligibilityIndex or priceIndex uninitialized gates index attacker can create those gate indexes in priceGateAddress or eligibilityAddress they are permission less with his own specif...