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

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

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

addFunds and execute may send tokens twice

Handle pauliax Vulnerability details Impact Both calls to IFulfillHelper addFunds and execute are wrapped in separate try/catch statements so basically if addFunds succeeds but execute fails or both of these functions fail, the catch will still send assets to the receivingAddress. I think these...

Transactions nearing block.gaslimit may cause issues

Handle hrkrshnn Vulnerability details transactions nearing block.gaslimit may cause issues A general problem with schemes that require multiple steps and whose inputs are dynamically typed is that: a user may be able to "commit" something, but never "execute" it because the "execute" transaction...

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

Griefing lenders

Handle gpersoon Vulnerability details Impact Suppose there are several lenders for token B of a pair. And an attacker puts in a deposit for token A to the maximum possible amount via accountA. And an attacker puts in a deposit for token B to the maximum possible amount via accountB. And then lend...

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

UniswapV3Oracle: Check for stale ETH prices

Handle greiart Vulnerability details Impact Freshness of the returned ETH price should be checked, since it affects an account's health and therefore liquidations. The latestAnswer function is deprecated see comment on EACAggregatorProxy, and the use latestRoundData is recommended instead...

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

No check transferFrom() return value

Handle s1m0 Vulnerability details Impact The smart contract doesn't check the return value of token.transfer and token.transferFrom, some erc20 token might not revert in case of error but return false. In the TracerPerpetualSwaps:deposit and Insurance:deposit this would allow a user to deposit fo...

Usage of deprecated ChainLink API in GasOracle

Handle cmichel Vulnerability details The Chainlink API latestAnswer used in the GasOracle 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. Checks similar to that of...

Wrong price scale for GasOracle

Handle cmichel Vulnerability details The GasOracle uses two chainlink oracles GAS in ETH with some decimals, USD per ETH with some decimals and multiplies their raw return values to get the gas price in USD. However, the scaling depends on the underlying decimals of the two oracles and could be...

Use latestRoundData instead of latestAnswer

Handle adelamo Vulnerability details Impact Use latestRoundData instead of latestAnswer for chainlink Oracles in order to be able to run more validations like roundId, rawPrice, , updateTime, answeredInRound = AggregatorV3Interfacesource.source.latestRoundData; requirerawPrice 0, "Chainlink price...

Wrong funding index in settle when no base?

Handle cmichel Vulnerability details The TracerPerpetualSwaps.settle function updates the user's last index to currentGlobalFundingIndex, however a comment states: "// Note: global rates reference the last fully established rate hence the -1, and not the current global rate. User rates reference...

Missing events for critical parameter changing operations by owner

Handle 0xRajeev Vulnerability details Impact The owner of TracerPerpetualSwaps contract, who is potentially untrusted as per specification, can change the market critical parameters such as the addresses of the Liquidation/Pricing/Insurance/GasOracle/FeeReceiver and also critical values such as...