trader-joe-v2 does not provide any mechanism to handle the pairs with different decimal values.

Lines of code Vulnerability details Impact Joe could not able to provide the swapping/staking for pair of tokens with different decimal values. Proof of Concept To my knowledge based on the code analys, nowhere I saw the handling for pairs with different decimal values. Tools Used Vs code and joe...

TokenHelper.sol#L40 : safeTransfer will revert due to insufficient gas.

Lines of code Vulnerability details Impact I am adding as high issue since most of the calling is done using the safeTransfer TokenHelper.solL40 : safeTransfer will revert due to insufficient gas. All the fuctions that are using the safeTransfer could fail due to insufficient gas. I see the...

Unchecked math in LBToken.sol may cause overflow

Lines of code Vulnerability details Impact Affected code: Currently the safeTransferFrom function in LBToken.sol calls transfer which performs unchecked math to deal with user balances. This is what happens: unchecked balancesidfrom = fromBalance - amount; balancesidto = toBalance + amount; While...

Missing input validation can lead to accidental burning of tokens

Lines of code Vulnerability details Impact Some token transfers do not check that the receiving address is not the zero address. This can lead to an unintended burning of tokens. Proof of Concept 1. Assume Alice uses a web3 frontend to interact with a DAI/USDT pool. 2. Alice wants to swap DAI for...

Redemption weight of tiered NFTs miscalculates, making users redeem incorrect amounts - Bug #1

Lines of code Vulnerability details Description Redemption weight is a concept used in Juicebox to determine investor's eligible percentage of the non-locked funds. In redeemParams, JB721Delegate calculates user's share using: uint256 redemptionWeight = redemptionWeightOfdecodedTokenIds; uint256...

_expectMint is not checked when tokenId != 0

Lines of code Vulnerability details Impact In the mintBestAvailableTier function from the JBTiered721Delegate contract the expectMint variable is used to determine if the user is expecting the contract to mint tokens with the left over funds or not, but the boolean value of expectMint is only...

Attacker can steal entire reserves by abusing fee calculation

Lines of code Vulnerability details Description Similar to other LP pools, In Trader Joe users can call mint to provide liquidity and receive LP tokens, and burn to return their LP tokens in exchange for underlying assets. Users collect fees using collectFessaccount,binID. Fees are implemented...

Future tier can be removed

Lines of code Vulnerability details Impact A newly created tier can be marked as removed right after it was added if this tier's ID was mistakenly removed earlier. Proof of Concept The recordRemoveTierIds function doesn't check whether a tier ID exists or not JBTiered721DelegateStore.solL890:...

When _dontMint == true, creditsOf does not add _value

Lines of code Vulnerability details Impact In the processPayment function of the JBTiered721Delegate contract, the contract does not mint NFTs for the user if the user sets dontMint to true in the parameter. But the contract also does not add the tokens sent by the user to the creditsOf variable...

Silent returns in _processPayment may cause users to lose assets

Lines of code Vulnerability details Impact The processPayment function of the JBTiered721Delegate contract is used to mint NFT for the user after receiving the tokens sent by the user. When the token sent by the user differs from pricingCurrency, it tries to get the corresponding price via...

launchProjectFor() does not check the address of _owner

Lines of code Vulnerability details Impact launchproject can be launchProject in launchProjectFor, but the entire process does not check the validity of owner, not even JBController.sol. This will enable owner to be specified as address0. This will enable projects to be created without an owner,...

NFT Owner can keep add new tiers to increase the NFT supply with no upper limit to dilute the redemption power of the NFT from old tiers

Lines of code Vulnerability details Impact Owern can adjust tiers, adding tiers with no restriction. function adjustTiersJB721TierParams calldata tiersToAdd, uint256 calldata tierIdsToRemove // Add the tiers. if numberOfTiersToAdd != 0 // Record the added tiers in the store. uint256 memory...

Internal function declared but not called

Lines of code Vulnerability details Impact JBTiered721Delegate.processPayment is an internal function that overrides JB721Delegate.processPayment, however is not called by any other function or contract. With this, users who make a paid contribution will not mint any tier for beneficiary. Proof o...

didPay() receives payment but does not mint NFT to beneficiary as expected.

Lines of code Vulnerability details Impact The function JB721Delegate.didPay is expected to mint an NFT to the beneficiary if conditions are met. However the function only receives payment through payable, but does nothing else in the processPayment call if function does not revert. The expected...

Contract locking eth

Lines of code Vulnerability details Impact The JB721Delegate contract contains payable functions, but no function to withdraw the ether. This would allow the ether sent to the contract to be unable to be withdrawn. JB721Delegate.didPay JB721Delegate.didRedeem Proof of Concept JB721Delegate.didPay...

Upgraded Q -> M from 236 [1666363743332]

Judge has assessed an item in Issue 236 as Medium risk. The relevant finding follows: ERC721 token can be lost in fillAsk PROBLEM When a user fills an ask order by calling fillAsk, the ERC721.transferFrom method is used to transfer the NFT to the receiver. Should the receiver be a smart contract...

Upgraded Q -> M from 266 [1666361934775]

Judge has assessed an item in Issue 266 as Medium risk. The relevant finding follows: Stop Using Transfer Use call instead of transfer, as per. This does not introduce any issues for re-entrancy as all calling functions are marked non-reentrant. --- The text was updated successfully, but these...

Making a payment to the protocol with _dontMint parameter will result in lost fund for user.

Lines of code Vulnerability details Impact User will have their funds lost if they tries to pay the protocol with dontMint = False. A payment made with this parameter set should increase the creditsOf balance of user. In processPayment, creditsOfdata.beneficiary is updated at the end if there are...

Upgraded Q -> M from 703 [1666368006323]

Judge has assessed an item in Issue 703 as Medium risk. The relevant finding follows: L01: Usage of transfer to send eth It is recommended to use call instead of transfer due to fixed gas stipend. In the GolomTrader, transfer is used to pay ether. --- The text was updated successfully, but these...

Upgraded Q -> M from 100 [1666359995498]

Judge has assessed an item in Issue 100 as Medium risk. The relevant finding follows: L-05 address.callvalue:x should be used instead of payable.transfer:- File: 2022-07-golom/contracts/core/GolomTrader.sol line 154: payablepayAddress.transferpayAmt; --- The text was updated successfully, but the...

Upgraded Q -> M from 964 [1666360503408]

Judge has assessed an item in Issue 964 as Medium risk. The relevant finding follows: Non-critical: EIP712 signatures on GolomTrader could be replayed in case of blockchain forks The chainId is burnt into EIP712DOMAINTYPEHASH rather than checked each time. This means that signatures could be...

Upgraded Q -> M from 612 [1666359673078]

Judge has assessed an item in Issue 612 as Medium risk. The relevant finding follows: NC-01 Replace assembly chainid with Solidity's chainId Description Retrieving the current chain id via the Yul chainid expression can be replaced with the Solidity native call to chainId. Findings...

Upgraded Q -> M from 330 [1666367006171]

Judge has assessed an item in Issue 330 as Medium risk. The relevant finding follows: GolomTrader is not using ERC721 safeTransferFrom when transferring tokens All the functions that fill orders are not using safeTransferFrom when the owner of the NFT transfer the token to the user or contract th...

Upgraded Q -> M from 889 [1666366016998]

Judge has assessed an item in Issue 889 as Medium risk. The relevant finding follows: L-05 safeTransferFrom of VoteEscrowCore does not fully comply ERC721 standard Per , safeTransferFrom needs to throw if the receiver is a contract that does not have the onERC721Received function that returns...

Upgraded Q -> M from 94 [1666365598508]

Judge has assessed an item in Issue 94 as Medium risk. The relevant finding follows: L-04 payEther should use .call instead of .transfer Use of .transfer is no longer encouraged, as it may fail if the receiver has any logic in their receive function, due to the 2300 gas consumption limit. --- The...

Upgraded Q -> M from 460 [1666364175291]

Judge has assessed an item in Issue 460 as Medium risk. The relevant finding follows: 7. transfer is used to transfer Ether Description: GolomTrader uses transfer method to send Ether. This is generally less recommended these days see article from Consensys Diligence, mainly because gas costs can...

Upgraded Q -> M from 921 [1666361310869]

Judge has assessed an item in Issue 921 as Medium risk. The relevant finding follows: 1. Should use call instead of transfer Line References GolomTrader.solL154 Impact The payableaddress.transfer function has a limit of 2300 gas source. If the receiver has a fallback/receive function that require...

Upgraded G -> M from 553 [1666369528441]

Judge has assessed an item in Issue 553 as Medium risk. The relevant finding follows: 01 - payEther use transfer instead of call Replace line 154 for payablepayAddress.callvalue: payAmt"" Reason --- The text was updated successfully, but these errors were encountered: All reactions...

Upgraded Q -> M from 207 [1666364300864]

Judge has assessed an item in Issue 207 as Medium risk. The relevant finding follows: L-03 Use safeTransferFrom for ERC721 tokens Description As OpenZeppelin recommends, the transferFrom function should not be used for transferring ERC721 NFT tokens and instead the safeTransferFrom function shoul...

Upgraded Q -> M from 400 [1666366548697]

Judge has assessed an item in Issue 400 as Medium risk. The relevant finding follows: Use address.call instead of payable.transfer Avoid use of transfer to send ether since transfer sends a fixed amount of gas and future gas cost change may cause revert, The receiving contract will also revert if...

Changing default reserved token beneficiary may result in wrong beneficiary for tier

Lines of code Vulnerability details Impact When the reservedTokenBeneficiary of a tier is equal to defaultReservedTokenBeneficiaryOfmsg.sender, it is not explicitly set for this tier. This generally works well because in the function reservedTokenBeneficiaryOfaddress nft, uint256 tierId,...

Lack of sanity check for initialQuantity and remainingQuantity when new tiers are being added can lead to underflow

Lines of code Vulnerability details Impact Underflow can happen in recordMint and recordBestMintAvailableTier as unchecked is used. This causes a wrong value of tokenId to be returned. Proof of Concept In recordAddTiers, it checks for many params of JB721TierParams, including...

Upgraded Q -> M from 497 [1666363365064]

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

Upgraded Q -> M from 796 [1666363133014]

Judge has assessed an item in Issue 796 as Medium risk. The relevant finding follows: L‑01 Use of transferFrom rather than safeTransferFrom for NFTs in will lead to the loss of NFTs The EIP-721 standard says the following about transferFrom: /// @notice Transfer ownership of an NFT -- THE CALLER ...

Upgraded Q -> M from 873 [1666362235337]

Judge has assessed an item in Issue 873 as Medium risk. The relevant finding follows: Avoid payableaddress.transfer GolomTraderpayEther uses payableaddress.transfer to send native ETH. It's considered a best practice to avoid this pattern for ETH transfers, since it forwards a fixed gas stipend...

Upgraded Q -> M from 437 [1666362126304]

Judge has assessed an item in Issue 437 as Medium risk. The relevant finding follows: Use safetransferfrom instead of transferfrom for ERC721. According to OpenZeppelin's documentation, transferFrom is discouraged and use safeTransferFrom instead. --- The text was updated successfully, but these...

Upgraded Q -> M from 868 [1666360678566]

Judge has assessed an item in Issue 868 as Medium risk. The relevant finding follows: Using .call instead of .transfer Currently it is using .transfer to transfer ETH payablepayAddress.transferpayAmt; Using deprecated transfer on address payable may revert in these cases: 1. The withdraw recipien...

Upgraded Q -> M from 42 [1666367610163]

Judge has assessed an item in Issue 42 as Medium risk. The relevant finding follows: Permit signature replay across forks Details: GolomTrader.sol defines chainId at contract deployment without reconstructing it for every signature. However, as stated in the security considerations section of...

Upgraded Q -> M from 129 [1666359797557]

Judge has assessed an item in Issue 129 as Medium risk. The relevant finding follows: 1.use transfer to pay eth GolomTrader.sol payEther use transfer to pay eth , and receiver can be specified arbitrarily, it is recommended to use call to avoid a certain chance of failure due to 2300 gas fee...

Upgraded Q -> M from 463 [1666363539691]

Judge has assessed an item in Issue 463 as Medium risk. The relevant finding follows: Ignores return value of onERC721Received The try block should include a check to make sure the function returns its selector as specified by IERC721Receiver. While the function may have the same parameters as th...

Upgraded Q -> M from 400 [1666366553858]

Judge has assessed an item in Issue 400 as Medium risk. The relevant finding follows: Use safeTransferFrom instead of transferFrom transferFrom can be replaced with safeTransferFrom as it makes sure that the receiver can handle ERC721 tokens and prevents it from being stuck in the contracts Refer...

Upgraded Q -> M from 336 [1666364078201]

Judge has assessed an item in Issue 336 as Medium risk. The relevant finding follows: Using transfer For Sending Ether Permalinks Description The transfer and send functions forward a fixed amount of 2300 gas. Historically, it has often been recommended to use these functions for value transfers ...

Upgraded Q -> M from 696 [1666361742731]

Judge has assessed an item in Issue 696 as Medium risk. The relevant finding follows: L05 - Usage of trasfer over call to send Ether could cause unexpected Reverts payablepayAddress.transferpayAmt; // royalty transfer to royaltyaddress The function payEther sends ether via transfer which passes a...

Iterations over all tiers in recordMintBestAvailableTier can render system unusable

Lines of code Vulnerability details Impact JBTiered721DelegateStore.recordMintBestAvailableTier potentially iterates over all tiers to find the one with the highest contribution floor that is lower than amount. When there are many tiers, this loop can always run out of gas, which will cause some...

The tier setting parameter are unsafely downcasted from type uint256 to type uint80 / uint48 / uint40 / uint16

Lines of code Vulnerability details Impact The tier setting parameter are unsafely downcasted from uint256 to uint80 / uint48 / uint16 the tier is setted by owner is crucial because the parameter affect how nft is minted. the the callstack is JBTiered721Delegate.solinitialize - StorerecordAddTier...

Owner can set lockUntil to a very large timestamp to create not-removeable tier and not-pause-able tier

Lines of code Vulnerability details Impact Owner can set lockUntil to a very large timestamp to create not-removeable tier. when a tier is created, the owner can set lockUntil parameter @member lockedUntil The time up to which this tier cannot be removed or paused. when the owner call adjustTier,...

Uninitialized Storage Variables

Lines of code github.com/jbx-protocol/juice-nft-rewards/blob/f9893b1497098241dd3a664956d8016ff0d0efd0/contracts/JBTiered721DelegateStore.solL344 github.com/jbx-protocol/juice-nft-rewards/blob/f9893b1497098241dd3a6649568016ff0d0efd0/contracts/JBTiered721DelegateStore.solL1024 Vulnerability details...

JBTiered721DelegateStore: Incorrect calculation of totalRedemptionWeight

Lines of code Vulnerability details Impact In the totalRedemptionWeight function of the JBTiered721DelegateStore contract, the calculation of the weight in the following code is incorrect, and the result of numberOfReservedTokensOutstandingFor should also be multiplied by...

Anyone can create a project with a forged JB721Delegate

Lines of code Vulnerability details Impact An attacker can create a project with a forged JB721Delegate fully controlled by the attacker. The contributors of such a malicious project will lost their funds. Proof of Concept function deployDelegateFor uint256 projectId, JBDeployTiered721DelegateDat...

Owner can set contribution floor to 0 , meaning the user lose the fund / fee for 0 contribution power NFT and NFT have no redemption weight

Lines of code Vulnerability details Impact Owner can set contribution floor to 0, meaning the user lose the fund / fee for 0 contribution power NFT and NFT have no redemption weight. the normal payment flow for user is Payment Terminal - pay - didPay - processPayment - // Mint rewards if they wer...