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 deposit lockup bypass

Handle cmichel Vulnerability details Vulnerability Details The SynthVault.harvestSingle function can be used to mint & deposit synths without using a lockup. An attacker sends BASE tokens to the pool and then calls harvestSingle. The inner iPOOLpoolOUT.mintSynthsynth, addressthis; call will mint...

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

Router.removeLiquiditySingle(uint256,bool,address) has unchecked transfers

Handle heiho1 Vulnerability details Impact Router.removeLiquiditySingleuint256,bool,address on lines 121, 126, 129 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...

Unbounded iteration in curatedPoolCount

Handle cmichel Vulnerability details The PoolFactory.curatedPoolCount iterates over all arrayPools. Anyone can push to this array by creating a pool making this attack easy to execute for an attacker. Impact The transactions can fail if the arrays get too big and the transaction would consume mor...

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

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

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

Misplaced logic leads to loss of member bondedLP funds

Handle 0xRajeev Vulnerability details Impact In claimForMember, the member claims back some of their bonded LPs. The check to see if claimRate can be made 0 should preceed the claimable deduction on L110. This misplaced check after deduction leads to incorrect zero-ing of member’s non-zero...

Router._handleTransferOut(address,uint256,address) has unchecked transfer

Handle heiho1 Vulnerability details Impact Router.handleTransferOutaddress,uint256,address on line 221 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 and not revert in which...

ERC20 return values not checked

Handle cmichel Vulnerability details The ERC20.transfer and ERC20.transferFrom functions return a boolean value indicating success. This parameter needs to be checked for success. Some tokens do not revert if the transfer failed but return false instead. This is generally not an issue when the...

Router has an arbitrary send

Handle heiho1 Vulnerability details Impact Router.sol line 221 has an arbitary-send of iBEP20token.transferrecipient, amount. The call ignore the transfer result. This is a brittle implementation because it relies on the boolean return value being hard-coded to true. Further, if a token...

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

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

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

DaoVault.withdraw(address,address) potentially subject to timestamp manipulation

Handle heiho1 Vulnerability details Impact DaoVault.withdrawaddress,address uses block.timestamp based comparisons can be affected by miner behavior, leading to withdrawal impacts on the user. Proof of Concept Tools Used Slither Recommended Mitigation Steps An external time oracle like ChainLink...

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

Pool: approveAndCall sets unnecessary approval

Handle cmichel Vulnerability details Vulnerability Details The Pool.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 us...

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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