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

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

Broken access control leads to protocol functionality freeze

Handle 0xRajeev Vulnerability details Impact The contracts use an access control pattern where the contract deployer is included in the onlyDAO modifier which is used for authorized access to critical functions. Such contracts also include a purgeDeployer function which renounces sets to...

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

Unchecked transfers found in 3 contracts

Handle maplesyrup Vulnerability details Impact This is a high priority vulnerability because it definitely affects the way that funds are transferred and sent between the contracts. You want to make sure that you check the boolean value from these transfer functions in order to make sure that the...

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

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

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

Pool._addPoolMetrics(uint256) is subject to potential miner manipulation

Handle heiho1 Vulnerability details Impact Pool.addPoolMetricsuint256 on line 334 relies on block.timestamp and is potentially vulnerable to miner manipulation. This could lead to erroneous pool metrics. Proof of Concept Tools Used Slither Recommended Mitigation Steps An external time oracle like...

Synth: cannot change approval once set to max value

Handle cmichel Vulnerability details The Synth.approve function performs a no-op if the allowance is currently set to typeuint256.max. This leads to the issue that approvals cannot be changed anymore once they are set to the max value. Impact Imagine someone approving an operator with the max val...

Pools can be created without initial liquidity

Handle cmichel Vulnerability details Vulnerability Details The protocol differentiates between public pool creations and private ones starting without liquidity. However, this is not effective as anyone can just flashloan the required initial pool liquidity, call PoolFactory.createPoolADD, receiv...

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

Router._handleTransferIn(address,uint256,address) has unchecked transfers

Handle heiho1 Vulnerability details Impact Router.handleTransferInaddress,uint256,address on lines 203 and 207 ignores the boolean return on transfers. This is a brittle implementation because it relies on the boolean return value being hard-coded to true. Tokens may return false instead of...

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

Router.zapLiquidity(uint256,address,address) has unchecked transfers

Handle heiho1 Vulnerability details Impact Router.zapLiquidityuint256,address,address has unchecked transfers on lines 65, 67 and 69. Several tokens do not revert in case of failure and return false. If one of these tokens is used in Router, deposit will not revert if the transfer fails, and an...

No ERC20 safe* versions called

Handle cmichel Vulnerability details Some tokens like USDT don't correctly implement the EIP20 standard and their transfer/transferFrom function return void instead of a success boolean. Calling these functions with the correct EIP20 function signatures will always revert. This is generally not a...

Pool has unchecked transfers

Handle heiho1 Vulnerability details Impact Pool.removeForMemberaddress, Pool.swapToaddress,address and Pool.burnSynthaddress,address on lines 198, 199, 224, 250, and 253 ignore the boolean return on transfers. This is a brittle implementation because it relies on the boolean return value being...

Missleading onlyDAO modifiers

Handle cmichel Vulnerability details Several contracts implement an onlyDAO modifier which, as the name suggests, should only authorize the function to be executed by the DAO. However, some implementations are wrong and either allow the DAO or the deployer to execute, or even only the deployer:...

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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