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

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

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

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

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

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

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

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

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

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

Uninitialized implementation for Vault can be destroyed

Lines of code Vulnerability details Impact Every Vault is a proxy of the same implementation contract. This implementation is deployed from VaultFactory but never initialized. /// @notice Initializes implementation contract constructor implementation = addressnew Vault; Someone can call init in t...

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

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

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

Migration Module: Disable to join, leave, commit by starting a buyout

Lines of code Vulnerability details Impact MED - the function of the protocol could be impacted. Anyone can call Buyout::start to disable to join, leave, commit functions for migration proposal Proof of Concept The Buyout module is unaware of migration module. So, even when some migration is...

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

Swivel withdraw wrapper function redeems an amount on underlying tokens

Lines of code Vulnerability details Impact Swivel contract uses the redeemUnderlying function from the Compound CERC20 tokens. This function takes as an input the amount of underlying tokens to be redeemed, which is inconsistent compared to other functions. This issue will lead to user fund losse...

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

Upgraded Q -> M from 270 [1657580270573]

Judge has assessed an item in Issue 270 as Medium risk. The relevant finding follows: 6.L- Admin config ProtocolFee and gasFee missing max amount check which can be used to take fund from user With PROTOCOLFEEBPS 10000 more than 100%, the exchange can steal user WETH who might approve max WETH...

Upgraded Q -> M from 270 [1657580410834]

Judge has assessed an item in Issue 270 as Medium risk. The relevant finding follows: 6.L- Admin config ProtocolFee and gasFee missing max amount check which can be used to take fund from user With PROTOCOLFEEBPS 10000 more than 100%, the exchange can steal user WETH who might approve max WETH...

Missing zero address check for bribesProcessor

Upgraded from 45: Missing zero address check for bribesProcessor MyStrategy.sol:100 ///@dev Change the contract that handles bribes function setBribesProcessorIBribesProcessor newBribesProcessor external onlyGovernance; bribesProcessor = newBribesProcessor; The bribeProcessor is not set in the...

Oracle data feeds are insufficiently validated

Lines of code Vulnerability details Impact If the oracle price feeds are insufficiently validated, there will be pricing errors leading to the miss-pricing of assets Proof of Concept The JBSingleTokenPaymentTerminalStore and abstract JBPayoutRedemptionPaymentTerminal both rely on their respective...

Issue with condition check handling

Lines of code Vulnerability details Impact It might cause data corruption, as most of the data handling is done in low level. Proof of Concept // Add up all the percents to make sure they cumulative are under 100%. uint256 percentTotal = 0; for uint256 i = 0; i typeuint56.max revert...

Distribution of reserved tokens may run out of gas

Lines of code Vulnerability details Impact If there are enough entries in the splits array, the function that distributes the reserved tokens will run out of gas, and the reserved tokens will be un-distributable until the current cycle is over, and the splits are changed. If cycles are long, the...

Missing inheritance for JBToken

Lines of code Vulnerability details Impact The JBToken contracts inherits IJBToken, however IJBToken does not inherit IERC20. This would cause the JBToken contract to not be ERC20 compliant and making many of the called ERC20 functions revert. Tools Used Manual review Recommended Mitigation Steps...

Usage of insecure transfer/transferFrom

Lines of code Vulnerability details Proof of Concept This is a well known issue, the protocol uses at many parts the insecure transfer/safeTransfer for generic ERC20 tokens. Recommended Mitigation Steps Use OZ Safe Transfer --- The text was updated successfully, but these errors were encountered:...

Stale prices can be used by the system

Lines of code Vulnerability details updatedAt field of Chainlink's latestRoundData isn't checked, so even substantially outdated price will be used by the system. Proof of Concept Price is the only field that is read from Chainlink: function currentPriceuint256 decimals external view override...

Reentrancy issues on function distributePayoutsOf

Lines of code Vulnerability details Impact In the contract JBPayoutRedemptionPaymentTerminal, the function distributePayoutsOf calls the internal function distributePayoutsOf and this internal function perfoms a loop where is using the function distributeToPayoutSplitsOfhttps://githu...

Weak PRNG (Pseudorandom number generator)

Lines of code Vulnerability details Impact Weak PRNG due to a modulo on block.timestamp, now or blockhash. These can be influenced by miners to some extent so they should be avoided. Proof of Concept Tools Used Slither Recommended Mitigation Steps Do not use block.timestamp, now or blockhash as a...

Increase balance for project without paying

Lines of code Vulnerability details Impact It seems that project owners can directly call recordAddedBalanceFor function at JBSingleTokenPaymentTerminalStore.solL696 to increase their project balance without paying anything Proof of Concept 1. Observe the recordAddedBalanceFor function 2. There i...

Unchecked transfer

Lines of code Vulnerability details Description The return value of an external transfer/transferFrom call is not checked Impact There are some tokens that do not revert on failure but return false instead, if such token is used, the return value won'...

Unchecked return value of transfer() and transferFrom()

Lines of code Vulnerability details Impact There are multiple external calls to IERC20.transfer and IERC20.transfer in contracts that suppose to transfer out tokens from/to the system. However, there are tokens like USDT that don't return any return value in their implementation of the transfer...

Reentrancy vulnerability due to _distributePayoutsOf

Lines of code Vulnerability details The function distributePayoutsOf has reentrancy vulnerability which is being called by the external function distributePayoutsOf There are external calls in this function along with calls sending ETH Impact The contract JBPayoutRedemptionPaymentTerminal inherit...

Chainlink's latestRoundData might return stale or incorrect results

Lines of code Vulnerability details Impact latestRoundData is used to fetch the asset price from a Chainlink aggregator, but it's missing additional validations to ensure that the round is complete. If there is a problem with Chainlink starting a new round and finding consensus on the new value f...

Migration to a new terminal allows project to use funding again in same funding cycle

Lines of code Vulnerability details Impact Distribution and overflow allowance can be reused after migration Proof of Concept When migrating to a new terminal only balances are transferred but not usedDistributionLimitOf or usedOverflowAllowanceOf. This means that both of these values will be 0,...

Users with the JBOperations.RECONFIGURE role can change the project controller if the project hasn't launched yet

Lines of code Vulnerability details Impact Only project owners, or users with the JBOperations.SETCONTROLLER role are supposed to be able to change the controller JBOperations.MIGRATION too if while doing a migration. The JBOperations.RECONFIGURE role is meant to be a less powerful role so this...

Unsafe _price cast

Lines of code Vulnerability details Impact The price provided by chainlink AggregatorV3 could be a negative, if that happend the cast of the price goes high, in example, cast -1 to uint256 was 2256 - 1 Proof of Concept return uint256price.adjustDecimalsfeedDecimals, decimals; Tools Used Manual...

Chainlink oracle lacks proper validation

Lines of code Vulnerability details Impact Calls to the Chainlink price oracle via latestRoundData in JBChainlinkV3PriceFeed.sol use the correct function latestRoundData per Chainlink's documentation, but lack the recommended validations to ensure that the round is complete and does not return...

JBProjects.createFor should be able to set the project controller

Lines of code Vulnerability details Impact JBProjects.createFor doesn’t set the project controller. Project owners need to call JBDirectory.setControllerOf to set the project controller. However, an allowedlisted address can set the controller before the actual project owner. We can assume that...

Improper validation Chainlink's latestRoundData() for stale or incorrect result

Lines of code Vulnerability details Impact The result from price feed needs further validation for stale and incorrect results. Proof of Concept function currentPriceuint256 decimals external view override returns uint256 // Get the latest round information. Only need the price is needed. , int25...

CreateFor#JBProjects.sol is vulnerable to reentrancy

Lines of code Vulnerability details Impact Reentrancy attack can trick users by spamming incorrect project Id event emission Proof of Concept The function createFor which uses safeMint is lacking any form a reentrancy protection. OZ Safe Mint implementation is function safeMint address to, uint25...

State Variable Shadowing

Lines of code Vulnerability details Impact Check: shadowing-state Severity: High Confidence: High It is possible to use the same variable twice in Solidity, but it can lead to unintended side effects.The TestAllowance.sol contract inherits from TesTBaseWorkflow.sol. In the TestAllowance.sol...

Use of IERC20.transfer() instead of SafeERC20.safeTransfer()

Lines of code Vulnerability details Impact There are multiple external calls to IERC20.transfer and IERC20.transfer in contracts that suppose to transfer out tokens from/to the system. However, there are tokens like USDT that don't return any return value in their implementation of the transfer...

No check for stale chainlink oracle data in currentPrice function

Lines of code Vulnerability details Impact There is no check if the value of price returned by chainlink latestRoundData is latest or stale. If stale price is returned, it may result in wrong calculation used further, and in JBPrices.sol Proof of Concept Contract : JBChainlinkV3PriceFeed.sol Line...

Failed transfer during migrate could lockup the fund in old terminal

Lines of code Vulnerability details Without require for boolean checking, if to.addToBalanceOfvalue: payableValueprojectId, balance, token, '', bytes''; fail when transfer, migrate will not revert. This could cause the fund to lockup in the old terminal since store.recordMigrationprojectId; will...

Double spend of tokens because unnecessary approve

Lines of code Vulnerability details Impact Double spend of tokens Proof of Concept I believe that there is an error in your logic, while you approve tokens before transfering them will give the receiver twice that you want. function beforeTransferToaddress to, uint256 amount internal override...

migrate can change the controller and bypass all the checks

Lines of code Vulnerability details Impact Controller can use migrate to change controller address, leading to the project owner changing a new controller with different logic that users may unexpect and bypassing the condition check / permission check of the previous controller. Proof of Concept...

configureFor's check for interface compliance can backfire

Lines of code Vulnerability details Impact Controlller supplied data.ballot is checked by configureFor to be compliant with IJBFundingCycleBallot interface by wrapping the call. Proof of Concept configureFor checks for interface support with a nested function: try...

The splits configuration will become frozen once enough splits are added

Lines of code Vulnerability details Impact If there are enough entries in the splits array, the checks done to ensure existing locks are respected will cause attempts to change the split to revert, preventing the existing split assignment from changing. If the project has a lock with a long...

Missing checks on return data from the chainlink

Lines of code Vulnerability details Impact MED - the function of the protocol could be impacted 1. Use stale price information resulting to wrong project's balance 2. In the case of zero price, functions using price information will revert. Proof of Concept // JBPrices::priceFor at line 69 calls...