Access control for hook function in RedeemHook Contract is inconsistent with the implementation.

Lines of code Vulnerability details Impact Access control for hook function in RedeemHook Contract is inconsistent with the implementation. Since the function involves a transfer of fees to Treasury, I've marked it as MEDIUM RISK RedeemHook checks if sender is in a list of pre-approved accounts i...

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

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

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

Deposit record does not update properly for withdrawals

Lines of code Vulnerability details Impact A side effect of calling deposit in the Collateral contract is that the userToDeposits map in the DepositRecord contract is updated. However, when the user withdraws funds, this userToDeposits map is not updated to reflect their total deposited amount. T...

WithdrawHook.hook doesn't validate amount when lastUserPeriodReset + userPeriodLength < block.timestamp

Lines of code Vulnerability details Impact WithdrawHook.hook doesn't validate amount when lastUserPeriodReset + userPeriodLength block.timestamp. As result user can withdraw more than he is allowed. Proof of Concept Function WithdrawHook.hook should not allow users to withdraw more than is allowe...

A whale user is able to cause freeze of funds of other users by bypassing withdraw limit

Lines of code Vulnerability details Description In Collateral.sol, users may withdraw underlying tokens using withdraw. Importantly, the withdrawal must be approved by withdrawHook if set: function withdrawuint256 amount external override nonReentrant uint256 baseTokenAmount = amount...

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

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

Broken access control on TokenSenderCaller contract

Lines of code Vulnerability details Impact The setTreasury function which is the function that is responsible to set the treasury role is made public with no access control, which makes attacker escalate his privileges to treasury Proof of Concept truffle console --networkId 555 compile user =...

Unsafe downcasting in TWA.sol truncate TWAP price

Lines of code Vulnerability details Impact Unsafe casting operation in TWA.sol truncation price. Proof of Concept the pool relies on the TWAP price to function properly, however, the code in TWA.sol sliently downcasting the price, which can truncate the price and affect user's trading unexpectedl...

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

TWA update is not correct

Lines of code Vulnerability details Impact Time-warped-price is updated incorrectly and this affects moving bins. Proof of Concept The protocol updates twa on every swap and uses that to decide how to move bins. But in the function swap, the delta's endSqrtPrice can not contribute negatively to t...

Incompatibility With Rebasing/Deflationary/Inflationary tokens

Lines of code Vulnerability details Impact The Maverick AMM does not appear to support rebasing/deflationary/inflationary tokens whose balance changes during transfers or over time. The necessary checks include at least verifying the amount of tokens transferred to contracts before and after the...

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

Ability to redeem excess collateral

Lines of code Vulnerability details Vulnerability details redeem of PrePOMarket.sol redeems long and short 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. Th...

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

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

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

limits are not correctly enforced in withdrawHook contract

Lines of code Vulnerability details Impact withdrawHook contract checks that inside a specified length of time only certain amount of withdrawal are possible per user and globally. But on every period reset the allowed withdraw limit check is missing. And a user can withdraw more that is allowed...

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

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

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

[NAZ-M2] Usage of send() Can Result In Revert

Lines of code Vulnerability details Impact Several functions are sendusing is used by the across several functions to transfer ETH/WETH. send uses a fixed amount of gas, which was used to prevent reentrancy. However this limit your protocol to interact with others contracts that need more than th...

Attacker can set anyone as the tokenSender role

Lines of code Vulnerability details Impact The setTokenSender function which is the function that is responsible to set the token sender role is made public with no access control, which makes attacker escalate his privileges to the token sender role Proof of Concept truffle console --networkId 5...

Admin may take non-fee baseTokens from Collateral.sol

Lines of code Vulnerability details Description In Collateral.sol, deposit and withdraw functions are subject to fees. They are either sent directly to the treasure in deposit / withdraw hooks, or are kept in the Collateral contract for safekeeping. Later, manager can use managerWithdraw function...

depositAndTrade function is incomplete & does not use returnValue of UniswapV3 router

Lines of code Vulnerability details Impact depositAndTrade function seems to be incomplete - the tokenOutput from swapRouter is currently owned by DepositTradeHelper account and needs to be transferred back to msg.sender who initiated this transaction. Since this contract doesn't seem to be part ...

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

Pool._amountToBin() returns a wrong value when protocolFeeRatio = 100%.

Lines of code Vulnerability details Impact Pool.amountToBin returns a larger value than it should when protocolFeeRatio = 100%. As a result, bin balances might be calculated wrongly. Proof of Concept delta.deltaInBinInternal is used to update the bin balances like this. if tokenAIn binBalanceA +=...

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

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

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

Possible malicious _uri in initialize() of Escher721.sol

Lines of code Vulnerability details Impact Loss of user funds if user approves malicious transaction Description Trace: createContract of Escher721.Factory.sol calls initialize of Escher721.sol The initialize function sets tokenUriDelegate to the passed in uri address. There's no input validation...

underflow in the getPrice() function can block the buy and refund in the LPDA sale

Lines of code Vulnerability details Impact In the LPDA sale the price decrease in values after each second, and when creating the sale the value of the lowest price possible is not checked, so the price could go below zero at a given timestamp which will lead to an underflow in the getPrice...

Update initializer library to prevent reentrancy during initialization

Lines of code Vulnerability details Since proxied contracts do not make use of a constructor, it's common to move constructor logic to an external initializer function, usually called initialize. It then becomes necessary to protect this initializer function so it can only be called once. The...

Oracle does not treat upward and downward price movement the same in validity checks, causing safety issues in oracle usage.

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. We can see how priceDeviation is calculated and compared to...

Loss of ETH for NFT buyers in LPDA contract

Lines of code Vulnerability details Impact The buy function of LPDA sale contract can be invoked with 0 as the input value and 0 ETH as the sent valuemsg.value = 0. The buy function automatically ends the sale when newId == sale.finalId and distributes ETH to feeReceiver and saleReceiver. Since t...

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

Implementation can be self destruct by deployer, effectively break all running sale and lock all assets.

Lines of code Vulnerability details Impact Sale in Escher is deployed using minimal proxy pattern, where there is only 1 implementation contract is deployed to save deployment gas. Also, in Open Edition and FixedPrice sale, when sale is not started yet, owner can cancel it, self destruct the prox...

Denial of Service at the LPDA.sol contract due to not be capable of minting an id already minted.

Lines of code Vulnerability details Impact The ether earned by the creator and the fees for the platform can get frozen. Proof of Concept The only way the creator can get out the ether earned by the sale and also the platform to get the feest is by the next code snippet from the buy funtion at th...

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

Discrepency in the Uniswap V3 position price calculation because of decimals

Lines of code Vulnerability details Impact When the squared root of the Uniswap V3 position is calculated from the getOracleData function, the price may return a very high number in the case that the token1 decimals are strictly superior to the token0 decimals. See: The reason is that at the...

LAST PRICE DUTCH AUCTION SALE (LPDA) CAN BE EXPLOITED

Lines of code Vulnerability details Impact The function logic of buy in LPDA.sol can be exploited by shrewd buyers to achieve the lowest finalPrice possible. Proof of Concept LPDA.solL58-L89 function buyuint256 amount external payable uint48 amount = uint48amount; Sale memory temp = sale;...

Downcast can lead to overflow and impact the functionality to remove an asset in NFTFloorOracle

Lines of code Vulnerability details Impact When adding a new asset, the index for the last asset will be saved and downcasted on L284. If the number of assets is bigger than 256, the downcasting operation will lead to a silent overflow. Consequentially, the incorrect index will be stored. This ca...

finalise() lacks authenticate calls to this method as anyone can access it.

Lines of code Vulnerability details Impact Unprotected call to a function sending Ether to an arbitrary address. This can be exploited by attackers . Proof of Concept function finalize public Sale memory temp = sale; requireblock.number = temp.endTime, "TOO SOON";...

Decimals of the oracle is not checked

Lines of code Vulnerability details Impact Decimals of the Chainlink oracle is not checked, it is assumed to have the same unit as BASECURRENCYUNIT but it might not always be the case. If the oracle use a different number of decimals all the price will be wrong by magnitudes. Proof of Concept...