Swivel.setFee() is implemented wrongly.

Lines of code Vulnerability details Impact Swivel.setFee is implemented wrongly. Swivel.feenominators won't be set as expected. Proof of Concept This function has a parameter "i" for the index of the new fee denomination but it isn't used during the update. Tools Used Solidity Visual Developer of...

minInkOut parameter will likely cause revert

Lines of code Vulnerability details Impact In payBase, the user is able to supply maxBaseIn and minInkOut params. There is logic in the function that if the user has attempted to send more base than the cauldron currently needs, then it only takes the amount of base necessary. // If offering too...

Fee-on-transfer tokens not supported

Lines of code Vulnerability details Impact Incorrect accounting will lead to wrong assets distribution and some users gaining more and some users getting fewer tokens than they should. Proof of Concept Functions rely on user input to calculate distribution of tokens instead of relying on the...

Upgraded Q -> M from 390 [1657876451641]

Judge has assessed an item in Issue 390 as Medium risk. The relevant finding follows: --- The text was updated successfully, but these errors were encountered: All reactions...

Upgraded Q -> M from 390 [1657876456166]

Judge has assessed an item in Issue 390 as Medium risk. The relevant finding follows: --- The text was updated successfully, but these errors were encountered: All reactions...

Accidentally cancel order

Lines of code Vulnerability details Accidentally cancel order Impact Some market maker's limit order might be accidentally cancelled, and has to be resubmitted again. If the user did not do this on purpose, without knowing the fact that the order has been cancelled, it may lead to unexpected...

User can increase migration's proposal total tokens artificially

Lines of code Vulnerability details Impact There are two functions that users can use to withdraw deposited ether/tokens from a joined proposal: leave and withdrawContribution. leave correctly decreases the total contribution from a proposal alongside the user's one: // Updates fraction balances ...

Arbitrary transfers following approvals can lead to loss of funds/NFTS

Lines of code Vulnerability details Impact These three transfer functions allow an attacker to supply an arbitrary from and to to transfer ERC20s, ERC721s, and ERC1155s. The moment that a user sets approval for the contract to spend their tokens, an attacker can front-run the next call and steal...

Proposal commitment takes all fractional balance in Migration contract

Lines of code Vulnerability details Impact The Migration contract holds all fractional token balance of all proposals. Let's suppose a single vault has multiple proposals going on, and one gets committed. If the target price is satisfied, then a buyout starts // Checks if the current price is...

fractionPrice precision can be lost if fractional tokens supply is high enough

Lines of code Vulnerability details Buyout's start now determine fractional token price by dividing native tokens amount by total supply number. Whenever the supply is high enough the precision can be lost, leading to severe losses to buyout proposer as his staked fractional tokens can be valued ...

Don't trust in references contracts

Lines of code Vulnerability details Impact The references contracts don't check anything, and the function receive a lot of parameters to manipulate the call Proof of Concept If somebody approveERC20/777/721/1155 the TransferReference contract or use a delegatecall the tokens can be stolen...

delegatecall() modify merkleRoot, vault may lose all

Lines of code Vulnerability details Impact If the vault contract delegatecall the malicious contract by execute，the important variable merkleRoot can be modified. If the merkleRoot is set well designed, the vault will lose all tokens. Because, there are two main requirements for function execute ...

Migration's withdrawContribution ignores the exchange between fractional tokens and ETH happened during Buyout attempt

Lines of code Vulnerability details withdrawContribution aims to return the funds to Migration participants. However, it uses initial userProposalFractionsproposalIdmsg.sender and userProposalEthproposalIdmsg.sender records for withdrawal accounting. Real funds structure is different after Buyout...

TEST

Lines of code L1 Vulnerability details Impact Detailed description of the impact of this finding. Proof of Concept Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept. Tools Used Recommended Mitigation Steps --- Th...

Buyout's buyFractions can be called multiple time reusing the same msg.value with Multicall

Lines of code Vulnerability details Passing multiple buyFractions calls to Multicall's multicall will use the same msg.value many times. This will inflate his contribution without real fund transfers with the corresponding fund loss for the system. Proof of Concept Buyout uses Multicall: contract...

authRedeem in Marketplace.sol calls nonexistent function

Lines of code Vulnerability details Impact Complete loss of user funds Proof of Concept In L156 and L164 marketplace.sol makes an external call to swivel.authRedeem, but Swivel.sol doesn't contain any function by that name. When calling a nonexistent function in solidity, the call will simply...

cash share amount calculated incorrectly

Lines of code Vulnerability details Impact When a buyout is successful, token owners can cash out their fractional tokens for ETH. The amount of ETH cashed out buyoutShare is calculated like this L268: uint256 buyoutShare = tokenBalance ethBalance / totalSupply + tokenBalance; with tokenBalance t...

div by 0 if user got all the fractions

Lines of code Vulnerability details Impact If you got all the fractions you got a div 0 when calculateContribution in the commit and migrateFractions functions Tools Used Manual Review Recommended Mitigation Steps Add if that returns totalEth if totalFractions == lastTotalSupply function...

Usage of deprecated transfer to send ETH

Lines of code Vulnerability details Impact Transaction can revert Proof of Concept The original transfer used to send ETH uses a fixed stipend of 2300 gas. This was used to prevent reentrancy. However, this limit your protocol to interact with others contracts that need more than that to proceess...

Migration Module: Re-enter commit using custom token

Lines of code Vulnerability details Impact HIGH - Assets can be compromised directly. One can drain eth out from migration module to buyout module using custom made FERC1155 token. Proof of Concept proof of concept: testCommitReenterpoc custom made FERC1155 for the attack The proof of concept sho...

Assets can be transferred to zero address on operational mistake

Lines of code Vulnerability details It is possible to withdraw all the assets after Buyout before settleVault was run and newVault created as asset transfer functions do not check the address. Proof of Concept /// @notice Migrates an ERC-20 token to the new vault after a successful migration ///...

buyoutPrice precision is lost in Buyout's start and Migration's commit

Lines of code Vulnerability details Buyout's start now determine the price for buyout with the truncation to 1% of supply. When buyout initiator brings in the big enough amount of fractional tokens current formula can yield substantial mispricing of initiator's fractional tokens value, which will...

# an attacker can block stuff from the users from being able to do a buyout or sell fractoins or just main functions ,causing grief of users money and time

Lines of code Vulnerability details details uint256 endTime = startTime + PROPOSALPERIOD; if block.timestamp endTime 1.end time is almost block.timestamp so when an user calls this function 2. an attacker can blockstuff and cause the main functions not to happen maybe causing the...

Mismatch in withdraw() between Yearn and other protocols can prevent Users from redeeming zcTokens and permanently lock funds

Lines of code Vulnerability details Impact As defined in the docs for Euler, ERC4626, Compound and Aave, when withdrawing and depositing funds the amount specified corresponds excactly to how many of the underlying assets are deposited or withdrawn. However, as specified by Yearn, the yearn...

Migration Module: The assets can be taken by a failed proposal

Lines of code Vulnerability details Impact HIGH - Assets can be stolen directly When there are multiple proposals for a vault, a failed proposal can withdraw the assets. Proof of Concept proof of concept: testMultiProposalpoc The proof of concept shows a scenario alice is taking assets. 1. setup:...

It is possible to burn someone's vault tokens

Lines of code Vulnerability details Impact It is possible to burn someone's vault tokens. Exploit Scenario Let's say Alice and Bob have some vault tokens. For some reason, Bob didn't grab an ice-cream for Alice, therefore Alice wanted to revenge for that. So she decided to burn Bob's vault tokens...

Anyone can call propose() and create a DOS

Lines of code Vulnerability details A denial of service can be initiated by anyone, since there is no check for any address, and also no Ether/ERC20 at stake. Impact Denial of service for vault migration Proof of Concept Contract : Migration.sol, function propose Recommended Mitigation Steps Make...

Native ETH transfer should use call() instead of transfer()

Lines of code Vulnerability details Impact It might be impossible for some addresses to receive ETH via transfer because receiver address might have methods that exceed 2300 gas, ultimately leading to frozen funds. Proof of Concept Native transfer function has a limit of 2300 gas, which might not...

Plugins can be abused, custom FERC1155 Token can be abused

Lines of code Vulnerability details Impact HIGH - Assets can be stolen/compromised/lost directly. The creator of vault can add any functionality they want by plugins. Also they can bring any tokens for the vault. It can be used against users, or it will make exploits easier to execute. Proof of...

Small scalar may make buyoutPrice calculation incorrectly

Lines of code Vulnerability details Impact In Buyout.start function, buyoutPrice is calculated using a scalar = 100. This small scalar and division rouding will reduce the precision of the formula significantly. Proof of Concept 1. Alice call start with msg.value = 1e18 and depositAmount = 4567...

Vault implementation can be selfdestructed due to lack of initialization

Lines of code Vulnerability details Impact HIGH - Assets can be lost directly Anybody can initialize the Vault's implementation contract. The worst case would be to selfdestruct and make all the already deployed and to be deployed Vault's proxies useless and assets in the deployed proxies will be...

Buyout Module: ethBalance is not properly updated

Lines of code Vulnerability details Impact HIGH - Assets can be stolen directly. An attacker can steal eth from buyout module Proof of Concept proof of concept1: testCashSharepoc proof of concept2: testCashRepeatpoc The proof of concept1 shows that the same amount of fractions will result in...

Call() should be used instead of transfer() on an address payable

Lines of code Vulnerability details Impact Detailed description of the impact of this finding. Proof of concept Migration.sol L 172 Tools Used Recommended Mitigation Steps Use call --- The text was updated successfully, but these errors were encountered: All reactions...

Migration's leave and withdrawContribution use payable.transfer calls with an arbitrary receiver

Lines of code Vulnerability details Migration's leave and withdrawContribution transfer out native tokens via payableto.transfer call. This is unsafe as transfer has hard coded gas budget and can fail when msg.sender is a smart contract. Such transactions will fail for smart contract users which...

Migration can be impacted, by frontrunning the commit function call

Lines of code Vulnerability details As part of the migration of a vault, the commit function is called to kickoff the buyout process for the migration. Its possible that any EOA address can initiate a start function on the Buyout module with a very low amount of msg.value and prevent the above...

feenominators are set incorrectly through setFee function

Lines of code Vulnerability details Impact feenominators supposed to be updated using the array of indexes i not the current position of array length d Proof of Concept The function updates the fenominatorsx instead of feenominatorsix for uint256 x; x len; if dx MINFEENOMINATOR revert Exception18...

Not check fractionPrice > 0 may allow users to buy or sell fractions with zero ETH

Lines of code Vulnerability details Impact In Buyout.start function, fractionPrice is calculated from msg.value and depositAmount, both can be changed by sender. And there is no check if fractionPrice is bigger than 0. So in case fractionPrice = 0, users may sell and buy fractions with zero ETH...

User can block migration actions by starting buyout process

Lines of code Vulnerability details Impact All migration actions such as propose, join, leave, and commit require that the pool's buyout state is INACTIVE. At any point, a user can call buyout.Start to start the buyout process which will change the buyout state to LIVE. Proof of Concept...

Migration total supply reduction can be used to remove minority shareholders

Lines of code Vulnerability details As new total supply can be arbitrary, setting it significantly lower than current say to 100 when it was 1e9 before can be used to remove current minority shareholders, whose shares will end up being zero on a precision loss due to low new total supply value...

Loss of funds in an underlying protocol would cause catostrophic loss of funds for swivel

Lines of code Vulnerability details Impact Loss of all user funds Proof of Concept This exploit stems from a quirk in the way that exchange rate is tracked for matured positions. We first need to breakdown how interest is calculate for a matured position. In L124 the yield for a matured position ...

Upgraded Q -> H from 187 [1657760940788]

Judge has assessed an item in Issue 187 as High risk. The relevant finding follows: 1. rescueETH cannot rescue Ether rescueETH sends msg.value to the destination address, which means it requires the caller of rescueETH to provide the Ether to send. Essentially the owner is directly paying the...

The FERC1155.sol don't respect the EIP2981

Lines of code Vulnerability details Impact The EIP-2981: NFT Royalty Standard implementation is incomplete, missing the implementation of function supportsInterfacebytes4 interfaceID external view returns bool; from the EIP-165: Standard Interface Detection Proof of Concept A marketplace...

ERC20 Incorrect check on returnedAddress in permit() results in unlimited approval of zero address

Lines of code Vulnerability details Impact When creating ERC20.sol from Solmate, a require in permit was converted to a custom error incorrectly. It now reads: if recoveredAddress != address0 && recoveredAddress != owner revert Invalidmsg.sender, owner; So if the recoveredAddress is non-zero and...

dont use transfer ,instead use call

Lines of code Vulnerability details details To withdraw eth it uses transfer, this trnansaction will fail inevitably when : - The withdrwer smart contract does not implement a payable function. Withdrawer smart contract does implement a payable fallback which uses more than 2300 gas unit Thw...

Able to call withdrawContributions with any vaultId may lead to loss of funds

Lines of code Vulnerability details Impact The withdrawContributions function in Migration.sol takes any vault as input. As long as the vault is valid and has an inactive buyout, a user may call withdrawContributions even if the proposal they contributed to is LIVE. This may lead to users not bei...

Committed Unsuccessful Migration can be settled later on

Lines of code Vulnerability details Impact When committing a migration proposal, it's status is changed to isCommited = true and the buyout starts. After 4 days, the buyout ends. Considering the case it was rejected, its status goes to INACTIVE. Nothing happens to the proposal in the Migration...

function mint() in FERC1155 don't follow check-effect-interact pattern, it's possible to call protocol contracts after tokens minted and before totalSupply updated

Lines of code Vulnerability details Impact Function mint mints new fractions for an ID and is only callable by VaultRegistry. code mints tokens then updates totalSupply value. when minting contract may make external call to target address, in that external call contract state is wrong, tokens are...

Any token approved for the base vault can be stolen

Lines of code Vulnerability details Impact All tokens approved for the BaseVault contract can be stolen by any attacker. The batchDeposit functions lack proper access controls and allow the token source to be an arbitrary address as opposed to restricting it to msg.sender. Since token approvals...

Royalty percentage in FERC1155 can be set to a value greater than 100%

Lines of code Vulnerability details Impact Function setRoyalties does not check that the value of percentage is less or equal to 100. Function royaltyInfo in L247 could return a royaltyAmount greater than salePrice. Proof of Concept This simple test shows how the controller can set a value of...

block.timestamp used as time proxy

Lines of code Vulnerability details block.timestamp used as time proxy Impact a. Summary: Risk of using block.timestamp for time should be considered. b. Details: block.timestamp is not an ideal proxy for time because of issues with synchronization, miner manipulation and changing block times. In...