anyone can change Parameters state

Handle jayjonah8 Vulnerability details Impact In BurnFlashStakeDeposit.sol the parameterize function can be called by anyone setting all the Parameters state in the contract. A user should not be able to do this. This function deals with important governance decisions being execute and should onl...

Unbounded loop on array that can only grow can lead to DoS

Handle robee Vulnerability details A malicious attacker that is also a protocol owner can push unlimitedly to an array, that some function loop over this array. If increasing the array size enough, calling the function that does a loop over the array will always revert since there is a gas limit...

Unchecked returns in multiple place

Handle 0v3rf10w Vulnerability details Impact Multiple checks needed Proof of Concept L1LPTGateway.outboundTransferaddress,address,uint256,uint256,uint256,bytes contracts/L1/gateway/L1LPTGateway.sol80-123 ignores return value by TokenLikel1Token.transferFromfrom,l1LPTEscrow,amount...

Missing slippage/min-return check in the curve Pool

Handle defsec Vulnerability details Impact Trades can happen at a bad price and lead to receiving fewer tokens than at a fair market price. The attacker's profit is the protocol's loss. Proof of Concept The NonUSTStrategy contract is missing slippage checks which can lead to being vulnerable to...

No slippage tolerance set in NonUSTStrategy

Handle palina Vulnerability details Impact The exchange performed in NonUSTStrategy.sol via Curve is executed with "0" as the minimum amount received as the result of the operation, which is likely to be exploited by front-running and may lead to the loss of funds. Proof of Concept...

When a user performs a withdrawal operation, a rollback that is not considered by the program may be triggered.

Handle ACai Vulnerability details Impact When the Vault contract deposits all/most of the token into the strategy contract, so that the remaining tokens in the Vault contract are less than the user's deposit, the user's withdrawal operation will result in a rollback that is not considered by the...

safeSymbol() can revert causing DoS

Handle sirhashalot Vulnerability details Impact The safeSymbol function, found in the SafeMetadata.sol contract and called in 4 Timeswap Convenience contracts in the symbol functions, can cause a revert. This could make the 4 contracts not compliant with the ERC20 standard for certain asset pairs...

Reliance on exact balance

Handle pauliax Vulnerability details Impact functions addLiquidity721WETH and addLiquidity1155WETH rely on the exact balance of tokens: uint256 balance = count BASE; // We should not be experiencing fees. requirebalance == IERC20Upgradeablevault.balanceOfaddressthis, "Did not receive expected...

It might not be possible to withdraw tokens from the basket

Handle Czar102 Vulnerability details Impact When enough basket token owners exit, it will be impossible to exit pool with the last MINAMOUNT tokens because of this check. This will result in locking some tokens forever. Recommended Mitigation Steps Consider resigning from this check or performing...

No checks if given product is created by the factory

Handle 0x0x0x Vulnerability details An attacker can create a fake product. Collateral contract does not check whether the given product is created by the factory. A malicious product can return arbitrary maintenance amounts, therefore they can make any deposit to fake product stuck simply return...

Aave's share tokens are rebasing breaking current strategy code

Handle cmichel Vulnerability details When depositing into Aave through the AaveYield.lockTokens contract strategy, one receives the sharesReceived amount corresponding to the diff of aToken balance, which is just always the deposited amount as aave is a rebasing token and 1.0 aToken = 1.0...

Allowing more than 256 epochs leads to loss of funds

Handle cmichel Vulnerability details The TwabRewards contract has an implicit restriction of 256 epochs per promotion as it uses a bitmask in a uint256 to mark claimed epochs 0-255, see isClaimedEpoch. "/// @dev We pack epochs claimed by a user into a uint256. So we can't store more than 255...

AaveVault is vulnerable to flashloan sandwich attack

Handle WatchPug Vulnerability details In the current implementation of AaveVault, tvl is only updated after each deposit/withdraw, which means unclaimed yields won't be included in tvl until someone deposit/withdraw, making it vulnerable to flashloan sandwich attack that steals pending yields. Po...

ChiefTrader.sol Wrong implementation of swapExactInput() and swapExactOutput()

Handle WatchPug Vulnerability details When a caller calls ChiefTrader.solswapExactInput, it will call ITradertraderAddress.swapExactInput. return ITradertraderAddress.swapExactInput0, amount, recipient, path, options; However, in the current implementation, inputToken is not approved to the...

Public variable unstreamed can be smaller than ∑ts.tokens due to unstreamed not being updated in withdraw()

Handle WatchPug Vulnerability details unstreamed is a public variable, and it's been actively managed in stake, updateStreamInternal. However, since users can also withdraw unstreamed depositToken, the global variable unstreamed should be updated in withdraw as well. For example: 1. Alice deposit...

Dutch auction can be manipulated

Handle gzeon Vulnerability details Impact When malt is under-peg and the swing trader module do not have enough capital to buy back to peg, a Dutch auction is triggered to sell arb token. The price of the Dutch auction decrease linearly toward endprice until endAuction is called. endAuction is...

transferFrom() and shareKey() doesn't check for _from != _recipient

Handle GiveMeTestEther Vulnerability details Impact / POC There is no check from != recipient in transferFrom https://github.com/code-423n4/2021-11-unlock/blob/ec41eada1dd116bcccc5603ce342257584bec783/smart-contracts/contracts/mixins/MixinTransfer.solL109 the key manager/approver can expire the k...

Fees can be more than 100%

Handle cmichel Vulnerability details The Overlayv1Mothership.adjustGlobalParams function allows setting a fee and feeBurn that is greater than 100%. Recommended Mitigation Steps Validate that fee and feeBurn is less than ONE = 1e18. --- The text was updated successfully, but these errors were...

Missing maxNumberOfKeys checks in shareKey and grantKey

Handle kenzo Vulnerability details More keys can be minted than maxNumberOfKeys since shareKey and grantKey do not check if the lock is sold out. Impact More keys can be minted than intended. Proof of Concept In both shareKey and grantKey, if minting a new token, a new token is simply minted and...

Using/Importing ERC1155Supply.sol from openzeppelin version 4.3.2

Handle hubble Vulnerability details Using/importing openZeppelin version 4.3.2 file ERC1155Supply.sol which has a security advisory. Reference : GHSA-wmpv-c2jp-j2xg Proof of Concept File : collateral/OverlayV1OVLCollateral.sol line 7: import...

function mint() No Tokens Will be Transfer to _recipient

Handle Meta0xNull Vulnerability details Impact mintrecipient, amount; At the end of mint, mint seems like responsible to transfer tokens to recipient but there is no code no function of mint in this contract. Thus No Tokens Will be Transfer to recipient. recipient who do not Receive Their Tokens...

NestedFactory.sol#_submitInOrders() Wrong implementation cause users to be overcharged

Handle WatchPug Vulnerability details When executing orders, the actual amountSpent + feesAmount can be lower than inputTokenAmount, the unspent amount should be returned to the user. However, in the current implementation, the unspent amount will be taken as part of the fee. function...

FeeSplitter: Unbounded number of shareholders can cause DOS

Handle GreyArt Vulnerability details Impact There is no limit to the number of shareholders. It is therefore possible to set a large number of shareholders such that sendFees will run out of gas when adding shares to each shareholder. This will cause denial of service to all NestedFactory...

No way to remove GasThrottle after deployment

Handle TomFrench Vulnerability details Impact Potential DOS on swaps Proof of Concept BasePool and BasePoolV2 make use of a validateGas modifier on swaps which checks that the user's gas price is below the value returned by FASTGASORACLE. Should FASTGASORACLE be compromised to always return zero...

Early user can break addLiquidity

Handle WatchPug Vulnerability details uint256 totalLiquidityUnits = totalSupply; if totalLiquidityUnits == 0 liquidity = nativeDeposit; // TODO: Contact ThorChain on proper approach In the current implementation, the first liquidity takes the nativeDeposit amount and uses it directly. However,...

Wrong design of swap() results in unexpected and unfavorable outputs

Handle WatchPug Vulnerability details The current formula to calculate the amountOut for a swap is: function calculateSwap uint256 amountIn, uint256 reserveIn, uint256 reserveOut public pure returns uint256 amountOut // x Y X uint256 numerator = amountIn reserveIn reserveOut; // x + X ^ 2 uint256...

Lack Of Return Value Check On the Oracle

Handle defsec Vulnerability details Impact During the code review, It has been seen that oracle return value has not been checked on the function. If oracle is returned price as a 0, balance of shares will be zero. Proof of Concept 1. Navigate to "" 2. The return value pricePerShare function has...

Users could lose funds if owner took out reward which is not multiplier of allocatedTokensPerEpoch

Handle xYrYuYx Vulnerability details Impact User could lost funds if owner take reward which is not multiplier of allocatedTokensPerEpoch. Proof of Concept This is my test case to proof this issue. This issue occur because of Line 104 Ex. If allocatedTokensPerEpoch is 1 CQT. and now available epo...

QuickAccManager.sol Potential replay attack

Handle WatchPug Vulnerability details This issue is possibly duplicate with a previous issue named "Potential replay attack" In QuickAccManager.solsendTxns and QuickAccManager.solsendTransfer, addressidentity is not included in the txHash, makes it possible to replay the transaction on another...

PoolCommitter.sol#commit() Calculation of amountOut is wrong

Handle WatchPug Vulnerability details else if commitType == CommitType.LongBurn // long burning: pull in long pool tokens from committer // A theoretical amount based on current ratio. Used to get same units as minimumCommitSize uint256 amountOut = PoolSwapLibrary.getWithdrawAmountOnBurn...

Swivel: implementation for initiateZcTokenFillingZcTokenExit is incorrect

Handle itsmeSTYJ Vulnerability details Impact In initiateZcTokenFillingZcTokenExit , this comment // transfer underlying tokens - the premium paid + fee in underlying to swivel from sender is incorrect because you are actually transferring the underlying tokens - premium paid to the maker from...

_addLiquidity will lose user funds due to frontrunning.

Handle tensors Vulnerability details Impact If addLiquidity is ever called with funds at stake anything more than a few thousand dollars it becomes profitable for MEV bots and other frontrunners to frontrun the addLiquidity call by skewing the pool reserves lowering the amount of LP tokens return...

Possibility to Stake Twice

Handle verifyfirst Vulnerability details Impact Potential for some users to double-stake their Yaxis. Proof of Concept // SPDX-License-Identifier: MIT pragma solidity 0.6.12; import "@openzeppelin/contracts/token/ERC20/ERC20.sol"; import "@openzeppelin/contracts/token/ERC20/SafeERC20.sol"; // yAx...

removeToken would break the vault/protocol.

Handle jonah1005 Vulnerability details removeToken would break the vault. Impact There's no safety check in Manager.sol's removeToken. Manager.solL454-L487 1. The token would be locked in the original vault. Given the current design, the vault would keep a ratio of total amount to save the gas...

ChainLink price data could be stale

Handle cmichel Vulnerability details Vulnerability Details There is no check in ExchangeRate.buildExchangeRate if the return values indicate stale data. This could lead to stale prices according to the Chainlink documentation: under current notifications: "if answeredInRound roundId could indicat...

executing instruction outside code can lead to failing transfer

Handle Omik Vulnerability details Impact in the , is handling transfer and transferfrom, and checking the return value of the transfer and transferfrom, but the checking is happening outside the code, therefore if the transfer successfull it will still return false Proof of Concept 1. deploy this...

Owner has a rugpull function

Handle tensors Vulnerability details Impact The owner of the contract has a rugpull function. This can be unsafe if the private key for the owner account falls into the wrong hands, allowing instant withdrawal of all the funds. In general, having a single point of failure like this is not...

Unchecked return value from transfer()

Handle JMukesh Vulnerability details Impact It is usually good to add a require-statement that checks the return value or to use something like safeTransfer; unless one is sure the given token reverts in case of a failure. Proof of Concept Tools Used manual review Recommended Mitigation Steps che...

Missing events/timelocks for owner/admin only functions that change critical parameters

Handle 0xRajeev Vulnerability details Impact Owner/admin only functions that change critical parameters should emit events and have timelocks. Events allow capturing the changed parameters so that off-chain tools/interfaces can register such changes with timelocks that allow users to evaluate the...

[Bug] A critical bug in bps function

Handle hrkrshnn Vulnerability details A critical bug in bps function: PoolBase.sol function bps internal pure returns IERC20 rt // These fields are not accessible from assembly bytes memory array = msg.data; uint256 index = msg.data.length; // solhint-disable-next-line no-inline-assembly assembly...

safeTransferFrom in TransferHelper is not safeTransferFrom

Handle jonah1005 Vulnerability details Impact A non standard erc20 token would always raise error when calling safeTransferFrom. If a user creates a USDT/DAI pool and deposit into the pool he would find out there's never a counterpart deposit. Proof of Concept TransferHelper does not uses SafeERC...

Expired transfers will lock user funds on the sending chain

Handle 0xRajeev Vulnerability details Impact The cancelling relayer is being paid in receivingAssetId on the sendingChain instead of in sendingAssetID. If the user relies on a relayer to cancel transactions and that receivingAssetId asset does not exist on the sending chain assuming only...

how to protect for weird erc20 tokens

Handle gpersoon Vulnerability details Impact The protocol only works as expected when the ERC20 tokens are well behaved. If rebasing tokens or malicious token contracts are used then unexpected results can happen. I don't see any code to restrict the tokens or verify the tokens. Proof of Concept...

Use of deprecated Chainlink function latestAnswer

Handle shw Vulnerability details Impact According to Chainlink's documentation, the latestAnswer function is deprecated. This function does not error if no answer has been reached but returns 0, causing an incorrect price fed to the Buoy3Pool. Proof of Concept Referenced code: Buoy3Pool.solL207...

Malicious owner can arbitrarily change fee to any % value

Handle 0xRajeev Vulnerability details Impact Tracer protocol like any other allows market creators to charge fees for trades. However, a malicious/greedy owner can arbitrarily change fee to any % value and without an event to observe this change or a timelock to react, there is no easy way for...

Card affiliate payouts are skipped if a single card does not have an affiliate

Handle cmichel Vulnerability details Vulnerability Details The Market.initialize function sets the cardAffiliateCut to zero if a single cardAffiliateAddresses is the zero address. for uint256 i = 0; i numberOfCards; i++ if cardAffiliateAddressesi == address0 cardAffiliateCut = 0; Impact Even if a...

User deposits can be turned into sponsors and then be stolen

Handle cmichel Vulnerability details Vulnerability Details When a user deposits to the treasury they first approve the contract and then call its deposit action which performs an ERC20.transferFrom. It's possible for an attacker to frontrun the final deposit transaction after the user approval an...

Anyone can affect deposits of any user and turn the owner of the token

Handle adelamo Vulnerability details Impact On RCTreasury, we have the method collectRentUser. This method is public, so anyone can call it using whatever user and whatever timestamp. So, calling this method using user = XXXXX and timeToCollectTo = typeuint256.max, would make isForecloseduser =...

User can redeem more tokens by artificially increasing the chi accrual

Handle shw Vulnerability details Impact A user can artificially increase the chi accrual after maturity by flash borrow on Compound, which affects the exchange rate used by the chi oracle. As a result, the user redeems more underlying tokens with the same amount of fyTokens since the accrual is...

A malicious receiver can cause another receiver to lose out on distributed fees by returning false for tokensReceived when receiveRewards is called on their receiver contract.

Handle janbro Vulnerability details Summary A malicious receiver can cause another receiver to lose out on distributed fees by returning false for tokensReceived when receiveRewards is called on their receiver contract. Risk Rating Medium Vulnerability Details A malicious receiver can cause anoth...