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

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

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

Signatures use only tx ID instead of entire digest

Handle 0xRajeev Vulnerability details Impact The signature check in recoverFulfillSignature only uses transaction ID along with the relayer fee which can be accidentally reused by the user, in which case the older signatures with the older relayer fees can be replayed. The signature should be on...

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

WETH not verified to be one of the pair tokens

Handle gpersoon Vulnerability details Impact The ETH functions of LendingPair don't check that WETH is one of the 2 pairs of the contract and introduces a third token. The third token ETH/WETH falls outside the other checks that the contract is doing. You could accidentally pay ETH/WETH to the...

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

Malicious owner can drain the market at any time using SafetyWithdraw

Handle 0xRajeev Vulnerability details Impact The withdrawERC20Token in SafetyWithdraw inherited in TracerPerpetualSwaps is presumably a guarded launch emergency withdrawal mechanism. However, given the trust model where the market creator/owner is potentially untrusted/malicious, this is a...

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

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

No support for token with decimals > 18

Handle s1m0 Vulnerability details Impact The smart contract doesn't behave correctly if deployed with token that have decimals 18. Proof of Concept The functions tokenToWad and wadToToken revert if the tokenDecimals is 18. These functions are called in critical places like deposit and withdraw...

Insurance ERC20 return values not checked

Handle cmichel Vulnerability details The ERC20.transfer and ERC20.transferFrom functions return a boolean value indicating success. This parameter should be checked for success. The Insurance.deposit and Insurace.withdraw functions dp not check the return value: // deposit...

Trader orders can be frontrun and users can be denied from trading

Handle cmichel Vulnerability details The Trader contract accepts two signed orders and tries to match them. Once they are matched and become filled, they can therefore not be matched against other orders anymore. This allows for a griefing attack where an attacker can deny any other user from...