Contract BasicSale is missing an approve(address(vestLock), 2**256-1) call

Handle Reigada Vulnerability details Impact As we can see in the contracts AirdropDistribution and InvestorDistribution, they both have the following approve call: mainToken.approveaddressvestLock, 2256-1; This is necessary because both contracts transfer tokens to the vesting contract by calling...

Claim airdrop repeatedly

Handle gpersoon Vulnerability details Impact Suppose someone claims the last part of his airdrop via claimExact of AirdropDistribution.sol Then airdropmsg.sender.amount will be set to 0. Suppose you then call validate again. The check "airdropmsg.sender.amount == 0" will allow you to continue,...

Can't claim last part of airdrop

Handle gpersoon Vulnerability details Impact Suppose you are eligible for the last part of your airdrop or your entire airdrop if you haven't claimed anything yet. Then you call the function claim of AirdropDistribution.sol, which has the following statement: "assertairdropmsg.sender.amount -...

Invalid validity check

Handle fr0zn Vulnerability details Vulnerability Details On the AirdropDistribution.sol, inside the validate function line 506 , the validatedmsg.sender is not checked and the validation for an already airdropped user is done using the airdropmsg.sender.amount value. Provide a detailed descriptio...

Invalid or missing validity check

Handle fr0zn Vulnerability details Vulnerability Details On the AirdropDistribution.sol, inside the validate function line 506 , the validatedmsg.sender is not checked and the validation for an already airdropped user is done using the airdropmsg.sender.amount value. Provide a detailed descriptio...

DOS attack possible for Token->ETH trades

Handle pmerkleplant Vulnerability details Impact Any user's trade from some token to a native token e.g. ETH can be DOSed if an attacker sends native tokens to the Executioner contract. If the attacker watches for trades, he/she could either make the whole trade fail or, depending on the...

Slingshot: Incorrect initial balance fetched for native token in executeTrades()

Handle hickuphh3 Vulnerability details Impact The executioner contract only supports ERC20ERC20 token trades. Native token swaps are supported by either wrapping / unwrapping the ERC20 wrapped native token before / after the trades respectively. When exchanging from the native token, the wrapping...

Inconsistent balance comparison of nativeToken in executeTrades

Handle kenzo Vulnerability details When toToken == nativeToken, executeTrades compares Executioner's starting nativeToken balance to Executioner's ending wrappedNativeToken balance. Impact Loss of user funds or DOS of executeTrades: if there is extra ETH in Executioner contract, finalOutputAmount...

Trades where toToken is feeOnTransferToken might send user less tokens than finalAmountMin

Handle kenzo Vulnerability details Slingshot's executeTrades checks that the trade result amount to be sent to the user is bigger than finalAmountMin, and after that sends the user the amount. But if the token charges fee on transfer, the final transfer to the user will decrease the amount the us...

Admin can abuse grantSlingshot and steal user funds

Handle kenzo Vulnerability details After user has .approved ApprovalHandler, admin can grantSlingshot himself, and then call ApprovalHandler.transferFrom with parameters that will transfer all tokens to himself before the user calls Slingshot's executeTrades. Although this vulnerability requires ...

Contract Selfdestruct via delegatecall

Handle elprofesor Vulnerability details Impact Using a malicious Module the ModuleRegistry admin can trigger a selfdestruct via delegate call in the Executioner.sol contract. ModuleRegistry and Executioner separate the logic between ModuleRegistry.sol admin and systems admin. Executioner.sol reli...

Privilege Escalation via Delegate Call

Handle elprofesor Vulnerability details Impact Using a malicious Module the ModuleRegistry admin can escalate to System admin via delegate call in the Executioner.sol contract. ModuleRegistry and Executioner separate the logic between ModuleRegistry.sol admin and systems admin. The systems admin ...

initialBalance for native token is wrong

Handle WatchPug Vulnerability details function executeTrades address fromToken, address toToken, uint256 fromAmount, TradeFormat calldata trades, uint256 finalAmountMin, address depricated external nonReentrant payable depricated; requirefinalAmountMin 0, "Slingshot: finalAmountMin cannot be zero...

WrappedIbbtcEth.sol Does Not Incentivize Price Updates

Handle leastwood Vulnerability details Impact The live ibBTC price for each share is cached to reduce gas costs of mint, burn and transfer operations. The updatePricePerShare function is intended to be called on a regular interval, however, there is no incentive that this value is regularly...

Front-running displacement to beat oracle updates for price manipulation

Handle hack3r-0m Vulnerability details For off-chain oracle, consider: A broadcasted transaction tx1 from a trusted oracle/s changes the price from X to X + d or X -d, this knowledge of update can be harmful and leveraged by malicious actors. If one had to transfer Y amount at current state after...

executeTrades is payable and doesn't use msg.value

Handle pants Vulnerability details The function executeTrades is payable and doesn't use msg.value is payable and doesn't use msg.value. --- The text was updated successfully, but these errors were encountered: All reactions...

Regular Trades Can Drain Executioner.sol Balance

Handle zer0dot Vulnerability details Impact This vulnerability allows any trade to effectively drain the balance, as long as it is not the chain's native asset which is not used directly in the modules , from the Executioner.sol contract. This is technically not critical because funds are not...

Incorrect calculation of initialBalance in Slingshot.executeTrades()

Handle daejunpark Vulnerability details Impact The Slingshot.executeTrades incorrectly calculates initialBalance when toToken == nativeToken. It should have been the balance of wrapped native tokens e.g., WETH, rather than that of native currencies e.g., Ether. This incorrect behavior introduces...

Leftover balance in the Executioner contract can be drained

Handle gzeon Vulnerability details Impact Leftover balance in the Executioner contract can be drained by swapping the target assetnative/erc20 into another asset. Slingshot.executeTrades allow user to execute trade using modules as long as the module is registered in the ModuleRegistry. The...

No sanity check on pricePerShare might lead to lost value

Handle kenzo Vulnerability details pricePerShare is read either from an oracle or from ibBTC's core. If one of these is bugged or exploited, there are no safety checks to prevent loss of funds. Impact As pricePerShare is used to calculate transfer amount, a bug or wrong data which returns smaller...

WrappedIbbtc and WrappedIbbtcEth contracts do not filter out price feed outliers

Handle hyh Vulnerability details Impact If price feed is manipulated in any way or there is any malfunction based volatility on the market, both contracts will pass it on a user. In the same time it's possible to construct mitigation mechanics for such cases, so user economics be affected by...

WrappedIbbtcEth.sol Sanity check of pricePerShare should be enforced

Handle WatchPug Vulnerability details /// @dev Update live ibBTC price per share from core /// @dev We cache this to reduce gas costs of mint / burn / transfer operations. /// @dev Update function is permissionless, and must be updated at least once every X time as a sanity check to ensure value ...

transferFrom amount may exceed allowance

Handle WatchPug Vulnerability details function transferFromaddress sender, address recipient, uint256 amount public virtual override returns bool /// @dev the balances mapping represents the underlying ibBTC shares "non-rebased balances" /// @dev the naming confusion is due to maintaining origina...

Null check in pricePerShare

Handle hack3r-0m Vulnerability details oracle can 0 as a price of the share, in that case, 0 will be the denominator in some calculations which can cause reverts from SafeMath for e.g here: resulting in Denial Of Service. Add a null check to ensure that on every update, the price is greater than ...

pricePerShare might become stale and cause to lose user funds, as update is not enforced

Handle kenzo Vulnerability details In WrappedIbbtcEth, as the code says, updatePricePerShare must be called once in a while to ensure pricePerShare is up to date. However the contract is not enforcing or validating it in any way. If for example the external server that runs the periodic update...

The design of wibBTC is not fully compatible with the current Curve StableSwap pool

Handle WatchPug Vulnerability details Per the documentation, wibBTC is designed for a Curve StableSwap pool. However, the design of wibBTC makes the balances change dynamically and automatically. This is unusual for an ERC20 token, and it's not fully compatible with the current Curve StableSwap...

WrappedIbbtcEth contract will use stalled price for mint/burn if updatePricePerShare wasn't run properly

Handle hyh Vulnerability details Impact Malicious user can monitor SetPricePerShare event and, if it was run long enough time ago and market moved, but, since there were no SetPricePerShare fired, the contract's pricePerShare is outdated, so a user can mint with pricePerShare that is current for...

pricePerShare is only lazily updated

Handle cmichel Vulnerability details The pricePerShare value is cached in updatePricePerShare and this value can be old and not reflect the actual core.pricePerShare value. The same issue exists on the sidechain variant when oracle price updates happen infrequently. This leads to potentially big...

approvalHandler.transferFrom(..) isn't safe

Handle pants Vulnerability details approvalHandler can be set by the admin to any ERC20 token. A popular possible token is USDT, for example, which the admin can use. Then ignoring transferFrom return value isn't safe - since USDT transferFrom doesn't revert on failure but returns false instead...

Approved spender can spend too many tokens

Handle cmichel Vulnerability details The approve function has not been overridden and therefore uses the internal shares, whereas transferFrom uses the rebalanced amount. Impact The approved spender may spend more tokens than desired. In fact, the approved amount that can be transferred keeps...

initialBalance and finalBalance calculated in a different way

Handle gpersoon Vulnerability details Impact The function executeTrades of Slingshot.sol calulates the initialBalance and finalBalance in a different way. If toToken == nativeToken then: initialBalance == executioner.balance; finalBalance == wrappedNativeToken.balanceOfaddressexecutioner This cou...

Lack Of Return Value Check On the Oracle

Handle defsec Vulnerability details Impact During the code review, It has been seen that oracle return value has not been checked on the function. If oracle is returned price as a 0, balance of shares will be zero. Proof of Concept 1. Navigate to "" 2. The return value pricePerShare function has...

hard to clear balance

Handle jonah1005 Vulnerability details Impact The contract does not allow users to transfer by share. It's hard for users to clear out all the shares. There will be users using this token with Metamask. There's likely the pricePerShare would increase after the user sends transactions. I consider...

Allowance is consistent in different functions

Handle jonah1005 Vulnerability details Impact Allowance in the contract stands for shares while the other functions use the balance. When a user tries to deposit 100 ibBTC to a contract, he calls approve, 100. The allowance would not be zero after the contract takes 100 ibBTC from the user. Users...

Ethereum wibBTC price per share dependant on being updated every X time, but contract has no checks whether price is updated

Handle loop Vulnerability details The price of pricePerShare in WrappedIbbtcEth.sol is dependant on two things: The pricePerShare of core. updatePricePerShare being invoked every X time to update the pricePerShare of wibBTC. The only time updatePricePerShare is invoked inside the contract is duri...

Profit from difference between core.pricePerShare() and pricePerShare()

Handle gpersoon Vulnerability details Impact Suppose there is a small difference between core.pricePerShare and pricePerShare of WrappedIbbtcEth.sol on Eth main chain. This could be due to the fact that updatePricePerShare hasn't been called for a while, or a sudden change in core.pricePerShare. ...

Token is not ERC20 compliant (potential DOS of other contracts by oracle)

Handle TomFrench Vulnerability details Impact calling transferFrom with amount will reduce the callers allowance by an amount other than amount. This under some circumstances can result in funds being locked in external contracts which perform exact approvals. Proof of Concept As a rebasing wrapp...

MochiTreasuryV0.sol Is Unusable In Its Current State

Handle leastwood Vulnerability details Impact MochiTreasuryV0.sol interacts with Curve's voting escrow contract to lock tokens for 90 days, where it can be later withdrawn by the governance role. However, VotingEscrow.vy does not allow contracts to call the following functions; createlock,...

Unable to transfer WrappedIbbtc if Oracle go down

Handle gzeon Vulnerability details Impact In WrappedIbbtc, user will not be able to transfer if oracle.pricePerShare L124 revert. This is because balanceToShares is called in both transfer and transferFrom, which included a call to pricePerShare. If this is the expected behavior, note that...

Curve Metapool does not support rebasing token

Handle gzeon Vulnerability details Impact For interest baring token, Curve use another version of the contract with consideration of underlying asset. The metapool contracts does not support such behaviour. Proof of Concept Actually it is written in Curve's doc Token balances must not change...

Inconsistent ERC20 function may lead to DOS and unexpected behaviour

Handle gzeon Vulnerability details Impact There are 2 important yet similar concept in this wrapper contract, namely "SHARE" and "BALANCE". SHARE equivalent to the number of underlying ibbtc and is stored in balance as the native value, while BALANCE is SHAREpricepershare and is the return value ...

transferFrom function can be frontrun and unusable

Handle pants Vulnerability details A malicious program can frontrun every transaction of transferFromfrom, to, amount where amount is the maximal allowance by frontrunning with transferFromfrom, to, amount/10000 and then the first transaction will fail and only amount / 10000 will be transferred...

MochiTreasuryV0.withdrawLock() Is Callable When Locking Has Been Toggled

Handle leastwood Vulnerability details Impact withdrawLock does not prevent users from calling this function when locking has been toggled. As a result, withdraws may be made unexpectedly. Proof of Concept Tools Used Manual code review Recommended Mitigation Steps Consider adding requirelockCrv,...

ReferralFeePoolV0.sol#claimRewardAsMochi() Array out of bound exception

Handle WatchPug Vulnerability details function claimRewardAsMochi external IUSDM usdm = engine.usdm; address memory path = new address; path0 = addressusdm; path1 = uniswapRouter.WETH; path2 = addressengine.mochi; usdm.approveaddressuniswapRouter, rewardmsg.sender; // we are going to ingore the...

MochiVault.sol Potential griefing attack by depositing 0 to target's position

Handle WatchPug Vulnerability details MochiVault.solwithdraw is using the wait modifier to prevent withdraw within delay duration from lastDeposit. However, MochiVault.soldeposit allows anyone to deposit to a specific position. This enables the attacker to initiate a griefing attack by depositing...

Tokens Can Be Stolen By Frontrunning VestedRewardPool.vest() and VestedRewardPool.lock()

Handle leastwood Vulnerability details Impact The VestedRewardPool.sol contract is a public facing contract aimed at vesting tokens for a minimum of 90 days before allowing the recipient to withdraw their mochi. The vest function does not utilise safeTransferFrom to ensure that vested tokens are...

Missing slippage checks

Handle cmichel Vulnerability details The contracts are missing slippage checks which can lead to being vulnerable to sandwich attacks. A common attack in DeFi is the sandwich attack. Upon observing a trade of asset X for asset Y, an attacker frontruns the victim trade by also buying asset Y, lets...

Uniswap / Sushiswap prices can be manipulated through flashloans

Handle cmichel Vulnerability details The UniswapV2CSSR.getExchangeRatio uses the current reserve to derive the exchange ratio. The fact that it mixes in historic data does not matter because it still uses the current reserves which can be manipulated through flashloans in currentPriceCumulative...

claimRewardAsMochi will produce a runtime error

Handle pauliax Vulnerability details Impact function claimRewardAsMochi in ReferralFeePoolV0 will produce a runtime exception because the length of the path is 2 but it tries to assign 3 entries: address memory path = new address; path0 = addressusdm; path1 = uniswapRouter.WETH; path2 =...

FeePoolV0.sol#distributeMochi() will unexpectedly flush treasuryShare, causing the protocol fee cannot be properly accounted for and collected

Handle WatchPug Vulnerability details distributeMochi will call buyMochi to convert mochiShare to Mochi token and call shareMochi to send Mochi to vMochi Vault and veCRV Holders. It wont touch the treasuryShare. However, in the current implementation, treasuryShare will be reset to 0. This is...