user's funds lock and incorrect code behavior because users withdrawal amount won't get reset for all users in each userPeriodLength in WithdrawHook contract

Lines of code Vulnerability details Impact according to the comments in code: "Every time userPeriodLength seconds passes, the amount withdrawn for all users will be reset to 0" . but in current implementation only one of the users userToAmountWithdrawnThisPeriod value gets reset and this will...

Pool.sol : Lack of slippage protection for swap

Lines of code Vulnerability details Impact Due to lack of slippage protection, user may suffer and incur loss of func in adverse condition. Proof of Concept Swap function is used to swap one toke for another token. While the swap function has check to ensure for exactOutput, but it does not have...

Router can perform swaps, add/remove liquidity to pools that do not belong to the protocol.

Lines of code Vulnerability details Impact Users can lose their funds PoC In UniswapV3 decodeFirstPool returns the tuple address tokenOut, address tokenIn, uint24 fee . From there it lookups the corresponding pool address with getPooltokenIn, tokenOut, fee which may not exist. See However, in you...

There is no option to cancel an EMERGENCY mode.

Lines of code Vulnerability details Impact There is no option to cancel an EMERGENCY mode in the pool. Once the pool is in an EMERGENCY mode, the pool can't work as normal forever. Proof of Concept The pool can be changed to an EMERGENCY mode by adminAction. function adminActionuint256 action,...

Liquidity cannot be removed by an approved address via Router

Lines of code Vulnerability details Impact Using the Router, liquidity can only be removed by the owner of an NFT, which significantly limits liquidity management. The Pool contract, however, does allow approved addresses to remove liquidity. Proof of Concept The Router contract is a higher level...

POSSIBLE FRONTRUNNING ATTACK ON MINTING LONG/SHORT POSITIONS

Lines of code Vulnerability details Impact createMarket function in PrePOMarketFactory.sol contract creates a new PrePOMarket contract. Salt is used for creating the contract which is computed from createPairTokens function. Variables passed to this function are visible from anyone they are input...

PrePOMarket.redeem() business logic may raise users' concerns of their assets

Lines of code Vulnerability details Impact When a user tries to redeem assets, current business logic may fail and pop the error msg fee = 0 repeatedly. Users may not understand what's happening behind, thus raise concers about the safety of their assets since it looks like their assets are out o...

Permanent freeze of yield when TokenSender rewards bank is depleted and deposit or withdraw is called.

Lines of code Vulnerability details Description In collateral deposit and withdraw flow, a fee is calculated as a percentage of user's requested amount. It is passed to the DepositHook and WithdrawHook, for example in deposit: uint256 amountAfterFee = amount - fee; if addressdepositHook != addres...

WithdrawHook doesn't check that userWithdrawLimitPerPeriod is less than globalWithdrawLimitPerPeriod

Lines of code Vulnerability details Impact WithdrawHook doesn't check that userWithdrawLimitPerPeriod is less than globalWithdrawLimitPerPeriod. It allows to user withdraw more then globalWithdrawLimitPerPeriod per period. Proof of Concept Function WithdrawHook.hook should not allow to withdraw...

Redeem should revert if the TokenSender.sol doesn’t have enough outputtoken to reimburse to users.

Lines of code https://github.com/prepo-io/prepo-monorepo/blob/49a7ed94272db013245d9364e69be713a8aef0a2/apps/smart-contracts/core/contracts/TokenSender.solL41 Vulnerability details According to the comments in the code of RedeemHook.hook function : Once a market has ended, users can directly settl...

Anyone can set the accountList object

Lines of code Vulnerability details Impact The setAccountList function which is the function that is responsible to set the account list object is made public with no access control on the AccountListCaller contract, Proof of Concept truffle console --networkId 555 compile attacker = "choose...

Overflow in BinMap can break pool

Lines of code Vulnerability details Impact The BinMap library performs multiplication on int32 values that can potentially overflow and cause the corresponding function calls to revert. The functions in question are used by essential Pool methods such as Pool.addLiquidity or Pool.swap and an...

Pool calls to toScale can revert

Lines of code Vulnerability details Pool calls to toScale can revert Impact A division by 0 would revert the code. If wrongly assigned value in constructor to tokenAScale or tokenBScale, toScale calls would always revert. Proof of Concept constructor uint256 fee, uint256 tickSpacing, int32...

Permit should not approve type(uint256).max

Lines of code Vulnerability details Impact The design of permits should not be allowed to approve typeuint256.max. If not, depositAndTrade can be called many times until the allowance value becomes 0. Proof of Concept We should only allow baseTokenAmount only for baseToken and...

A finding that cannot be disclosed at the moment

Lines of code Vulnerability details This finding couldn't be disclosed at the time when the contest was running. After consulting with CloudEllie, it was decided to create a stub report so the finding could be submitted after the contest is over. A checksum of the report:...

Attacker can make any function that relies on _satisfiesScoreRequirement reverts

Lines of code Vulnerability details Impact The NFTScoreRequirement contract have a function that checks the user score which diff it with the requiredScore variable, if it's higher then the function will continues, however the function that sets the requiredScore variable is made public with no...

PrePOMarket.sol : a sender who is not included in the account list can front run and mint prior to setting the _mintHook

Lines of code Vulnerability details Impact a sender who is not included in the account list can front run and mint prior to setting the mintHook. Proof of Concept The contract says, Minting will only be done by the team, and thus relies on the mintHook to enforce access controls. This is also why...

Misconfigured or malicious MANAGER Can drain, lose or steal ALL of the collateral.

Lines of code Vulnerability details Impact Collateral.sol allows withdrawal of funds to an arbitrary manager account. There are no inherent limitation to: 1. identity of manager address 2. withdrawable amount Details of 1. : manager setter is access controlled still may be misconfigured or a...

Mint might result in too few tokens minted

Lines of code Vulnerability details mint of PrePOMarket.sol allows the team to mint new short and long tokens in exchange for collateral tokens. Long and short tokens are default ERC20 tokens with 18 decimals, while the collateral ERC20 token uses the decimals of the underlying base token. Howeve...

Merged status is not updated and bins are not reset.

Lines of code Vulnerability details Impact Bins are not removed while removing liquidity and this can lead to serious problems. Proof of Concept While removing liquidity, if the target bin is a merged one mergeId!=0, the protocol does further calculations regarding mergeBinBalance. // Bin.sol 126...

Insufficient support for fee-on-transfer type of ERC20

Lines of code Vulnerability details Some ERC20 token implementations have a fee that is charged on each token transfer. This means that the transferred amount isn't exactly what the receiver will get. A call to IERC20token.transferrecipient, 100 with a fee-on-transfer FOT of 5% will entitle the...

Owner can rug PrePOMarket using re-initialized finalLongPayout.

Lines of code Vulnerability details Description In PrePOMarket.sol, the finalLongPayout represents the finalized value of a single long token in the market. It is settled post ICO / IPO according to predetermined rules. The issue is that this value may be re-initialized as many times as owner...

TWA Price should be updated in addLiquidity, removeLiqudity and swap and migrateBinsUpStack and transferLiquidity

Lines of code Vulnerability details Impact TWA Price should be updated in addLiquidity, removeLiqudity and swap and transferLiquidity Proof of Concept I want to quote from the documentation: In Maverick, the AMM smart contract tracks the time-weighted average price TWAP with a configurable lookba...

Changes not being stored in Delta.sol

Lines of code Vulnerability details Changes not being stored in Delta.sol Impact Functions working without proper storage dealing into unexpected behaviors Proof of Concept function combineInstance memory self, Instance memory delta internal pure if !self.skipCombine self.deltaInBinInternal +=...

Lack of access control in AllowedMsgSenders contract.

Lines of code Vulnerability details Impact setAllowedMsgSenders lacks access control. This enables anyone to set themselves as an allowed message sender and call the send in tokenSender.sol to transfer out any amount bypassing all the checks imposed in the hooks. Proof of Concept...

MintHook doesn't allow users with NFT score requirement to mint

Lines of code Vulnerability details Impact The depositHook contract checks that users are allowed to deposit either if they are on the allow list or if they have the required NFT score: if !accountList.isIncludedsender requiresatisfiesScoreRequirementsender, "depositor not allowed"; However, once...

Unsafe usage of ERC20 methods

Lines of code Vulnerability details There are many weird ERC20 tokens that don't follow the standard ERC20 interface. Depending on the ERC20 token, some transfer errors may result in passing unnoticed, or some successful transfers may be treated as failed. The current implementation assumes that...

Use wrong reserve values in Pool.addLiquidity()

Lines of code Vulnerability details Impact When doing Pool.addLiquidity, it would call bin.addLiquidity to add liquidity to the bins. And the calculation in bin.addLiquidity should be based on the amount of new tokens and the bin’s existing reserves. However, Pool.addLiquidity uses temp.deltaA an...

depositAndTrade::exactInputSingleParams the possible return values not checked

Lines of code Vulnerability details Impact The return values ​​from collateral.deposit and collateral.transferFrom are not used. This means that the values ​​returned by those functions will not be stored or used in the code. The swapRouter.exactInputSingle function also likely returns no value, ...

Upgraded Q -> M from #81 [1670783437328]

Judge has assessed an item in Issue 81 as M risk. The relevant finding follows: After carefully reading all submissions related to the use of selfdestruct, I will change my stance on this kind of issue. Changing the severity back to Medium. --- The text was updated successfully, but these errors...

Upgraded Q -> M from #268 [1670783513081]

Judge has assessed an item in Issue 268 as M risk. The relevant finding follows: Use of selfdestruct in FixedPrice.sol and OpenEdition.sol --- The text was updated successfully, but these errors were encountered: All reactions...

Upgraded Q -> M from #34 [1670783427149]

Judge has assessed an item in Issue 34 as M risk. The relevant finding follows: Selfdestruct is prone to being changed in a future hard fork --- The text was updated successfully, but these errors were encountered: All reactions...

Re-entrancy issue when executing safeTransfer in sweepToken function

Lines of code Vulnerability details Impact In the sweepToken function, we don't have a re-Entrancy check which will introduce the token lost by calling sweepToken multiple times during one request. Proof of Concept We don't have any check for the token and the token itself may call back into the...

Upgraded Q -> M from #506 [1670783534926]

Judge has assessed an item in Issue 506 as M risk. The relevant finding follows: selfDestruct will probably be deactivated soon --- The text was updated successfully, but these errors were encountered: All reactions...

Pool: The EMERGENCY state cannot be recovered

Lines of code Vulnerability details Impact The owner can call Pool.adminAction to set the Pool contract state to EMERGENCY, and in the EMERGENCY state, no liquidity can be added or transferred, and no swaps can be made. However, the Pool contract does not have a function to recover the EMERGENCY...

Value can be extracted from LPs

Lines of code Vulnerability details Impact Maverick allow liquidity to be moved based on TWAP to increase capital efficiency. However, the increased capital efficiency is not free. Consider a "right" move, the LP sell at lower price and move their liquidity to higher price, thus buying at higher...

LPDA price is not calculated correctly such that it won't end at the final price as expected

Lines of code Vulnerability details Impact LPDA price is not calculated correctly such that it won't end at the final price as expected Proof of Concept function getPrice public view returns uint256 Sale memory temp = sale; uint256 start, uint256 end = temp.startTime, temp.endTime; if...

Users can lose funds if they call buy with _amount larger than type(uint48).max.

Lines of code Vulnerability details Impact The function buy take amount of type uint256 as input. amount is used to check if msg.value is correct depending on the sale price as follows: requireamount sale.price == msg.value, "WRONG PRICE"; but is not casted to uint48 as done to calculate the newI...

The buy() function on Last Price Dutch Auction Sale doesn't check if auction ended, may lead to user loss asset

Lines of code Vulnerability details Impact The buy function on Last Price Dutch Auction Sale doesn't check if auction is ended, may lead to user loss asset if user call with amount 0 with msg.value 0 Proof of Concept Ideally if the max id finalId is reached, then the auction will end, so no user...

WPunk will become locked if liquidator ops to receive the underlying token instead of nToken

Lines of code Vulnerability details Impact WPunks received directly as the result of liquidation will be unable to be withdrawn Proof of Concept function withdrawPunkuint256 calldata punkIndexes, address to external nonReentrant INToken nWPunk = INToken Pool.getReserveDataaddressWPunk.xTokenAddre...

NFTFloorOracle trust feeder without any fallback

Lines of code Vulnerability details Impact In the current design, NFTFloorOracle take the median of valid feeder price as the oracle price. This design can be dangerous if more than half as few as 2 of the feeders decided to collude and publish malicious prices. If they set the price higher, they...

Price can deviate by much more than maxDeviationRate

Lines of code Vulnerability details Description NFTFloorOracle retrieves ERC721 prices for ParaSpace. maxPriceDeviation is a configurable parameter, which limits the change percentage from current price to a new feed update. function checkValidityaddress asset, uint256 twap internal view returns...

Pausing assets only affects future price updates, not previous malicious updates.

Lines of code Vulnerability details Description NFTFloorOracle retrieves ERC721 prices for ParaSpace. It is pausable by admin on a per asset level using setPauseasset, flag. setPrice will not be callable when asset is paused: function setPriceaddress asset, uint256 twap public onlyRoleUPDATERROLE...

NFTFloorOracle price feeders can be removed by anyone

Lines of code Vulnerability details Impact The nfts price feeders in the NFTFloorOracle contract should be added or removed only by the admin but because the removeFeeder function is missing the onlyRoleDEFAULTADMINROLE modifier any user can remove a feeder, this could impact the whole protocol i...

Oracle will become invalid much faster than intended on non-mainnet chains

Lines of code Vulnerability details Description NFTFloorOracle is in charge of answering price queries for ERC721 assets. EXPIRATIONPERIOD constant is the max amount of blocks allowed to have passed for the reading to be considered up to date: uint256 diffBlock = currentBlock - priceInfo.updatedA...

Some arbitrary feeders will not be removable, even by admin.

Lines of code Vulnerability details Description NFTFloorOracle receives data from different "feeders". They are added using addFeeders and removed by removeFeeder. Feeders are managed by two data structures. feeders is an array, each element in the address of the feeder. feederPositionMap maps...

NFTFloorOracle's asset and feeder structures can be corrupted

Lines of code Vulnerability details NFTFloorOracle's addAsset and addFeeder truncate the assets and feeders arrays indices to 255, both using uint8 index field in the corresponding structures and performing uint8assets.length - 1 truncation on the new element addition. 2^8 - 1 looks to be too tig...

Wrong logic in function _removeFeeder() cause feeders unable to be removed.

Lines of code Vulnerability details Impact In NFTFloorPrice, there are multiple components to allow easy adding/removing keepers address public feeders - List of all keepers' address mappingaddress = FeederPositionindex, register private feederPositionMap When adding new keeper, new keeper addres...

Attacker can drain pool using executeBuyWithCredit with malicious marketplace payload.

Lines of code Vulnerability details Description Paraspace supports leveraged purchases of NFTs through PoolMarketplace entry points. User calls buyWithCredit with marketplace, calldata to be sent to marketplace, and how many tokens to borrow. function buyWithCredit bytes32 marketplaceId, bytes...

User can pass auction recovery health check easily with flashloan

Lines of code Vulnerability details Description ParaSpace features an auction mechanism to liquidate user's NFT holdings and receive fair value. User has the option, before liquidation actually happens but after auction started, to top up their account to above recovery factor 1.5 instead of 1 an...