how to protect for weird erc20 tokens

Handle gpersoon Vulnerability details Impact The protocol only works as expected when the ERC20 tokens are well behaved. If rebasing tokens or malicious token contracts are used then unexpected results can happen. I don't see any code to restrict the tokens or verify the tokens. Proof of Concept...

addLiquidity transfers tokens from the wrong sender

Handle pauliax Vulnerability details Impact In addLiquidity function, a router is passed as a sender in LibERC20.transferFrom, not msg.sender, so it basically transfers assets from the router to the contract. Recommended Mitigation Steps requireLibERC20.transferFromassetId, msg.sender, addressthi...

Approval is not reset if the call to IFulfillHelper fails

Handle pauliax Vulnerability details Impact Function fulfill first approves the callTo to transfer an amount of toSend tokens and tries to call IFulfillHelper but if the call fails it transfers these assets directly. However, in such case the approval is not reset so a malicous callTo can pull...

activeTransactionBlocks are vulnerable to DDoS attacks

Handle pauliax Vulnerability details Impact There is a potential issue in function removeUserActiveBlocks and the for loop inside it. I assume you are aware of block gas limits they may be less relevant on other chains but still needs to be accounted for, so as there is no limit for...

Loss of funds from contract

Handle 0xsanson Vulnerability details Impact User and router can collude to exploit the contract and steal funds any erc20 tokens in the following way. They can set the callTo to an exploit contract that can call the receivingAssetId.transferFrom function. The logic in the fulfill function approv...

Router liquidity on receiving chain can be double-dipped by the user

Handle 0xRajeev Vulnerability details Impact During fulfill on the receiving chain, if the user has set up an external contract at txData.callTo, the catch blocks for both IFulfillHelper.addFunds and IFulfillHelper.excute perform transferAsset to the predetermined fallback address...

Logic error in fulfill() function

Handle s1m0 Vulnerability details The fulfill function could create a loss for the protocol in case addFunds succeed and execute function fails. Impact In the fulfill function the toSend amount could be transferred twice: 1. In the external call addFunds to the txData.callTo. 2. If the execute...

The fallback receiver address could get twice the toSend amount

Handle s1m0 Vulnerability details In that block of code there are 2 external call inside a try/catch statements. In both the catch the toSend amount is transferred to the fallback receiver address effectively transferring twice if the 2 external call fail. Impact In the fulfill function the...

Anyone can arbitrarily add router liquidity

Handle 0xRajeev Vulnerability details Impact The addLiquidity function takes a router address parameter whose liquidity is increased instead of assuming that router == msg.sender like done on removeLiquidity on this contract/chain by transferring the fund amount from router address to this contra...

Expired transfers will lock user funds on the sending chain

Handle 0xRajeev Vulnerability details Impact The cancelling relayer is being paid in receivingAssetId on the sendingChain instead of in sendingAssetID. If the user relies on a relayer to cancel transactions and that receivingAssetId asset does not exist on the sending chain assuming only...

Funds are sent twice on callTo errors

Handle cmichel Vulnerability details Vulnerability Details The fulfill transaction on the receiving chain tries to call the addFunds and execute actions on txData.callTo. When any of the calls reverts, the funds are sent to the txData.receivingAddress. The txData.callTo is user-controlled and an...

Variables maxIndex and minIndex in sortVaultsByDelta are uninitialized

Handle shw Vulnerability details Impact The sortVaultsByDelta function of Exposure does not properly initialize the maxIndex and minIndex variables. Consider an edge case where the delta of the three stable coins are all 0. The maxIndex and minIndex variables will be all 0 and vaultIndexes will b...

Use of deprecated Chainlink function latestAnswer

Handle shw Vulnerability details Impact According to Chainlink's documentation, the latestAnswer function is deprecated. This function does not error if no answer has been reached but returns 0, causing an incorrect price fed to the Buoy3Pool. Proof of Concept Referenced code: Buoy3Pool.solL207...

User prepare can be denied

Handle cmichel Vulnerability details Vulnerability Details Upon observing a prepare transaction, an attacker can frontrun it with the same invariantData but an amount of a single wei. This inserts a value into variantTransactionDatadigest and the original transcation will fail because of the...

addLiquidity can be denied

Handle cmichel Vulnerability details Vulnerability Details The addLiquidity function can be called by anyone to transfer funds from the router address specified as a function argument. These funds must be approved first by the router prior to calling this function. There are different griefing...

safetyCheck does not ensure all ratios of prices are healthy

Handle shw Vulnerability details Impact The safetyCheck function of Buoy3Pool checks the two ratios, a/b and a/c, to be healthy but not the ratio b/c. This ratio may be unhealthy, causing assets USDC, USDT to be exchanged at a not-so-good price. Proof of Concept Consider the following situation: ...

TransactionIDs may not be reused by user

Handle cmichel Vulnerability details Vulnerability Details The fulfill signature is only on txData.transactionId, relayerFee which allows the router to steal user funds for cross-chain transfers that go to the same router and use the same transaction ID as an earlier transfer. Example: a successf...

Funds can be stolen because of approval + send

Handle cmichel Vulnerability details Vulnerability Details The fulfill transaction on the receiving chain first approves the txData.callTo contract with the toSend amount. It then tries to call the addFunds and execute actions on txData.callTo. When any of the calls reverts, the funds are sent to...

Malicious router can block cross-chain-transfers

Handle cmichel Vulnerability details Vulnerability Details The agreement between the user and the router seems to already happen off-chain because all the fields are required for the initial InvariantTransactionData call already. A router could pretend to take on a user's cross-chain transfer, th...

grief a user by not allowing him to retrieve funds

Handle gpersoon Vulnerability details Impact The function removeUserActiveBlocks contains a "for" loop, which depends on the size of the array activeTransactionBlocks. If the array is too large then the for loop will take so much gas that the transaction will revert. The function fulfill, which...

receive funds 2x

Handle gpersoon Vulnerability details Impact When the function fulfill tries to call the functions of a receiving contract callTo and toSend 0: it first calls addFunds and if that fails that it transfers the funds via transferAsset it secondly calls execute and if that fails that it transfers the...

receive and retrieve funds

Handle gpersoon Vulnerability details Impact When the function fulfill tries send ERC20 tokens to a receiving contract, it approve the callTo address to be able to retrieve the tokens. Later on on line 414 it calls the function "execute" of callTo, where this contract is supposed to retrieve the...

Use of deprecated Chainlink API

Handle 0xRajeev Vulnerability details Impact The contracts use Chainlink’s deprecated API latestAnswer. Such functions might suddenly stop working if Chainlink stopped supporting deprecated APIs. Impact: Deprecated API stops working. Prices cannot be obtained. Protocol stops and contracts have to...

Incorrect use of operator leads to arbitrary minting of GVT tokens

Handle 0xRajeev Vulnerability details Impact The distributeStrategyGainLoss function distributes any gains or losses generated from a harvest and is expected to be called only by valid protocol vault adaptors. It is an externally visible function and the access control is indirectly enforced on...

Exposure.sortVaultsByDelta can underflow

Handle cmichel Vulnerability details Vulnerability Details The sortVaultsByDelta function performs an unsafe subtraction on two uint256 before casting them to int256. The subtraction can underflow and the cast to int256 can either fail and revert the transaction if greater than typeint256.max, or...

Flash loan risk mitigation is optional and not robust enough

Handle 0xRajeev Vulnerability details Impact The switchEoaOnly allows the owner to disable preventSmartContracts the project’s plan apparently is to do so after the beta-period which will allow any smart contract to interact with the protocol and potentially exploit any underlying flash loan...

Safe addresses can only be added but not removed

Handle 0xRajeev Vulnerability details Impact The addSafeAddress takes an address and adds it to a “safe list". This is used in eoaOnly to give exemption to safe addresses that are trusted smart contracts, when all other smart contacts are prevented from protocol interaction. The stated purpose is...

index check should use AND condition, not OR

Handle pauliax Vulnerability details Impact The condition should be AND, not OR and err msg looks weird here: function distributeStrategyGainLossuint256 gain, uint256 loss external override uint256 index = vaultIndexesmsg.sender; requireindex 0 || index = NCOINS + 1, "!VaultAdaptor"; Now basicall...

LifeGuard3Pool.investSingle can be sandwich attacked

Handle cmichel Vulnerability details Vulnerability Details The investSingle function calls exchange which trades in the curve pool without any min return amount / slippage checks. crv3pool.exchangein, out, amount, 0; Impact Certain large deposits could potentially be susceptible to a sandwich...

No way of removing an address from safeAddresses

Handle pauliax Vulnerability details Impact An address can be whitelisted in safeAddresses but this cannot be undone later in case e.g. address becomes malicious. Recommended Mitigation Steps Add a function to remove an address from safeAddresses. --- The text was updated successfully, but these...

Early user can break minting

Handle cmichel Vulnerability details Vulnerability Details The protocol computes a factor when minting and burning tokens which is the exchange rate of rebase to base tokens base supply / total assets value, see GToken.factor. The first user can manipulate this factor such that it always returns ...

For chainlink oracle, use latestRoundData instead of latestAnswer to run more validations

Handle adelamo Vulnerability details Impact I would recommend using latestRoundData instead of latestAnswer. This way we can run extra validations and makes sure that the data is okay. roundId, rawPrice, , updateTime, answeredInRound = AggregatorV3InterfaceXXXX.latestRoundData; requirerawPrice 0,...

Buoy3Pool.safetyCheck can underflow

Handle cmichel Vulnerability details Vulnerability Details The safetyCheck function performs an unsafe subtraction on two uint256 before casting them to int256. The subtraction can underflow and the cast to int256 can either fail and revert the transaction if greater than typeint256.max, or, fit...

BaseVaultAdaptor assumes sharePrice is always in underlying decimals

Handle cmichel Vulnerability details Vulnerability Details The two BaseVaultAdaptor.calculateShare functions computes share = amount.muluint25610decimals.divsharePrice uint256 sharePrice = getVaultSharePrice; // amount is in "token" decimals, share should be in "vault" decimals share =...

Buoy3Pool.safetyCheck is not precise and has some assumptions

Handle cmichel Vulnerability details Vulnerability Details The safetyCheck function has several issues that impact how precise the checks are: 1. only checks if the a/b and a/c ratios are within BASISPOINTS. By transitivity b/c is only within 2 BASISPOINTS if a/b and a/c are in range. For a more...

Usage of deprecated ChainLink API in Buoy3Pool

Handle cmichel Vulnerability details Vulnerability Details The Chainlink API latestAnswer used in the Buoy3Pool oracle wrappers is deprecated: This API is deprecated. Please see API Reference for the latest Price Feed API. Chainlink Docs Impact It seems like the old API can return stale data...

implicit underflows

Handle gpersoon Vulnerability details Impact There are a few underflows that are converted via a typecast afterwards to the expected value. If solidity 0.8.x would be used, then the code would revert. int256a-b where a and b are uint, For example if a=1 and b=2 then the intermediate result would ...

sortVaultsByDelta doesn't work as expected

Handle gpersoon Vulnerability details Impact The function sortVaultsByDelta doesn't always work as expected. Suppose all the delta's are positive, and delta1 = delta2 = delta3 0 Then maxIndex = 0 And delta minDelta ==0 is never true, so minIndex = 0 Then assuming bigFirst==true: vaultIndexes0 =...

distributePriceChange might revert

Handle gpersoon Vulnerability details Impact The function distributePriceChange includes the following statement: lastGvtAssets = gvtAssets.addcurrentTotalAssets.subtotalAssets; If you look at this: lastGvtAssets = gvtAssets.addcurrentTotalAssets.subgvtAssets.addpwrdAssets; lastGvtAssets =...

distributeStrategyGainLoss can be abused

Handle gpersoon Vulnerability details Impact The function distributeStrategyGainLoss does the following check to allow access to the function: requireindex 0 || index = NCOINS + 1, "!VaultAdaptor"; However the expression index 0 || index = NCOINS + 1 is always TRUE, because the OR || is used shou...

Add reentracy protections on function executeTrade

Handle shw Vulnerability details Impact As written in the to-do comments, reentrancy could happen in the executeTrade function of Trader since the makeOrder.market can be a user-controlled external contract. Proof of Concept Referenced code: Trader.solL121-L126 Recommended Mitigation Steps Add a...

Return values of ERC20 transfer and transferFrom are unchecked

Handle shw Vulnerability details Impact In some contracts e.g., TracerPerpetualSwaps.sol, the return values of ERC20 transfer and transferFrom are not checked to be true, which could be false if the transferred tokens are not ERC20-compliant. In that case, the transfer fails without being noticed...

Using deprecated Chainlink function latestAnswer

Handle shw Vulnerability details Impact According to Chainlink's documentation, the latestAnswer function is deprecated. This function does not error if no answer has been reached but returns 0. Besides, the latestAnswer is reported with 18 decimals for crypto quotes but 8 decimals for FX quotes...

A market's hourly average price can be biased by a large number of trades

Handle shw Vulnerability details Impact An attacker can artificially move a market's hourly average price i.e., the result of getHourlyAvgTracerPrice by executing a large number of trades on the market with only paying gas fees. Proof of Concept The hourly average price is calculated by the...

Insurance slippage reimbursement can be used to steal insurance fund

Handle cmichel Vulnerability details The Liquidation contract allows the liquidator to submit "bad" trade orders and the insurance reimburses them from the insurance fund, see Liquidation.claimReceipt. The function can be called with an orders array which does not check for duplicate orders. An...

Use of deprecated Chainlink API

Handle 0xRajeev Vulnerability details Impact The contracts use Chainlink’s deprecated API latestAnswer. Such functions might suddenly stop working if Chainlink stopped supporting deprecated APIs. Impact: Deprecated API stops working. Prices cannot be obtained. Protocol stops and contracts have to...

Underflow problems occurring when a token has >18 decimals

Handle tensors Vulnerability details Impact The contracts assume that all tokens will have =18 decimals. If the Tracer team are the only people deploying the contracts, and they keep this in mind, this isn't a problem. If the contracts are to be deployed by other people, this assumption should be...

Malicious owner can arbitrarily change fee to any % value

Handle 0xRajeev Vulnerability details Impact Tracer protocol like any other allows market creators to charge fees for trades. However, a malicious/greedy owner can arbitrarily change fee to any % value and without an event to observe this change or a timelock to react, there is no easy way for...

avoid paying insurance

Handle gpersoon Vulnerability details Impact It's possible to avoid paying insurance in the following way: once per hour at the right moment, do the following: ----using a flash loan, or with a large amount of tokens, call deposit of Insurance.sol to make sure that the pool is sufficiently filled...

Logic error in fee subtraction

Handle 0xsanson Vulnerability details Impact In LibBalances.applyTrade we need to collect a fee from the trade. The current code however subtracts a fee from the short position and adds it to the long. The correct implementation is to subtract a fee to both see TracerPerpetualSwaps.solL272. This...