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

Free stake by replacing input token address

Handle jonah1005 Vulnerability details Impact In PoolOpen contract, pools' authentication is done through baseData. However, it handles token transfer based on the given parameter token. By appending a real token address at the end of the transaction and a fake address in the function parameter,...

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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