Payout.deduction computation uses wrong decimals

Handle cmichel Vulnerability details The doSherX function computes the burned SherX tokens deduction as: // excludeUsd = amountsi.mulsx.tokenUSDtokensi deduction = excludeUsd.divcurTotalUsdPool.divSherXERC20Storage.sx20.totalSupply.div10e17; This seems to only work if tokensi has 18 decimals, as...

[Bug] A critical bug in bps function

Handle hrkrshnn Vulnerability details A critical bug in bps function: PoolBase.sol function bps internal pure returns IERC20 rt // These fields are not accessible from assembly bytes memory array = msg.data; uint256 index = msg.data.length; // solhint-disable-next-line no-inline-assembly assembly...

[Gov.sol] Ignoring the return value of function _token.approve(...)

Handle eriksal1217 Vulnerability details Impact Medium Risk vulnerability - This does not immediately affect the contract, tokens, or funds associated but could have negative effects in regards to how the contract behaves when executing this functionality. Proof of Concept According to Slither...

TokenToLock default value

Handle cmichel Vulnerability details The PoolBase.TokenToLockXRate function returns the "Current exchange rate from token to lockToken". It does not specify the precision and according to the documentation, it sounds like one just has to multiply this value by any token amount to get the...

Single under-funded protocol can break paying off debt

Handle cmichel Vulnerability details The SherXERC20.payOffDebtAll function iterates over all protocols of the token. If a single project does not have enough funds to cover the premium payments, the transactions come to a halt, see payOffDebt: debt = accruedDebtps, protocol, blocks; // this can...

Token price should not be set manually.

Handle tensors Vulnerability details Impact The Manager.sol file contains many methods to let Watsons manually set the token price. This should never be done, and gives free incentives for malicious users to arbitrage price discrepancies from the pool. Proof of Concept In general, these price...

Deposits don't work with fee-on transfer tokens

Handle cmichel Vulnerability details There are ERC20 tokens that may make certain customizations to their ERC20 contracts. Some tokens charge a certain fee for every transfer or transferFrom. Others types are rebasing tokens that increase in value over time like Aave's aTokens balanceOf changes...

_token parameter not validated

Handle pauliax Vulnerability details Impact function depositProtocolBalance does not validate the token, nor the caller. It is possible to call this function passing any arbitrary token and amount values and thus artificially increasing protocolBalance which may lead to further failed computation...

_doSherX optimistically assumes premiums will be paid

Handle cmichel Vulnerability details The doSherX function does not attempt to pay off the accrued premiums "pay off debt" for most tokens, only for the ones that would otherwise revert the tx: // Expensive operation, only execute to prevent tx reverts if amountsi ps.sherXUnderlying...

[SherX.sol] Multiple external function values are ignored

Handle eriksal1217 Vulnerability details Impact Medium Risk vulnerability - This does not immediately affect the contract, tokens, or funds associated but could have negative effects in regards to how the contract behaves when executing this functionality. Proof of Concept According to Slither...

Yield distribution after large payout seems unfair

Handle gpersoon Vulnerability details Impact When a large payout occurs, it will lower unallocatedSherX. This could mean some parties might not be able to get their Yield. The first couple of users for which harvest is called or which transfer tokens will be able to get their full Yield, until th...

transfer blocked by insufficient unallocatedSherX

Handle gpersoon Vulnerability details Impact When doYield is done, it subtracts the "withdrawableamount" from unallocatedSherX. However unallocatedSherX could be lowered by a large payout, possibly being insufficient to subtract "withdrawableamount". If this would be the case, then doYield would...

DoS for protocol withdrawals

Handle walker Vulnerability details type: Denial of Service severity: High A problem exists in the poolbase logic which calls LibPool.payOffDebtAlltoken in the withdrawProtocolBalance function. This call will fail if one of the protocols in the respective pool has an insufficient balance to pay o...

Insider attack

Handle walker Vulnerability details type: Custodial One should note the custodial nature of the current implementation. A single malicious developer could use their access to the governance multi-sig accounts to extract all the funds from the smart contract. Even under the assumption that the who...

Incorrect internal balance bookkeeping

Handle walker Vulnerability details type: Incorrect Assumptions on External Systems The sherlock smart contract system uses internal bookkeeping of arbitrary ERC20 token balances. It doesn't assert that the ERC20 doesn't implement some non-standard behaviour. For example, deflationary tokens, or...

Ensure payOffDebtAll can always run

Handle gpersoon Vulnerability details Impact If one of the protocols doesn't have enough funds in its protocolBalance, then payOffDebt will revert when trying to subtract the debt. This also means the function payOffDebtAll will revert. As this function is called from several other functions,...

reputation risks with updateSolution

Handle gpersoon Vulnerability details Impact GovDev.sol has a function updateSolution to upgrade parts of the contract via the Diamond construction. Via updateSolution any functionality can be changed and all the funds can be accessed/rugged. Even if this is well intended the project could still ...

Possible DoS attack in creating new DAO proposals

Handle shw Vulnerability details Impact The functions of creating new DAO proposals e.g., newActionProposal are permissionless. Anyone can create a new proposal by paying some fees in SPARTA, as long as the previous proposal is closed. Thus, an attacker could then front-run proposals of benign...

Flash loan manipulation on getPoolShareWeight of Utils

Handle shw Vulnerability details Impact The getPoolShareWeight function returns a user's pool share weight by calculating how many SPARTAN the user's LP tokens account for. However, this approach is vulnerable to flash loan manipulation since an attacker can swap a large number of TOKEN to SPARTA...

Allowance cannot be reset once it is set to the maximum

Handle shw Vulnerability details Impact The approve functions of the pool LP tokens and synths do nothing if the allowances is already the maximum number, i.e., typeuint256.max. Therefore, Alice cannot change her allowance to Bob once she approved him with the maximum approval. Proof of Concept...

Return values of BEP20.transfer and BEP20.transferFrom are unchecked

Handle shw Vulnerability details Impact The return values of BEP20.transfer and BEP20.transferFrom are not checked to be true in multiple contracts. The return value could be false if the transferred token is not BEP20-compliant, indicating that the transfer fails, while the calling contract will...

Improper access control of claimAllForMember allows anyone to reduce the weight of a member

Handle shw Vulnerability details Impact The claimAllForMember function of Dao is permissionless, allowing anyone to claim the unlocked bonded LP tokens for any member. However, claiming a member's LP tokens could decrease the member's weight in the BondVault, thus affecting the member's votes and...

Swap fees are not accounted for if users swap directly from the pools

Handle shw Vulnerability details Impact The Pool calculates swap fees whenever there is a swap between the SPARTA and the TOKEN or synths. The Router uses this fee value to decide the amount of dividend transferred from the Reserve to the Pool. However, if a user performs the swap by directly...

PoolBase enables an easy withdrawal of funds

Handle walker Vulnerability details PoolBase enables an easy withdrawal of all funds severity: critical type: memory safety Description A memory safety bug in the pool base allows participants to trick the system into believing they're interacting with a pool's token. While in reality, they're...

Possible to deny proposal creations

Handle cmichel Vulnerability details There can only ever be one active proposal. An attacker could front-run a proposal creation by submitting a dummy proposal. The original proposal creation would fail. Impact The cost of the attack is the proposal fees. There can be an incentive to deny the...

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

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

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

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

Deployer backdoors in DAOVault, Router and SynthVault contracts

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

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

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

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

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

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

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

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

Handle heiho1 Vulnerability details Impact Router.swapSynthToAssetuint256,address,address on line 249 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...

[PoolFactory.sol] createPoolADD() function is payable but does not contain a function to withdraw funds

Handle maplesyrup Vulnerability details Impact This is a medium risk vulnerability as it can affect funds within pools that are created via this contract. With no withdraw functions being implemented, it is possible that funds can be locked in the contract with no way to retrieve earnings or...

Dao has unchecked transfers

Handle heiho1 Vulnerability details Impact Dao.moveBASEBalanceaddress and Dao.handleTransferInaddress,uint256 do not check transfer return values on lines 220 and 266. If the token implementation returns false and does not revert, then potentially the user can deposit for free. Proof of Concept...

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

Dao.calcReward(address) has potential division before multiplication arithmetic errors

Handle heiho1 Vulnerability details Impact Dao.calcRewardaddress on lines 203 and 204 are potentially problematic in that division may potentially truncate values resulting in loss of precision. Proof of Concept Tools Used Slither Recommended Mitigation Steps Potentially this issue is lessened wi...

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

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

Router.addDividend(address,uint256) has potentially unsafe arithmetic

Handle heiho1 Vulnerability details Impact Router.addDividendaddress,uint256 has potentially unsafe division before multiplication. This could lead to truncation. Proof of Concept Tools Used Slither Recommended Mitigation Steps Potentially this issue is mitigated by recent Solidity changes to...

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

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

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

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

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