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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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