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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

SafetyWithdraw can transfer any erc20 token

Handle pauliax Vulnerability details Impact contract TracerPerpetualSwaps is SafetyWithdraw. It allows an owner to transfer all tokens from this contract whenever he wants e.g. no protection against tracerQuoteToken, etc. Tracer representative's answer on Discord: 'safetyWithdraw isn't needed as ...

Bots can frontrun market maker orders and cancel orders.

Handle tensors Vulnerability details Impact Market makers will not be able to have competitive spreads or cancel their orders to update their strategies. It will be difficult for them to provide liquidity on the platform, which will hurt the platform overall. Proof of Concept In traditional finan...

Use of incorrect index leads to incorrect updation of funding rates

Handle 0xRajeev Vulnerability details Impact The updateFundingRate function updates the funding rate and insurance funding rate. While the instant/new funding rates are calculated correctly, the cumulative funding rate calculation is incorrect because it is always adding the instant to 0, not the...

erc20 transfers do not check the return value

Handle pauliax Vulnerability details Impact When transfering erc20 tokens, functions transfer and transferFrom are used. These functions return boolean to indicate if the action was sucessfull, however, none of the usages check the returned value: collateralToken.transferFrommsg.sender,...