Chainlink latestRoundData validation

Lines of code Vulnerability details Impact Oracle returns Chainlink latestRoundData without proper validation, e.g.: function getUnderlyingPriceaddress underlying ... ,answer,,, = AggregatorV3InterfacechainLinkAggregatorMapunderlying.latestRoundData; answer /= 100; And other functions that call...

NestedFactory: User can utilise accidentally sent ETH funds via processOutputOrders() / processInputAndOutputOrders()

Lines of code Vulnerability details Impact Should a user accidentally send ETH to the NestedFactory, anyone can utilise it to their own benefit by calling processOutputOrders / processInputAndOutputOrders. This is possible because: 1. receive has no restriction on the sender 2. processOutputOrder...

NestedFactory: User can utilise accidentally sent ETH funds via processOutputOrders() / processInputAndOutputOrders()

Lines of code Vulnerability details Impact Should a user accidentally send ETH to the NestedFactory, anyone can utilise it to their own benefit by calling processOutputOrders / processInputAndOutputOrders. This is possible because: 1. receive has no restriction on the sender 2. processOutputOrder...

Reentrancy

Lines of code Vulnerability details Impact Potential Reentrancy Proof of Concept Reentrancy in ConcurRewardPool.claimRewardsaddress contracts/ConcurRewardPool.sol34-40: External calls: - IERC20tokensi.safeTransfermsg.sender,getting contracts/ConcurRewardPool.sol37 State variables written after th...

Duplicate tokens can be added which might cause DOS

Lines of code Vulnerability details Impact Duplicate tokens can be added which could exceed maxHoldingsCount and thus rejecting all functions in Factory Proof of Concept 1. Observe the store function in NestedRecords.solL111 function store uint256 nftId, address token, uint256 amount, address...

Users’ staking rewards can get redeemed by other users

Lines of code Vulnerability details Can be medium or high: When a user deposits through ConvexStakingWrapper, the contract calls the MasterChef's deposit function and deposits to it with recipient = msg.sender, but the MasterChef actually updates the deposited amount of the ConvexStakingWrapper...

Reentrancy in ConcurRewardPool::claimRewards

Lines of code Vulnerability details Impact Any address that has nonzero reward for a token tokensi is able to drain all contact token funds if the transfer function is reentrant for example, ERC777 token. As tokensi is an arbitrarily implemented, a reentrant transfer function can be assumed to be...

[WP-H8] ConvexStakingWrapper.sol#_calcRewardIntegral Wrong implementation can disrupt rewards calculation and distribution

Lines of code Vulnerability details uint256 bal = IERC20reward.token.balanceOfaddressthis; uint256 dreward = bal - reward.remaining; // send 20 % of cvx / crv reward to treasury if reward.token == cvx || reward.token == crv IERC20reward.token.transfertreasury, dreward / 5; dreward = dreward 4 / 5...

MasterChef.add must update pools first

Lines of code Vulnerability details The MasterChef.add function changes the total pool allocation but does not update other pools first. When other pools are finally updated at some point, then accConcurPerShare will be wrongly computed with their smaller allocPoint / newTotalAllocPoint share eve...

tokenOut is only required on finalize

Lines of code Vulnerability details Impact I thought of a potential rug pull from the owner: when users buy tokenOut, it is not required that the contract has already escrowed enough tokenOut. It is only required when finalizing the sale: require tokenOut.balanceOfaddressthis = totalTokenOutBough...

UniV2ClassDex.sol#uniClassSell() Tokens with fee on transfer are not fully supported

Handle WatchPug Vulnerability details function uniClassSellDexInfo memory dexInfo, address buyToken, address sellToken, uint sellAmount, uint minBuyAmount, address payer, address payee internal returns uint buyAmount address pair = getUniClassPairbuyToken, sellToken, dexInfo.factory;...

Slippage protection

Handle pauliax Vulnerability details Impact Consider adding a configurable slippage parameter here to prevent users suffering from sandwitch bots: minAmountsOut0 = 0; minAmountsOut1 = 0; IVault.ExitPoolRequest ... minAmountsOut and used in both sNOTE and TreasuryManager contracts:...

MINTING to collateralVault could inflating totalsupply, without giving the balance to anyone

Handle Tomio Vulnerability details Impact First of all, this is an address zero issue, however, this could lead to an imbalance between total supply circulating and the actual balance that was assigned to another user, in the...

OpenLevV1Lib's and LPool's doTransferOut functions call native payable.transfer, which can be unusable for smart contract calls

Handle hyh Vulnerability details Impact When OpenLev operations use a wrapped native token, the whole user withdraw is being handled with a payable.transfer call. This is unsafe as transfer has hard coded gas budget and can fail when the user is a smart contract. This way any programmatical usage...

Use of deprecated Chainlink API

Handle 0x1f8b Vulnerability details Impact Deprecated API stops working. Prices cannot be obtained. Protocol stops and contracts have to be redeployed. Proof of Concept The contracts use Chainlink’s deprecated API latestAnswer. Such functions might suddenly stop working if Chainlink stopped...

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

approve() function does not require that _token and _spender args are unique

Handle jayjonah8 Vulnerability details Impact In L1Escrow.sol the approve function takes in a token to approve the spender to be able to spend but does not check that the token and spender are different addresses which can leave the protocol open to be expo;ited. Proof of Concept function approve...

No slippage protection on _swapUstToUnderlying can lead to lost funds

Handle harleythedog Vulnerability details Impact The function swapUstToUnderlying exists to swap Ust to underlying tokens. The last argument to exchangeunderlying is mindy, which specifies the minimum number of underlying to be returned from the swap. Currently, this value is set to 0, so the...

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

Re-entrancy in _createDeposit allows stealing tokens

Handle cmichel Vulnerability details The Vault.createDeposit function first caches the current total shares and underlying, and then iterates over all claims using a call to createClaim. Only afterwards, does it pull in the required total amount in the deposit. function depositDepositParams...

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

TimeswapPair.sol#borrow() Attacker can increase pool.state.y to an arbitrary target value

Handle WatchPug Vulnerability details TimeswapPair.solborrow takes a user input value of yIncrease, and the BorrowMath.check at L316 only checks for a minimal yIncrease, which means that it allows the state of pool.state.y to increase by the value of the user's input as long as it's larger than...

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

safeDecimals can revert causing DoS

Handle sirhashalot Vulnerability details Impact The safeDecimals function, found in the SafeMetadata.sol contract and called in 3 different Timeswap Convenience contracts, can cause a revert. This is because the safeDecimals function attempts to use abi.decode to return a uint8 when data.length =...

Unbounded loop on array can lead to DoS

Handle robee Vulnerability details The attacker 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. This is an High Risk issue since thos...

Reentrancy allows attacker to lock the funds deposited in the contract

Handle tqts Vulnerability details Impact It is possible for attackers to exploit a reentrancy issue in lockuint256,uint256,address that would allow them to lock all funds deposited prior to the attack. Proof of Concept The problem lies in the lockuint256,uint256,address function, that transfers t...

Anyone can steal XDEFI from the XDEFIDistribution contract and make the contract insolvent

Handle onewayfunction Vulnerability details Impact Anyone can steal XDEFI from the XDEFIDistribution contract, thereby making the contract insolvent. In the process, they also make the updateDistribution function uncallable -- and thus make the value of pointsPerUnit unchangeable. This comes with...

100

Handle 0v3rf10w Vulnerability details Impact Detailed description of the impact of this finding. Proof of Concept Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept. Tools Used Recommended Mitigation Steps --- The...

SafeERC20 Is Not Used On Token Transfers

Handle leastwood Vulnerability details Impact The transfer function is used on WETH and vault token transfers over OpenZeppelin's SafeERC20 library. As a result, these function may return unintended results which are not handled correctly. The SafeERC20 library ensures transfer and transferFrom...

SingleTokenJoinV2 doesn't take input deadline to consideration

Handle kenzo Vulnerability details SingleNativeTokenExitV2 takes as input from the user a deadline for the trades. However, it does not use this input for the actual trade but sets the deadline to be block.timestamp. Impact Trades will not work as expected. User might set a deadline for the trade...

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

Fee calculation is slightly off

Handle gzeon Vulnerability details Impact The fee calculation uint256 timeDiff = block.timestamp - lastFee; uint256 feePct = timeDiff licenseFee / ONEYEAR; uint256 fee = startSupply feePct / BASE - feePct; tries to calculate a fee such that fee/supply+fee = %fee using a simple interest formula i....

IsContract Function Usage

Handle defsec Vulnerability details Impact the isContract function that uses EXTCODESIZE was discovered to be hackable. The function will return false if it is invoked from a contract's constructor because the contract has not been deployed yet. The code should be used very carefully, if at all, ...

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

LockeERC20 is vulnerable to frontrun attack

Handle egjlmn1 Vulnerability details Impact a user can steal another user's tokens if he frontrun before he changes the allowance. The approve function receives an amount to change to. Lets say user A approved user B to take N tokens, and now he wants to change from N to M, if he calls approveM t...

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

Permissions - return values not checked when sending ETH

Handle ScopeLift Vulnerability details Impact On lines 85 and 101, ETH is transferred using a .call to an address provided as an input, but there is no verification that the call call succeeded. This can result in a call to emergencyWithdrawGAS or partialWithdrawGAS appearing successful but in...

MEV miner can mint larger than expected UDT total supply

Handle elprofesor Vulnerability details Impact UnlockProtocol attempts to calculate gas reimbursement using tx.gasprice, typically users who falsify tx.gasprice would lose gas to miners and therefore not obtain any advantage over the protocol itself. This does present capabilities for miners to...

Wrong design/implementation of freeTrial allows attacker to steal funds from the protocol

Handle WatchPug Vulnerability details The current design/implementation of freeTrial allows users to get full refund before the freeTrial ends. Plus, a user can transfer partial of thier time to another user using shareKey. This makes it possible for the attacker to steal from the protocol by...

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

_transferBurn should reduce totalSupply

Handle WatchPug Vulnerability details function transferBurn address sender, address recipient, uint256 amount, uint256 burnt internal uint256 senderBalance = balancessender; requiresenderBalance = amount + burnt, "OVL:balance= amount + burnt, "OVL:balanceamount+burnt"; unchecked balancessender =...

Transmuter yield can be gamed by only staking when yield is distributed

Handle cmichel Vulnerability details The Transmuter.distribute function distributes the yield to the buffer which is then distributed to all stakers over a TRANSMUTATIONPERIOD, see runPhasedDistribution and updateAccount. If the elapsed time from the last phase distribution is greater than the...

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

Zap contract's redeem() function might transfer more tokens than the user should be able to receive

Handle Ruhum Vulnerability details Impact The redeem function transfers the whole balance of the specified token to the user. That can be more than the amount the user should receive. Because at the time of the call to redeem the Zap contract's balance of the passed token might not be 0. Proof of...

LinearVesting does not calculate vested amount linearly

Handle xYrYuYx Vulnerability details Impact These calculations are incorrect for linear vesting. Proof of Concept i.e. if start amount is 10000, and duration is 100 seconds. After 50 seconds, user can claim 5000 which is 50% After another 10 seconds, user need to claim 1000 which is 10%, but...

FSDVesting: Restrict updateVestedTokens() calls to only FSD token contract

Handle hickuphh3 Vulnerability details Impact The updateVestedTokens increases the amount of tokens to be vested for a beneficiary. There is no access restriction to the function. The intended total vesting duration is 30 months with a 12-month cliff where 5% is immediately unlocked, and the...

Unbounded loop in TwapOracle.update can result in oracle being locked

Handle TomFrench Vulnerability details Impact Loss of ability of TwapOracle to update should too many pools be added. Proof of Concept TwapOracle allows an unlimited number of pairs to be added and has no way of removing pairs after the fact. At the same time TwapOracle.update iterates through al...