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

Insufficient Chainlink price feed validation

Lines of code Vulnerability details JBChainlinkV3PriceFeedcurrentPrice reads the price value from the underlying Chainlink price feed, but ignores the other values returned by latestRoundData, which include the round timestamps and round ID in which the returned price was computed. These values...

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

JBDirectory.primaryTerminalOf might suffer from out of gas DoS if too many terminals are added through JBDirectory._addTerminalIfNeeded

Lines of code Vulnerability details Impact There is no limit to the size of terminalsOf. if too many terminals are added through JBDirectory.addTerminalIfNeeded. JBDirectory.primaryTerminalOf might suffer from out of gas DoS. Proof of Concept The for loop in JBDirectory.primaryTerminalOf may caus...

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

Arbitrary msg.sender can execute recordPaymentFrom, recordRedemptionFor, recordDistributionFor, recordUsedAllowanceOf, recordAddedBalanceFor. Malicious terminal may be passed to inject logic to data store.

Lines of code Vulnerability details Impact Arbitrary msg.sender can execute recordPaymentFrom, recordRedemptionFor, recordDistributionFor, recordUsedAllowanceOf, recordAddedBalanceFor. Malicious terminal may be passed to inject logic to data store. If data store is using custom logic that doesn't...

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

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

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

Some fund could be locked in the project forever because only contributors but not project token holders can redeem

Lines of code Vulnerability details Some fund could be locked in the project forever because only contributors but not project token holders can redeem Impact Once the contributor transfered the project tokens to someone elsein some DEX maybe, these tokens could never be redeemed. One of the...

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

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

JBPayoutRedemptionPaymentTerminal#processFees may run out of gas and revert due to long list of _heldFeesOf[_projectId]

Lines of code Vulnerability details Proof of Concept // Get a reference to the project's held fees. JBFee memory heldFees = heldFeesOfprojectId; // Delete the held fees. delete heldFeesOfprojectId; // Push array length in stack uint256 heldFeeLength = heldFees.length; // Process each fee. for...

JBProjects.sol#createFor(...) function is open to reentrancy attacks

Lines of code Vulnerability details JBProjects.solcreateFor... which used in JPController.sollaunchProjectFor... can be used for reentrancy. PoC contracts/JBProjects.sol: 131: function createForaddress owner, JBProjectMetadata calldatametadata 132 external 133 override 134 returns uint256 project...

Attacker can frontrun and reenter this function causing users to get griefed

Lines of code Vulnerability details Impact 1. attacker calls createFor and there is reentracy in safemint and an attacker can reenter and just keep increasing count just a side note 2. but what an attacker can do is when a user wants to call this function or launch a project an attacker frontruns...

In JBERC20PaymentTerminal.sol#_transferFrom(...) ignoring return value when IERC20(token).transferFrom(...)

Lines of code Vulnerability details In JBERC20PaymentTerminal.soltransferFrom... ignoring return value when IERC20token.transferFrom... Some of ERC20 token implementations return false upon unsuccess transfer like USDT token, this can lead to some unpredictable balances to rise up without actual...

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

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

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

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

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

rewaer

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 164 [1657055445786]

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

Upgraded Q -> M from 95 [1657055392912]

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

Upgraded Q -> M from 139 [1656985204675]

Judge has assessed an item in Issue 139 as Medium risk. The relevant finding follows: 1. Buyouts that occur during the timestamp wrap will have valuation errors The blockTimestamp has a modulo applied, so at some point, there will be a timestamp with a value close to 2^32, followed by a timestamp...

Attacker may front-run acceptCounterOffer() cause users to take both original order and new one

Lines of code Vulnerability details Impact Function PuttyV2.acceptCounterOffer is used in case users see better deal and want to cancel their own order before filling the new one. But attacker can abuse this function by front-run calling fillOrder before it is cancelled in acceptCounterOffer...

Any one can mint penny NFT to take some advantage

Lines of code Vulnerability details Any one can mint penny NFT to take some advantage Impact Some users might be taken advantage of, for those who long call or short put the floortoken, and finnally lead to loss of these users. Proof of Concept There is no control over the floortoken platform,...

Counter offer is not implemented correctly

Lines of code Vulnerability details Impact acceptCounterOffer is not verifying if the original order has already been filled. In case maker makes a counter offer and by the time counter offer is called, some user has already filled the original order then both original and counter offer will be...

Doesn’t check order.baseAsset.code when exercising call order

Lines of code Vulnerability details Impact It is safe to check order.baseAsset.code.length and token.code.length in fillOrder and transferERC20sIn. Because SafeTransferLib doesn’t check whether the erc20 token is actually a contract. /// @dev Note that none of the functions in this library check...

Fake orders can be created for non existant tokens

Lines of code Vulnerability details Impact NFT's can be stolen by an attacker Proof of Concept The putty contract is using the solmate safeTransfer instead of OpenZeppelin's so contract existence isn't being checked for transferred tokens. In the putty case, contract existance is checked for the...

The fee is not paid as intended for put orders

Lines of code Vulnerability details Impact MEDIUM - functions of the protocol could be impacted For put options, the fees are not paid as intended. Proof of Concept poc0: fee on non-exercised put option poc0 shows that fee is applied for put option, even though the option was not exercised. poc1:...

Zero strike call options can be systemically used to steal premium from the taker

Lines of code Vulnerability details Some non-malicious ERC20 do not allow for zero amount transfers and order.baseAsset can be such an asset. Zero strike calls are valid and common enough derivative type. However, the zero strike calls with such baseAsset will not be able to be exercised, allowin...

Cannot create short call floorToken option

Lines of code Vulnerability details The solution is not supporting maker shorting a call of N floorToken although platform declare it support all four types: short\long call\put Proof of Concept Exercising an option is done one-sided by the long taker, at this point the floorTokens should be...

PUT short option is always in loss

Lines of code Vulnerability details Impact For a long PUT condition the party which is doing short will always be in loss Proof of Concept 1. Alice creates and signs a long put option order off-chain for 2 Bored Ape floors with a duration of 30 days, a strike of 124 WETH and a premium of 0.8 WETH...

Short position owner will loose funds when put option expired

Lines of code Vulnerability details Impact Fees are expected to be paid whenever an option is exercised as per the function comment on L235. However, the current protocol implementation also charges fees for expired put options. The owner of a short put option is subject to paying fees whenever t...

both order side positions don’t exist is not checked

Lines of code Vulnerability details Impact The flow in fillOrder as documented in expects the check order side positions doesn't exist . However the fillOrder function does not have a require check for this. This allows for the same side positions to be created multiple times leading to loss of...

onERC721Received() could delegatecall to transder the tokens

Lines of code Vulnerability details onERC721Received could delegatecall to transder the tokens Impact Some onERC721Received could use delegatecall to drain the tokens Putty holds. Because the msg.sender is Putty contract, the tarnsfer can go through. Or taking the ownership of the Putty contract...

Not-yet-exist ERC20 Could Be Used Within An Order

Lines of code Vulnerability details Vulnerability Details It was observed that the PuttyV2 contract uses solmate's SafeTransferLib for pulling ERC20 assets from the order maker or taker to the PuttyV2 contract. There is some difference between the implementation of solmate's SafeTransferLib and...

Attacker can take premium from taker who fill short-call orders for free.

Lines of code Vulnerability details Impact Takers who fill short-call orders may unable to exercise their long-call position. Attackers can abuse it to take premium from takers for free. In PuttyV2.fillOrder function, when taker fill short-call order, order’s maker only has to transfer erc20Asset...

fee proportional to strike even for unexercised orders

Lines of code Vulnerability details Impact The protocol takes a fee proportional to the order's strike. This happens during a withdraw: // transfer strike to owner if put is expired or call is exercised if order.isCall && isExercised || !order.isCall && !isExercised // send the fee to the admin/D...

PuttyV2.sol is allowing the cancelled orders to exercise and withdraw

Lines of code Vulnerability details Impact A cancelled order can be exercised and withdrawn Proof of Concept function exerciseOrder memory order, uint256 calldata floorAssetTokenIds public payable / CHECKS / bytes32 orderHash = hashOrderorder; // check user owns the position...

fund steal by crating a lot of bad long positions and then transferring NFT token of long position to all users and trick them(or by mistake) to click on exercise()

Lines of code Vulnerability details Impact when fillOrder is called code mints two PuttyV2 NFT token, one for Long position and one for Short Position and It's possible to transfer this NFT tokens to others. exercising unwanted bad Long positions can cause users to lose funds and tokens, for...

Fee is being deducted when Put is expired and not when it is exercised.

Lines of code Vulnerability details Impact Fee is being deducted when Put is expired and not when it is exercised in PuttyV2.sol. Comment section of the setFee function mentions "fee rate that is applied on exercise" which signifies that the fee amount is meant to be deducted from strike only whe...

Create a short call order with non empty floor makes the option impossible to exercise and withdraw

Lines of code Vulnerability details Impact HIGH - assets can be lost If a short call order is created with non empty floorTokens array, the taker cannot exercise. Also, the maker cannot withdraw after the expiration. The maker will still get premium when the order is filled. If the non empty...

[H-02] Owner does not get any fee when call is expired

Lines of code Vulnerability details Owner\creators lose profit by not collecting the fees on half the expired cases all calls. The only place where owner receives fee is when withdrawing an exercised call or expired put: But when a call expired, no fee has been collected on any part of the order...

Orders could be unable to be fulfilled due to insufficient fund/underlying when when filled

Lines of code Vulnerability details Orders could be unable to be fulfilled due to insufficient fund/underlying when when filled Proof of Concept Let's say on day 1, Alice created a long call order for 5 ERC721 Bored Apesid1, id2, id3, id4, id5, with premium 2.5 ETH. But there will be a gap betwee...

Short Call Maker Asset Can Be Frozen

Lines of code Vulnerability details Impact The fillOrder function only checks that the floorAssetTokenIds.length must be 0 when a taker fill a short call order. In other words, it does not check whether order.floorTokens.length is 0 or not, which means that if the maker includes any address in...

Using SafeTransferLib as a transfer medium has a certain probability of causing problems

Lines of code Vulnerability details Impact Detailed description of the impact of this finding. function exerciseOrder memory order, uint256 calldata floorAssetTokenIds public payable / CHECKS / bytes32 orderHash = hashOrderorder; // check user owns the position requireownerOfuint256orderHash ==...

Problems with the access control of the fillOrder() function

Lines of code Vulnerability details Impact Detailed description of the impact of this finding. fillOrder does not check order.maker ! = caller, which means that mintorder.maker, uint256orderHash; & mintmsg.sender, positionId; can mint NFT to the same person and therefore may create unintended...

Returned Value not checked when Depositing Eth

Lines of code Vulnerability details Impact In fillorder function, when eth is converted to weth, at line 335, it deposit the msg.value here. But it does not check the return value if the call succeeded. Proof of Concept IWETHweth.depositvalue: msg.value; Tools Used Visual Studio Code Recommended...