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

ISwivel.authRedeem() doesn't have an implementation in Swivel.sol

Lines of code Vulnerability details Impact ZcToken.withdraw and ZcToken.redeem will always revert because Swivel.sol doesn't contain authRedeem function. Proof of Concept ZcToken.withdraw and ZcToken.redeem call MarketPlace.authRedeem and ISwivelswivel.authRedeem isn't implemented. I think...

IF A SINGLE ORDER FAILS, THE WHOLE FUNCTION REVERTS

Lines of code Vulnerability details Impact If a single order of the orders array fails, the whole function initiate or exit reverts with no information about which was the vulnerable order, the only information given is an Exception with a code of 5 Proof of Concept Tools Used editor Recommended...

Upgraded Q -> M from 340 [1657877639047]

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

zcTokens cannot be redeemed through authRedeemZcToken()

Lines of code Vulnerability details Impact The description of the function says "Allows users to redeem zcTokens and withdraw underlying, boiling up from the zcToken instead of starting on Swivel". In order for the function to be called, it needs to pass the modifier authorizedmarketPlace, where...

With most functions in VaultTracker.sol, users can call them only once after maturity has been reached.

Lines of code Vulnerability details Impact With most functions in VaultTracker.sol, users can call them only once after maturity has been reached. So from the second call of any functions after maturity, it will revert and users might lose their funds or interests. Proof of Concept The main probl...

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

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

Use of deprecated transfer function to send ETH

Lines of code Vulnerability details Appears in: Migration::leave, Migration::withdrawContribution Vulnerability details Using payableaddress.transfer has been deprecated in favor of using .callvalue:..."" as the proper way of sending ETH. Using transfer or send will make transactions fail when th...

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

Buyout Module: redeeming before the update of totalSupply will make buyout's current state success

Lines of code Vulnerability details Impact MED - a hypothetical attack path with stated assumptions, but external requirements. Attacker can create a vault with successful buyout status and non zero supply. The attacker can sell the fractions and then simply withdraw the assets. Proof of Concept...

Migration Module: After successful migration, ERC20 assets can be thrown away by anyone

Lines of code Vulnerability details Impact HIGH - Assets can be lost directly After proposal and proposed buyout is successful, anyone can transfer ERC20 asset in the vault to the zero address and the asset will be lost. Proof of Concept proof of concept: testAnyoneCanThrowERC20poc The proof of...

Not update ethBalance after users cash out in Migration may cause loss of funds

Lines of code Vulnerability details Impact In Buyout.cash function, users will burn their fractions to cash out ETH. The ETH amount is proportionate the number of fractions they have. So when someone burn fractions, total amount of ETH ethBalance should be updated accordingly. But in cash functio...

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

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

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

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

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

call() should be used instead of transfer() on address payable

Lines of code Vulnerability details call should be used instead of transfer on address payable Impact The use of the deprecated transfer function for an address wll make the transaction fail when 1. The withdrawer smart contract does not implement a payable function. 2. The withdrawer smart...

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

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

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

createCollectionFor doesn't transfer ownership to _controller that specified in the parameter.

Lines of code Vulnerability details Impact controller is supplied to the function parameter to be controller of the token and owner of the vault. But actually owner of the vault is VaultRegistry that can't control anything in the Vault. Proof of Concept function createCollectionFor bytes32...

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

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

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

merkleRoot can be set by delegatecall(), violate setMerkelRoot()

Lines of code Vulnerability details Impact When delegatecall to target, the context is on Caller contract, all state change logics reflect on Caller’s storage. If the Vault contract calls the malicious contract by function execute，the important storage variable merkleRoot can be modified. Accordi...

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

Incorrect condition will always fail withdrawal

Lines of code Vulnerability details Impact Due to an incorrect approval check, the if condition will always lead to transaction reversal when withdrawal is requested for a holder who is not msg.sender. This can lead to user unable to withdraw funds Proof of Concept 1. Let us see the withdraw...

Upgraded Q -> H from 19 [1657761554720]

Judge has assessed an item in Issue 129 High risk. The relevant finding follows: InfinityExchange.solL326 and InfinityExchange.solL362: When a user pays too much ETH, the additional cost is not reimbursed in contrast to ERC20 transfers, where this is not possible. Consider reimbursing the user li...

Migration Module: Usage of withdrawContribution instead of leave before buyout

Lines of code Vulnerability details Impact HIGH - Assets can be compromised directly. One can use eth from the module for buyout process. Basically sending somebody else's eth from migration module to buyout module Proof of Concept proof of concept The proof of concepts shows a scenario where one...

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

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

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

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

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

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

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

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

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

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

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

unpaused(p) modifier missing in authRedeem function

Lines of code Vulnerability details Impact Due to missing modifier, User will be able to redeem zcTokens and withdraw underlying even in paused Market. This happens due to missing unpausedp modifier Proof of Concept 1. Lets see function definition for authRedeem function function authRedeemuint8 ...

Buyout griefing can block almost all functionalities

Lines of code Vulnerability details Impact Everyone can start a Buyout for a vault by paying only 1 wei. For the next 4 days no other Buyout can start. If someone is fast enough, they can start another griefing buyout as soon as one finishes, meaning that it's possible to block the functionality ...

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

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

percsion error that causes buyoutprice to be very low causing the fractionPrice very low or very high

Lines of code Vulnerability details percsion error that causes buyoutprice to be very low causing the fractionPrice very low detials how does buyout become realy low? because of this percison error in the code if you supply msg.value=1 depsitAmount=1 totalSupply=5 1. also there is a possiblity th...

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

initiateVaultFillingZcTokenInitiate and initiateVaultFillingVaultExit may become nonfunctional after vault maturity

Lines of code Vulnerability details Impact initiateVaultFillingZcTokenInitiate and initiateVaultFillingVaultExit may become nonfunctional after vault maturity Proof of Concept The root of the issue is in VaultTracker.sol transferNotionalFee L222-231: L224 only functions under the assumption that...