Synth: approveAndCall sets unnecessary approval

Handle cmichel Vulnerability details Vulnerability Details The Synth.approveAndCall function approves the recipient contract with the max value instead of only the required amount. Impact For safety, the approval should not be set to the max value, especially if the amount that the contract may u...

BondVault BASE incentive can be gamed

Handle cmichel Vulnerability details BondVault deposits match any deposited token amount with the BASE amount to provide liquidity, see Docs and DAO.handleTransferIn. The matched BASE amount is the swap amount of the token trade in the pool. An attacker can manipulate the pool and have the DAO...

[Pool] - Flash loan + Synth.realise allows anyone to extract value from LPs

Handle adelamo Vulnerability details Impact On Synth.sol, we have the method realise that checks if the LP value is higher than the Synth value. If confirmed, it will burn the premium LP. Using a flash loan, we can add liquidity to the pool, mint some LP tokens. Then, call realise using the pool...

SynthVault rewards can be gamed

Handle cmichel Vulnerability details Vulnerability Details The SynthVault.deposit function adds weight for the user that depends on the spot value of the deposit synth amount in BASE. This spot price can be manipulated and the cost of manipulation is relative to the pool's liquidity. However, the...

Dividend reward can be gamed

Handle cmichel Vulnerability details The Router.addDividend function tells the reserve to send dividends to the pool depending on the fees. The attacker provides LP to a curated pool. Ideally, they become a large LP holder to capture most of the profit, they should choose the smallest liquidity...

unchecked return value from Transfer()/TransferFrom

Handle JMukesh Vulnerability details Impact Proof of Concept Tools Used manual review Recommended Mitigation Steps add a require condition to check the return value --- The text was updated successfully, but these errors were encountered: All reactions...

removeLiquiditySingle loses funds

Handle cmichel Vulnerability details Vulnerability Details The Pool.removeLiquiditySingle function redeems liquidity tokens for underlying to the router contract. If toBase == true, it then tries to convert these to base tokens. However, only the swapped token - BASE amount is sent to the user, t...

[Pool] - Anyone can remove liquidity from Pools, allowing them to alter the price

Handle adelamo Vulnerability details Impact On the Pool.sol, the function removeForMember is public. Allowing anyone to call the method using an address of an LP in order to remove liquidity from the pools and return to the LP account. If we combine the ability to remove liquidity and being able ...

Vote removals for critical proposal types are not re-checked for hasMajority at execution

Handle 0xRajeev Vulnerability details Impact If votes are removed such that hasMajority is false but hasQuorum is true voting weight is in-between quorum and majority i.e. 50% and =66.6% for critical proposal types DAO, UTILS, RESERVE, GETSPARTA, ROUTER, LISTBOND, GRANT, ADDCURATEDPOOL, they will...

Unnecessary/Incorrect onlyDAO modifier could be an indication of missing access control

Handle 0xRajeev Vulnerability details Impact Unnecessary/incorrect access control modifier is typically an indication of missing critical authorization checks. The onlyDAO modifier used in various protocol contracts is present in synthFactory.sol but used only in the purgeDeployer which sets...

withdraw() not defined (Router.sol#217)

Handle 7811 Vulnerability details Impact withdraw not defined. iWBNBWBNB.withdrawamount; Router.sol217 Proof of Concept Tools Used editor Recommended Mitigation Steps --- The text was updated successfully, but these errors were encountered: All reactions...

Missing input validation in realise()

Handle 0xsanson Vulnerability details Impact In synth.sol, the function realisepool can be called using any existing pool as input. From my understanding, it's supposed that pool and synth must have the same underlying token. With the current implementation an user can call various synth contract...

Missing function setParams in Dao

Handle 0xsanson Vulnerability details Impact The function setParams in synthVault is supposed to be called by the Dao, but this contract doesn't have it, causing the impossibility to update the parameters by the protocol. Proof of Concept Tools Used editor Recommended Mitigation Steps Add the...

SynthVault withdraw forfeits rewards

Handle cmichel Vulnerability details Vulnerability Details The SynthVault.withdraw function does not claim the user's rewards. It decreases the user's weight and therefore they are forfeiting their accumulated rewards. The synthReward variable in processWithdraw is also never used - it was probab...

Missing purgeDeployer function

Handle 0xsanson Vulnerability details Impact In most of the contracts there's a purgeDeployer function that sets the Deployer address to zero. Contracts synthVault and Router though don't have it. Since the idea is to decentralize the protocol giving all the power to the Dao, the function should ...

Unused/Incorrect onlyDAO modifier could be an indication of missing access control

Handle 0xRajeev Vulnerability details Impact Unused/incorrect access control modifier is typically an indication of missing critical authorization checks. The onlyDAO modifier used in various protocol contracts is present in Synth.sol but unused in any of the contract functions and is also...

Router.removeLiquidityExact(uint256,address) has unchecked transfers

Handle heiho1 Vulnerability details Impact Router.removeLiquidityExactuint256,address line 114 does not check the boolean return of a token transfer. This is a brittle implementation because it relies on the boolean return value being hard-coded to true. Tokens may not revert in case of failure a...

Missing slippage checks

Handle cmichel Vulnerability details The Router and Pool does not implement any slippage checks with comparing the swap / liquidity results with a minimum swap / liquidity value. Impact Users can be frontrun and receive a worse price than expected when they initially submitted the transaction...

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 UniswapV3Oracle. Proof of Concept Referenced code:...

Deflationary and fee-on-transfer tokens are not correctly accounted

Handle shw Vulnerability details Impact When a user transfers deflationary or fee-on-transfer tokens to the LendingPair, the pair does not correctly handle the received amount, causing the minted LP token amount to be greater than the received amount. Transactions calling the withdrawAll function...

LendingPair.liquidateAccount fails if tokens are lent out

Handle cmichel Vulnerability details The LendingPair.liquidateAccount function tries to pay out underlying supply tokens to the liquidator using safeTransferIERC20supplyToken, msg.sender, supplyOutput but there's no reason why there should be enough supplyOutput amount in the contract, the contra...

Total LP supply & total debt accrual is wrong

Handle cmichel Vulnerability details The total debt and total supply only increase when debt/supply is minted to the user when it should increase by the entire new interest amount on each accrual. function accrueAccountaddress account public distributeRewardaccount; // accrue only updates...

Unused return value from transfer()

Handle JMukesh Vulnerability details Impact It is usually good to add a require-statement that checks the return value or to use something like safeTransfer; unless one is sure the given token reverts in case of a failure. Proof of Concept Tools Used manual review Recommended Mitigation Steps che...

Insuffiecient input validation in initialize() in LendingPair.sol

Handle JMukesh Vulnerability details Impact function initialize address lpTokenMaster, address controller, IERC20 tokenA, IERC20 tokenB It lack the input validtion of tokenA and tokenB whether they are different or not , if it is same then we will have two lptoken with same address but different...

Use of deprecated Chainlink API

Handle 0xRajeev Vulnerability details Impact UniswapV3Oracle contract uses 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...

LendingPair.liquidateAccount does not accrue and update cumulativeInterestRate

Handle cmichel Vulnerability details The LendingPair.liquidateAccount function does not accrue and update the cumulativeInterestRate first, it only calls accrueAccountInterest which does not update and instead uses the old cumulativeInterestRate. Impact The liquidatee borrower's state will not be...

Incompatibility With Rebasing/Deflationary/Inflationary tokens

Handle 0xRajeev Vulnerability details Impact WildCredit allows the permissionless listing of any ERC20 assets/pairs to be used in the protocol. Some of these tokens could charge a fee, add a reward or rebase over time. However, the protocol does not have the required support to handle such tokens...

ChainLink price data could be stale

Handle cmichel Vulnerability details There is no check in UniswapV3Oracle.ethPrice if the return values indicate stale data. This could lead to stale prices according to the Chainlink documentation: under current notifications: "if answeredInRound roundId could indicate stale data." under...

SelfBurn lpToken may create bad debt

Handle jonah1005 Vulnerability details Impact Users can create bad debt by burning their lpToken. Bad debt of the system would increase the utilization rate and hence increase the interest rate of the counterpart token. It would be a reasonable action to burn lp if the user is no longer profitabl...

Rewards can be migrated to an arbitrary address at anytime by owner

Handle 0xRajeev Vulnerability details Impact The migrateRewards function which is onlyOwner takes recipient and amount parameters, which effectively allows owner to migrate the contract’s entire rewardToken balance at any time to that address. While the stated purpose is that this “Allows to...

LendingPair._safeTransfer does not work with all ERC20 tokens

Handle cmichel Vulnerability details The LendingPair.safeTransfer function will revert if tokens do not return a boolean because the interface ERC20.transfer function it uses indicates that this function always returns a boolean. Some tokens like USDT don't correctly implement the EIP20 standard...

The safe versions of transfer/transferFrom are not implemented as expected

Handle 0xRajeev Vulnerability details Impact The “safe” versions of token transfer/transferFrom as implemented either by OpenZeppelin’s SafeERC20 or Uniswap libraries, use a low-level call and make checks on the return data to handle cases where tokens may not return any value on...

TransferHelper does not work with all ERC20 tokens

Handle cmichel Vulnerability details The TransferHelper.safeTransferFrom function will revert if tokens do not return a boolean because the interface ERC20.transfer function it uses indicates that this function always returns a boolean. Some tokens like USDT don't correctly implement the EIP20...

Reward computation is wrong

Handle cmichel Vulnerability details The LendingPair.accrueAccount function distribtues rewards before updating the cumulative supply / borrow indexes as well as the index + balance for the user by minting supply tokens / debt. This means the percentage of the user's balance to the total is not...

Recommended

Handle 0xsanson Vulnerability details Impact Detailed description of the impact of this finding. Proof of Concept Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept. Tools Used Recommended Mitigation Steps --- The...

Owner can burn other users token shares

Handle JMukesh Vulnerability details Impact This privilege should not be given to the owner, which can burn other users token shares, due to this user will not be able to claim their liquidity. Due to this privilege user will lost control over liquidity amount Proof of Concept Tools Used manual...

_wethWithdrawTo is vulnerable re-entrancy

Handle pauliax Vulnerability details Impact function withdrawBorrowETH invokes wethWithdrawTo and later checkMinReserve, however, the check of reserve is not necessary here, as function wethWithdrawTo also does that after transferring the ether. However, this reserve check might be bypassed as...

safeTransferFrom in TransferHelper is not safeTransferFrom

Handle jonah1005 Vulnerability details Impact A non standard erc20 token would always raise error when calling safeTransferFrom. If a user creates a USDT/DAI pool and deposit into the pool he would find out there's never a counterpart deposit. Proof of Concept TransferHelper does not uses SafeERC...

Chainlink - Use latestRoundData instead latestAnswer to run more validations

Handle adelamo Vulnerability details Impact UniswapV3Oracle.sol is calling latestAnswer to get the last WETH price. This method will return the last value, but you won't be able to check if the data is fresh. On the other hand, calling the method latestRoundData allow you to run some extra...

Lending Pair initialize function can be front run.

Handle jonah1005 Vulnerability details Impact LendingPair does not initialize tokenMaster, controller, tokens. A hacker can listen the deployer address and front run the initialize transaction. The initialized contract would look almost exactly the same if the hacker only replace lpTokenMaster wi...

Signature replay attacks are possible if a user's transactionId is not unique

Handle shw Vulnerability details Impact If a user's two transactions can have the same transactionId while their router addresses differ, then the fulfill and cancel signatures for one of the transaction is replayable on the other. Proof of Concept The only transaction-related data in the...

Assets are transferred if the execute call fails

Handle shw Vulnerability details Impact When the user calls fulfill with a non-zero callTo parameter, the TransactionManager tries to call execute on callTo, and if the function call fails, the manager transfers toSend amount of receiving assets to receivingAddress. However, since the assets may...

Gas griefing attack on the removeUserActiveBlocks function

Handle shw Vulnerability details Impact The consumed gas to remove a user's active block is proportional to the total number of array elements i.e., block numbers. However, the array size can be arbitrarily increased by an attacker with only paying gas fees, causing a gas griefing attack when the...

The approval before addFunds could allow users to transfer out more assets

Handle shw Vulnerability details Impact When the user calls fulfill on the receiving chain with a non-zero callTo address, the TransactionManager approves callTo to transfer toSend amount of receiving assets. Then, the manager tries to call addFunds on callTo, and if the call fails, the manager...

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

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

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

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

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

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