addRegistrationTributeGovernance shoud call_addGovernanceTribute ?

Handle gpersoon Vulnerability details Impact The function addRegistrationTributeGovernance makes a call to addTribute, the same as addRegistrationTribute is doing However a function addGovernanceTribute also exists and this function is never called. It seem more logical that...

Unchecked return value of transferFrom in function timeLockERC20

Handle shw Vulnerability details Impact In the function timeLockERC20 line 610, the return value of IERC20.transferFrom is unchecked. The return value could be false if the transferred token is not ERC20-compliant, indicating that the transfer fails. In that case, the variable timelockERC20Balanc...

Sorry, I used the wrong submission form, the recent issues are not relevant for NFTX 🤦‍♂️

Handle cmichel Vulnerability details --- The text was updated successfully, but these errors were encountered: All reactions...

transferERC721 doesn't clean timelockERC721s

Handle gpersoon Vulnerability details Impact The function transferERC721 works similar to the functions timeUnlockERC721 with timelocked NFT's. However timeUnlockERC721 cleans timelockERC721s delete timelockERC721skey;, while transferERC721 doesn't clean timelockERC721s This could mean that...

The direct redeem fee can be circumvented

Handle janbro Vulnerability details Summary The direct redeem fee can be circumvented Risk Rating Medium Vulnerability Details Since the random NFT is determined in the same transaction a payment or swap is being executed, a malicious actor can revert a transaction if they did not get the NFT the...

_sendForReceiver is vulnerable to reentrancy. This enables a receiver to drain the remaining fees to distribute.

Handle janbro Vulnerability details Summary sendForReceiver is vulnerable to reentrancy. This enables a receiver to drain the remaining fees to distribute. Risk Rating Critical Vulnerability Details NFTXFeeDistributor.sol Line 163: bool success, bytes memory returnData =...

getRandomTokenIdFromFund yields wrong probabilities for ERC1155

Handle @cmichelio Vulnerability details Vulnerability Details NFTXVaultUpgradeable.getRandomTokenIdFromFund does not work with ERC1155 as it does not take the deposited quantity1155 into account. Impact Assume tokenId0 has a count of 100, and tokenId1 has a count of 1. Then getRandomId would have...

mint for 0 cost when the sale is over

Handle paulius.eth Vulnerability details Impact function getPrice returns 0 when elapsed saleDuration, it does not revert when the sale is over and function mint does not check that. So a 0 salePrice will be used to charge the msg.sender and make a useless transfer to the beneficiary. I am not su...

Missing zero/threshold check for NFT sale price

Handle 0xRajeev Vulnerability details Impact A zero or some minimum threshold check is missing for price parameter of startSale function which sets the mint price for NFTs. If accidentally set to 0 then all sales happen at this incorrect price leading to missed revenue. This cannot be corrected...

Unrestricted access to lockUnits allows an attacker to steal funds from any user.

Handle shw Vulnerability details Impact The lockUnits and unlockUnits functions in Pools.sol allow anyone to call without any restrictions or access control on the caller. An attacker can steal any user's member units by directly calling lockUnits. Proof of Concept Referenced code:...

Wrong slippage protection on Token -> Token trades

Handle @cmichelio Vulnerability details Vulnerability Details The Router.swapWithSynthsWithLimit allows trading token to token and specifying slippage protection. A token to token trade consists of two trades: 1. token to base 2. base to token The slippage protection of the second trade base to...

changeDAO should be a two-step process in Vader.sol

Handle 0xRajeev Vulnerability details Impact changeDAO updates DAO address in one-step. If an incorrect address is mistakenly used and voted upon then future administrative access or recovering from this mistake is prevented because onlyDAO modifier is used for changeDAO, which requires msg.sende...

Bypass or reduction on the lockup period.

Handle shw Vulnerability details Impact In Pool.sol, the lockup restriction of withdrawal can be bypassed or reduced if new liquidity providers cooperate with existing ones. Proof of Concept 1. A liquidity provider Alice deposits liquidity assets into the pool and gained some FDTs. She then waits...

Isolated margin contracts declare but do not set the value of liquidationThresholdPercent

Email address [email protected] Handle paulius.eth Eth address 0x523B5b2Cc58A818667C22c862930B141f85d49DD Vulnerability details CrossMarginTrading sets value of liquidationThresholdPercent in the constructor: liquidationThresholdPercent = 110; Isolated margin contracts declare but do not set the...

Missing checks if pairs equal tokens

Email address [email protected] Handle @cmichelio Eth address 0x6823636c2462cfdcD8d33fE53fBCD0EdbE2752ad Vulnerability details The UniswapStyleLib.getAmountsOut, PriceAware.setLiquidationPath and others don't check that path.length + 1 == tokens.length which should always hold true. Also, it does n...

Inconsistent usage of applyInterest

Email address [email protected] Handle paulius.eth Eth address 0x523B5b2Cc58A818667C22c862930B141f85d49DD Vulnerability details It is unclear if the function applyInterest is supposed to return a new balance with the interest applied or only the accrued interest? There are various usages of it,...

Missing fromToken != toToken check in MarginRouter.crossSwapExactTokensForTokens/MarginRouter.crossSwapTokensForExactTokens

Email address [email protected] Handle @cmichelio Eth address 0x6823636c2462cfdcD8d33fE53fBCD0EdbE2752ad Vulnerability details Attacker calls MarginRouter.crossSwapExactTokensForTokens with a fake pair and the same token0 == tokne1. crossSwapExactTokensForTokens1000 WETH, 0, ATTACKERCONTRACT, WETH,...

TRANSACTION EXECUTION IS DoS IN THE CROSS-CHAIN GOVERNANCE CONTRACTS AND IN THE GNOSIS SAFE COMMUNITY MULTISIG TRANSACTION CHECKS SINCE THE WRONG payload IS EXTRACTED FROM THE data BYTES ARRAY

Lines of code Vulnerability details Impact The GuardCM.verifyBridgedData function is used to verify the bridged data for authorized combinations of targets and selectors in the Gnosis Safe community multisig. The data payload is passed into the verifyBridgedData function which is then unpacked...

NO access control in decreaseAllowance and increaseAllowance

Lines of code Vulnerability details Impact Detailed description of the impact of this finding. NO access control in decreaseAllowance and increaseAllowance.Anyone can call this function and increase or decrease the allowlance. Proof of Concept Provide direct links to all referenced code in GitHub...

Upgraded Q -> 2 from #49 [1704028025372]

Judge has assessed an item in Issue 49 as 2 risk. The relevant finding follows: L-05 Some tokens revert on 0 amount transfer ParticlePositionManager::liquidatePosition: File: protocol/ParticlePositionManager.sol 376: // reward liquidator 377: TransferHelper.safeTransfercloseCache.tokenFrom,...

Delisted wLp still could be used for collateralization by changing position mode

Lines of code Vulnerability details Protocol governor address has the power to whitelist and delist wLp addresses using the ConfigsetWhitelistedWLps function. Only whitelisted wLp tokens are allowed to collateralize and de-collateralize users' positions: File: InitCore.sol 244: function...

setPosMode should not allow changing the mode when the new mode's canRepay status is disabled

Lines of code Vulnerability details Impact In the scenario where the mode's canRepay status is set to false, positions using that mode cannot be repaid and liquidated. However, users are allowed to change their position's mode to one where the canRepay status is currently set to false. This could...

collateralizeWLp can be bypassed even when collateralization is paused

Lines of code Vulnerability details Impact Admin can pause collateralization for a specific mode to prevent users from providing more collateral either via collateralize or collateralizeWLp. However, due to not properly using internal accounting when tracking wLP collateral, users can still provi...

Error Handling and Consistency in '_settleAuction' Function

Lines of code Vulnerability details Potential Risk: The 'settleAuction' function is responsible for settling an auction by finalizing the bid and handling payouts to various parties. It performs several operations and interactions with external contracts. However, the function lacks proper error...

MaxHeapify: find children with large value and swap

Lines of code Vulnerability details Input Validation: - The maxHeapify function assumes that pos is a valid position within the heap. - Similar to the swap function, you should consider adding a check to verify that pos is within the bounds of your heap. requirepos size, "Invalid position...

The builderReferral, purchaseReferral and deployer can never be equal to address(0), which leads to the revolutionRewardRecipient stealing their rewards

Lines of code Vulnerability details HIGH The builderReferral, purchaseReferral and deployer can never be equal to address0, which leads to the revolutionRewardRecipient stealing their rewards Description: revolutionRewardRecipient will receive the rewards of the builderReferral, purchaseReferral...

Missing access control on critical functions

Lines of code Vulnerability details The broad admin role enables arbitrary manipulation of the heap without restrictions. Recommendation: Implement granular access control and privilege separation. Implement an access control system such as OpenZeppelin AccessControl to restrict access to these...

Signature Verification for _verifyVoteSignature Function

Lines of code Vulnerability details Potential Risk: The verifyVoteSignature function in the CultureIndex contract is responsible for verifying signatures for specific votes. While it attempts to verify signatures, there are potential risks associated with signature verification. Proof of Concept...

Efficiency

Lines of code Vulnerability details Depending on the size of your heap, the while loop may execute a significant number of times. Consider whether you can optimize this loop for performance, especially if you expect a large number of insertions. Assessed type Loop --- The text was updated...

Signature Verification for voteForManyWithSig Function

Lines of code Vulnerability details Potential Risk: The voteForManyWithSig function in the CultureIndex contract allows users to vote on multiple pieceIds using a provided signature. While it attempts to verify the signature, there are some potential risks associated with signature verification...

Error Handling in '_createAuction' Function

Lines of code Vulnerability details Potential Risk: The 'createAuction' function attempts to mint a new Verb by calling the 'verbs.mint' function. However, it lacks proper error handling for the minting process. If the minting operation fails e.g., due to insufficient gas or other reasons, the...

CultureIndex.sol#_vote() - Creators of certain piece can vote for their piece

Lines of code Vulnerability details Impact In CultureIndex there is a function vote that allows users to vote for a piece to get sold on the auction house. Each piece has creators that get cut of the sale. The problem is that there is no checks if the user voting for a certain piece is it's own...

The creator does not receive additional ether - it accumulates on the contract

Lines of code Vulnerability details Impact In some cases, the ERC20TokenEmitter contract may accumulate residual ether that was not sent to the creator. The contract does not have a function to pick up the remaining ether. When a user wants to buy tokens, he sends ether to the...

When the returnNative parameter is set to true in the _params provided to MoneyMarketHook.execute, it is not handled properly and could disrupt user expectations

Lines of code Vulnerability details Impact When param.returnNative is set to true while calling MoneyMarketHook.execute, users expect the returned token from the withdraw operation to be in native form and sent to the caller. However, in the current implementation, this is not considered and coul...

_safeMint() should be used rather than _mint() wherever possible

Lines of code 139, 178, 204 Vulnerability details mint is discouraged in favor of safeMint which ensures that the recipient is either an EOA or implements IERC721Receiver. Both OpenZeppelin and solmate have versions of this function. In the cases below, mint does not call...

Return values of transfer()/transferFrom() not checked

Lines of code 377, 509, 491, 530, 42, 50 Vulnerability details Not all IERC20 implementations revert when there's a failure in transfer/transferFrom. The function signature has a boolean return value and they indicate errors that way instead. By not checking the return value, operations that shou...

Unchecked return value of low-level call()/delegatecall()

Lines of code 120, 141, 411, 184, 160, 189, 152, 444, 625, 638https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contract...

In case if wLP will be blacklisted then user will not be able to withdraw it

Lines of code Vulnerability details Proof of Concept When users deposit wLP tokens as collateral, then they are checked to be whitelisted. Later, it's possible that for some reason wLP token will be backlisted by governor. And once it's done, then users, who already used that wLP tokens as...

bad debt is not socialized

Lines of code Vulnerability details Proof of Concept In case if borrower's position is unhealthy, then he can be liquidated. Liquidator can provide amount of shares in poolToRepay that he will cover and expects to get back poolOut shares. It is possible that position created a bad debt. This mean...

The owner is a single point of failure and a centralization risk

Lines of code 56, 109, 127, 172, 219, 250, 115, 131, 154, 116, 131https://github.com/Tapioca-DAO/tapiocaz-audit/blob/bcf61f79464cfdc0484aa272f9f6e28d...

Calls to get_virtual_price() are vulnerable to read-only reentrancy

Lines of code 117 Vulnerability details getvirtualprice was originally considered to be a manipulation-resistant price - suitable as a price oracle, but it was later found to be vulnerable to a read-only reentrancy attack, where the Curve contract could be put into a partially-modified state, and...

addRewardToken() does note remove old entries before adding new ones

Lines of code 455, 280, 378, 411 Vulnerability details Each time addRewardToken is called, new entries are added to the array, but doing so does not remove any old entries. By calling the function multiple times, an attacker can can increase their voting power indefinitely, without having to...

Interface improperly implemented

Lines of code 34, 34, 34, 34, 30, 31, 32, 34, 35, 38https://github.com/Tapioca-DAO/tapioca-yieldbox-strategies-audi...

Unsafe usage of msg.value in a loop

Lines of code 140 Vulnerability details The value of msg.value in a transaction's call never gets updated, even if the called contract ends up sending some or all of the Eth to another contract. This means that using msg.value in a for- or while-loop, without extra accounting logic, will either...

addRewardToken() does note remove old entries before adding new ones

Lines of code 455, 280, 378, 411 Vulnerability details Each time addRewardToken is called, new entries are added to the array, but doing so does not remove any old entries. By calling the function multiple times, an attacker can can increase their voting power indefinitely, without having to...

Return values of transfer()/transferFrom() not checked

Lines of code 377, 509, 491, 530, 42, 50 Vulnerability details Not all IERC20 implementations revert when there's a failure in transfer/transferFrom. The function signature has a boolean return value and they indicate errors that way instead. By not checking the return value, operations that shou...

Incorrect indexing in constructor of Curve Adapters causes both contracts to assume xToken and lpToken to be the same token

Lines of code Vulnerability details Impact Curve Adapter contracts are unusable, as the protocol won't be able to tell the difference between which token USDC/USDT or lpToken is intended on being used for the transaction. Vulnerability details In the constructors of the Curve2PoolAdapter.sol and...

CurveTricryptAdapter::primitiveOutputAmount & Curve2PoolAdapter::primitiveOutputAmount can swap without slippage tolerance

Lines of code Vulnerability details Impact While there is a “Slippage protection” implementation in the contract if uint256minimumOutputAmount outputAmount revert SLIPPAGELIMITEXCEEDED; There is no validation that minimumOutputAmount is not set to 0. This can result in lost of funds. Although Oce...

Ocean cannot _mintBatch() as onERC1155BatchRecieved() not implemeneted on the Ocean contract when batch transferring to itself

Lines of code Vulnerability details The comment @ Ocean L348 states: The Ocean never initiates ERC1155 Batch Transfers. This is untrue, note the following callstack: Ocean.doMultipleInteractions | Ocean.forwardedDoMultipleInteractions Ocean.doMultipleInteractions calls mintBatch @ L560...

latestAnswer() may return stale values

Lines of code 121, 122, 123, 124, 51 Vulnerability details latestAnswer only returns the latest answer or zero, and thus there is no way to tell whether the value is stale or not. Use latestRoundData instead, and check whether the latest timestamp is within your protocol's limits. File:...