Changing engine.nft contract breaks vaults

Handle cmichel Vulnerability details Governance can change the engine.nft address which is used by vaults to represent collateralized debt positions CDP. When minting a vault using MochiVault.mint the address returned ID will be used and overwrite the state of an existing debt position and set it...

Unhandled return values of transfer

Handle WatchPug Vulnerability details ERC20 implementations are not always consistent. Some implementations of transfer could return ‘false’ on failure instead of reverting. It is safer to wrap such calls into require statements to these failures. Unsafe transfer calls were found in the following...

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

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

Chainlink Adapter Missing Validation Of latestRoundData() Outputs

Handle leastwood Vulnerability details Impact ChainlinkAdapter.getPrice queries a Chainlink oracle to retrieve the latest price for a given asset. However, this external call does not validate the data retrieved is fresh. Proof of Concept Tools Used Manual code review Recommended Mitigation Steps...

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

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

UniswapV2TokenAdapter does not support Sushiswap-only assets

Handle cmichel Vulnerability details The UniswapV2TokenAdapter.supports function calls its aboveLiquidity function which returns the UniswapV2 liquidity if the pair exists. If this is below minimumLiquidity, the supports function will return false. However, it could be that the Sushiswap pair has...

ERC20 return values not checked

Handle cmichel Vulnerability details The ERC20.transfer and ERC20.transferFrom functions return a boolean value indicating success. This parameter should checked for success. See VestedRewardPool.claim which performs ERC20 transfers without checking for the return value. Impact As the trusted moc...

MochiProfileV0.registerAsset() Can Reset Existing Asset Classes

Handle leastwood Vulnerability details Impact registerAsset is used to add assets to the Mochi Protocol. These assets have an associated asset class which represents an asset's risk factor. registerAsset can be called by any user and abused in such a way that existing assets can have their...

DutchAuctionLiquidator.settleLiquidation() Does Not Operate On Non-Standard ERC20 Tokens

Handle leastwood Vulnerability details Impact settleLiquidation does not validate the return value of an ERC20 transfer call. As a result, if the call to transfer does not revert but instead returns a boolean or void value, it may be possible that a user liquidates a user's position but does not...

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

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

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

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

MochiCSSRv0.update() Does Not Operate Correctly On Bluechip Assets

Handle leastwood Vulnerability details Impact Mochi vaults query price feeds for updates via the update function in MochiCSSRv0.sol. If the asset to be queried is a bluechip asset, the function will call getPrice on an adapter which adheres to the ICSSRAdapter interface. If the adapter is not...

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

Liquidated positions can potential have non-zero debts, making users who deposit to the positions suffer from unfair debt

Handle WatchPug Vulnerability details In the current implementation, new borrows will be charged a 0.5% interest right away. Making the borrower be recorded a 100.5% amount of debt. However, when a position got liquidated, the unrealized interest may still remain in the position while the...

ChainLink price data could be stale

Handle cmichel Vulnerability details There is no check in ChainlinkAdapterEth.getPrice if the return values indicate stale data. This could lead to stale prices according to the Chainlink documentation: under current notifications: "if answeredInRound roundId could indicate stale data." under...

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

Chainlink price data could be stale

Handle WatchPug Vulnerability details function getPriceaddress asset public view override returns float memory , int256 price, , , = feedasset.latestRoundData; uint256 decimalSum = feedasset.decimals + IERC20Metadataasset.decimals; if decimalSum 18 return float numerator: uint256price, denominato...

A malicious user can potentially escape liquidation by creating a dust amount position and trigger the liquidation by themself

Handle WatchPug Vulnerability details In the current implementation, a liquidated position can be used for depositing and borrowing again. However, if there is a liquidation auction ongoing, even if the position is now liquidatable, the call of triggerLiquidation will still fail. The liquidator...

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

treasuryShare is Overwritten in FeePoolV0._shareMochi()

Handle leastwood Vulnerability details Impact The FeePoolV0.sol contract accrues fees upon the liquidation of undercollaterised positions. These fees are split between treasury and vMochi contracts. However, when distributeMochi is called to distribute mochi tokens to veCRV holders, both mochiSha...

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

One auction at a time would lead to bad debt

Handle jonah1005 Vulnerability details One auction at a time would lead to bad debt Impact The current DutchAuctionLiquidator only allows one auction for a position at a time. This is not a desirable design. If a position is liquidated due to the price movement of collaterals, the position would...

Vault fails to track debt correctly that leads to bad debt

Handle jonah1005 Vulnerability details Impact It's similar to the issue "misuse amount as increasing debt in the vault contract". Similar issue in a different place that leads to different exploit patterns and severity. When users borrow usdm from a vault, the debt increased by the amount 1.005...

Liquidation will never work with non-zero discounts

Handle harleythedog Vulnerability details Impact Right now, there is only one discount profile in the github repo: the "NoDiscountProfile" which does not discount the debt at all. This specific discount profile works correctly, but I claim that any other discount profile will result in liquidatio...

Anyone can extend withdraw wait period by depositing zero collateral

Handle harleythedog Vulnerability details Impact In MochiVault.sol, the deposit function allows anyone to deposit collateral into any position. A malicious user can call this function with amount = 0, which would reset the amount of time the owner has to wait before they can withdraw their...

Unchecked ERC20 transfer calls

Handle loop Vulnerability details ERC20 transfer and transferFrom calls normally return true on a succesful transfer. In DutchAuctionLiquidator the call asset.transfermsg.sender, collateral; is made. asset refers to whichever ERC20 asset is used for the vault of that auction. If asset is an ERC20...

treasury is vulnerable to sandwich attack

Handle jonah1005 Vulnerability details treasury is vulnerable to sandwich attack. Impact There's a permissionless function veCRVlock in MochiTreasury. Since everyone can trigger this function, the attacker can launch a sandwich attack with flashloan to steal the funds. MochiTreasuryV0.solL73-L94...

Changing NFT contract in the MochiEngine would break the protocol

Handle jonah1005 Vulnerability details Impact MochiEngine allows the operator to change the NFT contract. MochiEngine.solL91-L93 All the vaults would point to a different NFT address. As a result, users would not be access their positions. The entire protocol would be broken. IMHO, A function tha...

feePool is vulnerable to sandwich attack.

Handle jonah1005 Vulnerability details Impact There's a permissionless function distributeMochi in FeePoolV0. Since everyone can trigger this function, an attacker can launch a sandwich attack with flashloan to steal the funds. FeePoolV0.solL55-L62 The devs have mentioned this concern in the...

registerAsset would change asset class of existing asset

Handle jonah1005 Vulnerability details Impact The permissionless function registerAsset only checks current liquidity. There's no sanity check whether the asset has registered as another asset class. An attacker can set an asset into AssetClass.Sigma. Unexpected liquidation would happen if an Alp...

Referrer can drain ReferralFeePoolV0

Handle gzeon Vulnerability details Impact function claimRewardAsMochi in ReferralFeePoolV0.sol did not reduce user reward balance, allowing referrer to claim the same reward repeatedly and thus draining the fee pool. Proof of Concept L28-47 did not reduce user reward balance Tools Used None...

registerAsset() can overwrite _assetClass value

Handle gpersoon Vulnerability details Impact Everyone can call the function registerAsset of MochiProfileV0.sol Assuming the liquidity for the asset is sufficient, registerAsset will reset the assetClass of an already registered asset to AssetClass.Sigma. When the assetClass is changed to...

debts calculation is not accurate

Handle gpersoon Vulnerability details Impact The value of the global variable debts in the contract MochiVault.sol is calculated in an inconsistent way. In the function borrow the variable debts is increased with a value excluding the fee. However in repay and liquidate it is decreased with the...

Usage of transfer

Handle cmichel Vulnerability details The address.transfer function is used in sweepFees and and swapByQuote to send ETH to an account. It is restricted to a low amount of GAS and might fail if GAS costs change in the future or if feeRecipient is a smart contract and its fallback function handler...

Transfer function is unreliable

Handle Koustre Vulnerability details Impact Opcode pricing is not stable and should not be relied upon to protect against re-entrancy attacks. Proof of Concept Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept...

Change require conditions can prevent fund loss when called with mistaken input data

Handle WatchPug Vulnerability details function swapByQuote address zrxSellTokenAddress, uint256 amountToSell, address zrxBuyTokenAddress, uint256 minimumAmountReceived, address zrxAllowanceTarget, address payable zrxTo, bytes calldata zrxData, uint256 deadline external payable whenNotPaused...

Wrong calculation of erc20Delta and ethDelta

Handle WatchPug Vulnerability details function fillZrxQuote IERC20 zrxBuyTokenAddress, address payable zrxTo, bytes calldata zrxData, uint256 ethAmount internal returns uint256, uint256 uint256 originalERC20Balance = 0; if!signifiesETHOrZeroaddresszrxBuyTokenAddress originalERC20Balance =...

use of transfer() instead of call() to send eth

Handle JMukesh Vulnerability details Impact Use of transfer might render ETH impossible to withdraw becuase after istanbul hardfork , there is increases in the gas cost of the SLOAD operation and therefore breaks some existing smart contracts.Those contracts will break because their fallback...

Arbitrary contract call allows attackers to steal ERC20 from users' wallets

Handle WatchPug Vulnerability details function fillZrxQuote IERC20 zrxBuyTokenAddress, address payable zrxTo, bytes calldata zrxData, uint256 ethAmount internal returns uint256, uint256 uint256 originalERC20Balance = 0; if!signifiesETHOrZeroaddresszrxBuyTokenAddress originalERC20Balance =...

fillZrxQuote doesn't return correct values when zrxSellTokenAddress == zrxBuyTokenAddress

Handle harleythedog Vulnerability details Impact Suppose that swapByQuote is called with zrxSellTokenAddress == zrxBuyTokenAddress, and neither of these addresses "signifiesETHOrZero". The contract first transfers amountToSell of these tokens from the sender's account into the contract and update...

Swap.sol implements potentially dangerous transfer

Handle elprofesor Vulnerability details Impact The use of .transfer in Swap.sol may have unintended outcomes on the eth being sent to the receiver. Eth may be irretrievable or undelivered if the msg.sender or feeRecipient is a smart contract. Funds can potentially be lost if; 1. The smart contrac...

Unused ERC20 tokens are not refunded

Handle WatchPug Vulnerability details Based on the context and comments in the code, we assume that it's possible that there will be some leftover sell tokens, not only when users are selling unwrapped ETH but also for ERC20 tokens. However, in the current implementation, only refunded ETH is...

transfer() is not recommended for sending ETH

Handle WatchPug Vulnerability details Since the introduction of transfer, it has typically been recommended by the security community because it helps guard against reentrancy attacks. This guidance made sense under the assumption that gas costs wouldn’t change. It's now recommended that transfer...

.transfer is used for transferring ether

Handle pauliax Vulnerability details Impact payablemsg.sender.transfertoTransfer; feeRecipient.transferaddressthis.balance; It is no longer recommended to use .transfer when sending ether as recipients with custom fallback functions smart contracts will not be able to handle that. You can read mo...

Users can avoid paying fees for ETH swaps

Handle pants Vulnerability details Users can call Swap.swapByQuote to execute an ETH swap where they receive ETH without paying swap fee for the gained ETH. They can trick the system by setting zrxBuyTokenAddress to an address of a malicious contract and making it think they have executed an ERC2...