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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

anyone can create a vault by directly calling the factory

Handle jonah1005 Vulnerability details Impact MochiVaultFactory.solL26-L37 There's no permission control in the vaultFactory. Anyone can create a vault. The transaction would be reverted when the government tries to deploy such an asset. As the protocol checks whether the vault is a valid vault b...

Improper Validation Of create2 Return Value

Handle leastwood Vulnerability details Impact The BeaconProxyDeployer.deploy function is used to deploy lightweight proxy contracts that act as each asset's vault. The function does not revert properly if there is a failed contract deployment or revert from the create2 opcode as it does not...

Avoid Using Block.timestamp in checkClaimable

Handle Koustre Vulnerability details Impact checkClaimable function is dependent on using block.timestamp and this can be manipulated to claim funds earlier than it should in functions claim, forceClaim and lock Proof of Concept Provide direct links to all referenced code in GitHub. Add...

UniswapV2/SushiwapLPAdapter update the wrong token

Handle cmichel Vulnerability details The UniswapV2LPAdapter/SushiswapV2LPAdapter.update function retrieves the underlying from the LP token pair asset but then calls router.updateasset, proof which is the LP token itself again. This will end up with the router calling this function again...

Lack of data validation in update function

Handle Koustre Vulnerability details Impact There is no data validation of the data input into update function in MochiCSSRv0.sol. Allowing anyone to set the price of any non ERC20 token that is not a bluechip asset DAI, WETH, ETH, etc. Proof of Concept Provide direct links to all referenced code...

Debt accrual is path-dependant and inaccurate

Handle cmichel Vulnerability details The total debt in MochiVault.accrueDebt increases by the current debt times the debt index growth. This is correct but the total debt is then reduced again by the calling user's discounted debt, meaning, the total debt depends on which specific user performs t...

liquidation factor < collateral factor for Sigma type

Handle cmichel Vulnerability details The MochiProfileV0 defines liquidation and collateral factors for different asset types. For the AssetClass.Sigma type, the liquidation factor is less than the collateral factor: function liquidationFactoraddress asset public view override returns float memory...

Possible to restart liquidation process after liquidation is settled

Handle harleythedog Vulnerability details Impact In the triggerLiquidation function, there is a require statement as follows: requireauction.startedAt == 0 || auction.boughtAt != 0, "on going"; However, after an auction has been settled, we have auction.boughtAt != 0, so this require statement wi...