Wrong indexes: positionId vs incentiveId

Handle 0xsanson Vulnerability details Impact In multiple functions in ConcentratedLiquidityPoolManager, the index positionId is used instead of the correct incentiveId when dealing with the incentives mapping. Of course the issue is that incentives cannot be used, or in some cases only by lucky...

uToken's approve() Function is Susceptible to Frontrunning Attacks

Handle leastwood Vulnerability details Impact Order makers will call the approve function to facilitate transactions between order takers and makers. If a taker sees that a maker is seeking to reduce its order exposure by reducing Swivel.sol's allowance, a taker can frontrun this transaction to...

Erc20.transferFrom() doesn't work on non-standard compliant tokens like USDT

Handle pants Vulnerability details Non-standard compliant tokens like USDT don't return a return value on transferFrom. The function Erc20.transferFrom reverts if the token doesn't return a return value, as it is defined to always return a boolean. Impact When using any non-standard compliant tok...

Erc20.transfer() return value is ignored

Handle pants Vulnerability details According to the ERC-20 Token Standard, the function transfer returns false on failure. However, the return value of Erc20.transfer is ignored 8 times: 1. In Swivel.exitVaultFillingZcTokenExit, line 313. 2. In Swivel.exitVaultFillingZcTokenExit, line 315. 3. In...

User can get more/less interest than required

Handle csanuragjain Vulnerability details Impact It seems that owner must not get interest for transfer fee which is collected using transferNotionalFee No interest is calculated on owner side before reducing the owner notional amount But this can simply be bypassed if owner adds new notional whi...

TridentRouter.addLiquidity() Add liquidity to IndexPool through TridentRouter may casue loss of a small portion of users funds

Handle WatchPug Vulnerability details The mint function in IndexPool requires the liquidity provider to transfer in amounts no less than the amounts of tokens' reserve proportionally to the toMint amount. However, the TridentRouter won't calculate the toMint amount and amountsIn for the liquidity...

UniswapV3Helper: getSqrtPriceX96() doesn't work for tokens with non-18 decimals

Handle hickuphh3 Vulnerability details Impact The getSqrtPriceX96 will return incorrect values for pairs comprising of non-18 decimals. This affects the amounts calculated for a position. Proof of Concept Let us take the ETH-WBTC pair as an example. Note that WBTC has 8 decimals, and is an active...

onlyOwner Role Can Manipulate Price Oracle

Handle leastwood Vulnerability details Impact The V2 LendingController.sol contract incorporates a custom oracle used to fetch the relevant price feeds for token pairs. The onlyOwner role is controlled by a modified timelock contract callable only from a single externally owned account. This role...

bondBurn() Leads to Unrecoverable Tokens

Handle leastwood Vulnerability details Impact An auction bonder has a single day's worth of blocks to call settleAuction and perform the necessary rebalance. In the event this does not happen, any user is able to call bondBurn and have the bond burnt in its entirety. As a result, the basketTokens...

approveUnderlying isn't safe

Handle 0xsanson Vulnerability details Impact In Basket.sol, approveUnderlying is used to approve tokens to be spent by the Auction. The current implementation uses a simple approve function, instead of the safer safeApprove. Also it's recommended to have an approve to zero first, since the...

Basket.sol#auctionBurn() A failed auction will freeze part of the funds

Handle WatchPug Vulnerability details Given the auctionBurn function will burn the auction bond without updating the ibRatio. Once the bond of a failed auction is burned, the proportional underlying tokens won't be able to be withdrawn, in other words, being frozen in the contract. Proof of Conce...

Zero fees

Handle goatbug Vulnerability details Impact The fees system is broken. Anyone can easily pay zero fees. Proof of Concept Create a bundle of two txs to be mined in one block. First tx calls mintTo with an extrememly small amount, handleFees is called and importantly lastFee = block.timestamp; Seco...

Auction.sol#settleAuction() Mishandling bounty state could potentially disrupt settleAuction()

Handle WatchPug Vulnerability details function withdrawBountyuint256 memory bountyIds internal // withdraw bounties for uint256 i = 0; i bountyIds.length; i++ Bounty memory bounty = bountiesbountyIdsi; requirebounty.active; IERC20bounty.token.transfermsg.sender, bounty.amount; bounty.active =...

Auction multiplier set to zero

Handle goatbug Vulnerability details Impact function setAuctionMultiplieruint256 newAuctionMultiplier public override onlyOwner auctionMultiplier = newAuctionMultiplier; auction multiplier can be set to zero by factory owner. This would stop the auction settling, function would always revert...

lack of checks in Factory::setBondPercentDiv allow owner to prevent bonding in Auction::bondForRebalance()

Handle GalloDaSballo Vulnerability details Impact setBondPercentDiv has no checks for min and max Setting bondPercentDiv to 0 will cause Auction::bondForRebalance to revert This allows the owner to prevent bonding by setting the bondPercentDiv to 0 Recommended Mitigation Steps Refactor to functio...

Adding assymetric liquidity in _addLiquidity results in fewer LP tokens minted than what should be wanted

Handle tensors Vulnerability details Impact Because the call in addLiquidity forwards the entire balances of the 3 stablecoins without checking the ratio between the 3, less liquidity is minted than what should be wanted. Furthermore, an attacker can abuse this arbitrage the forwarded balances if...

missing access control in basket.sol

Handle jah Vulnerability details Impact function mint and function minTo are not protected so anyone can mint Proof of Concept Tools Used manual analysis Recommended Mitigation Steps use modifier to check who can call the function --- The text was updated successfully, but these errors were...

Carefully add tokens to the list that the protocol uses

Handle tensors Vulnerability details Impact As of right now I believe the only outside tokens the protocol uses are DAI, USDC, USDT and WETH. If other tokens are added, make sure to check that they have no callbacks on transfer. For example, CREAM protocol added the AMP token which has a callback...

Use mutex lock on VaultHelper.sol

Handle tensors Vulnerability details Impact I strongly recommend adding a nonreentrant modifier on the functions within VaultHelper.sol The contract makes a bunch of unsafe external calls to the user submitted addresses vault and gauge. Also, add some checks to make sure vault and gauge are...

convert fails for fee-on-transfer tokens

Handle 0xsanson Vulnerability details Impact The Controller contract can call converter.convert inside earn and withdraw functions, after transferring amount of tokens to the Converter contract. This contract assumes that it has received exactly amount tokens, however this isn't true for...

Controller.withdraw(...) User may lose funds when withdraw wantToken from the underlying contract

Handle WatchPug Vulnerability details The wantToken of the strategy may be different from the token argument of Controller.withdrawaddress token, uint256 amount according to code at line 469-474 of Controller.sol. if want != token address converter = vaultDetailsmsg.sender.converter;...

Vault: Withdrawal amount isn't un-normalized

Handle hickuphh3 Vulnerability details Impact In withdraw, the withdrawal amount is the proportion of the normalized amounts of all the tokens in the vault and its strategies. However, this amount isn't un-normalized to the output token's decimals, thus leading to an erroneous token amount being...

The function addToken does not check if the token was already added

Handle hrkrshnn Vulnerability details addToken does not check if the token was already added The function addToken does not check if the token was already present. function addToken address vault, address token external override notHalted onlyStrategist requireallowedTokenstoken, "!allowedTokens"...

Controller.withdrawAll sets wrong vault balance

Handle cmichel Vulnerability details The Controller.withdrawAll decreases the vault balance by amount, the want token amount that has been withdrawn from the strategy and transferred to the vault. Note that amount gets overwritten in the convert != address0 branch and is a convert token value...

Vault may not have enough tokens for withdraw

Handle 0xRajeev Vulnerability details Impact There is an assumption in LegacyController.vault that the vault will have enough tokens0 to cover the balance difference. If not, the user may receive less than amount requested and balance funds get lost/locked unless the vault withdraws from the...

Vault.withdraw sometimes burns too many shares

Handle cmichel Vulnerability details The Vault.withdraw function attempts to withdraw funds from the controller if there are not enough in the vault already. In the case the controller could not withdraw enough, i.e., where diff toWithdraw, the user will receive less output tokens than their fair...

Vault.balanceOfThis values all tokens equally

Handle cmichel Vulnerability details The Vault.balanceOfThis function values all tokens equally. They are normalized to 18 decimals and then simply added up: for uint8 i; i tokens.length; i++ address token = tokensi; // adds up different tokens here, treating them as exactly equal value 1.0 A = 1...

PostAuctionLauncher.sol#finalize() Adding liquidity to an existing pool may allows the attacker to steal most of the tokens

Handle WatchPug Vulnerability details PostAuctionLauncher.finalize can be called by anyone, and it sends tokens directly to the pair pool to mint liquidity, even when the pair pool exists. An attacker may control the LP price by creating the pool and then call finalize to mint LP token with unfai...

harvestNextStrategy never executes because lastCalled is 0 and stays 0

Handle gpersoon Vulnerability details Impact The function harvestNextStrategy of Harvester.sol checks canHarvest to make sure it can harvest. Initially strategy.lastCalled will be 0 so canHarvest will return false. Thus the require in harvestNextStrategy fails And it never reaches the point where...

Controller transfer extra token on withdrawing tokens

Handle jonah1005 Vulnerability details Impact The Controlle�r's function withdrawaddress token, uint256 amount should return whatever amount of the token user/vault asks. However, it tries to withdraw strategy.want token and convert it. Take for example, when a user/vault calls withdrawdai, 100,...

Access restrictions on NotionalV1ToNotionalV2.notionalCallback can be bypassed

Handle cmichel Vulnerability details Vulnerability Details The NotionalV1ToNotionalV2.notionalCallback is supposed to only be called from the verified contract that calls this callback but the access restrictions can be circumvented by simply providing sender = this as sender is a parameter of th...

Possible reentrancy in balanceOf, decimals, mint

Handle tensors Vulnerability details Impact Registering tokens that aren't properly vetted can lead to a loss of funds if the token has callbacks. CREAM finance got hacked in a similar way because the ampleforth token had a callback in the transfer method that wasn't noticed when they vetted it...

ERC20Rewards.sol: Consider making rewardsToken immutable

Handle hickuphh3 Vulnerability details Impact While it might seem like a good feature to have, being able to switch reward tokens will only be useful for tokens which are equivalent in value probably stablecoins, pegged tokens since it carries over unclaimed rewards from the previous reward...

TimeLock cannot schedule the same calls multiple times

Handle cmichel Vulnerability details The TimeLock.schedule function reverts if the same targets and data fields are used as the txHash will be the same. This means one cannot schedule the same transactions multiple times. Impact Imagine the delay is set to 30 days, but a contractor needs to be pa...

ERC20Rewards breaks when setting a different token

Handle cmichel Vulnerability details The setRewards function allows setting a different token. Holders of a previous reward period cannot all be paid out and will receive their old reward amount in the new token. This leads to issues when the new token is more less valuable, or uses different...

No ERC20 safe* versions called

Handle cmichel Vulnerability details The claim function performs an ERC20 transfer rewardsToken.transferto, claiming; but does not check the return value, nor does it work with all legacy tokens. Some tokens like USDT don't correctly implement the EIP20 standard and their transfer/transferFrom...

Missing events/timelocks for owner/admin only functions that change critical parameters

Handle 0xRajeev Vulnerability details Impact Owner/admin only functions that change critical parameters should emit events and have timelocks. Events allow capturing the changed parameters so that off-chain tools/interfaces can register such changes with timelocks that allow users to evaluate the...

Difficult for the project to be decentralized if the Watsons share one address.

Handle tensors Vulnerability details Impact The Watsons share a single address. As it stands right now the Watsons could be a single person effectively providing insurance with other peoples risk. There should be mechanisms in place to make sure Watson's have an accurate amount of skin in the gam...

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

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

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

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

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

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

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

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

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

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

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