Incorrect repayAmount used in CNote

Lines of code Vulnerability details Impact repayAmount was used instead of repayAmountFinal in CNote.repayBorrowFresh. This will lead to revert if the borrower want to repay full amount with typeuint.max Proof of Concept uint actualRepayAmount = doTransferInpayer, repayAmount; Recommended...

Base rate update frequency can be bypassed by an admin

Lines of code Vulnerability details Impact Base rate update frequency can be bypassed by an admin. Above function require updateFrequency blocks to be passed. Proof of Concept function updateBaseRateuint newBaseRatePerYear public // check the current block number uint blockNumber = block.number;...

Duplicate token can be added to MasterChef

Lines of code Vulnerability details Impact MasterChefV2.add does not check if the lpToken is previously added. This would lead to incorrect accounting of lpToken in different pid. Proof of Concept function adduint256 allocPoint, IERC20 lpToken, IRewarder rewarder public onlyOwner uint256...

newProposal.executed automatically set to true

Lines of code Vulnerability details Impact Proposals might be falsely deemed as executed when in fact they have not been executed yet. Proof of Concept The queue function sets newProposal.executed = true even though that is not technically true. For the proposal to be executed it needs to be call...

WETH.allowance() returns wrong result.

Lines of code Vulnerability details Impact WETH.allowance returns wrong result. I can't find other contracts that use this function but WETH.sol is a base contract and it should be fixed properly. Proof of Concept In this function, the "return" keyword is missing and it will always output 0 in th...

Overprivileged admin can grant unlimited WETH

Lines of code Vulnerability details Impact Admin can grantComp to any address using any amount and drain the contract. Proof of Concept If admin key gets compromised there is no timelock, no amount boundaries and no address limitations to prevent the assets to be drained immediately to the...

Anyone can execute a proposal

Lines of code Vulnerability details Impact Anyone can call execute and pass in a malicious proposal. Proof of Concept There is no access control for the execute function. Tools Used Manual Review. Recommended Mitigation Steps Implement access control to execute. --- The text was updated...

Unable to check state() if proposalId == 0

Lines of code Vulnerability details Impact state function cannot be called to view proposal state if proposalId == 0. Proof of Concept There is no check to prevent queueing a proposalId with a value of 0 via the queue function. However, in the state function there is a check preventing using a...

BaseRate can be update by anyone

Lines of code Vulnerability details Impact There is no access modifier in updateBaseRate due to which, anyone can change Baserate to a very low value an borrow the large value function updateBaseRateuint newBaseRatePerYear public Proof of Concept Tools Used manual review Recommended Mitigation...

Only the state() of the latest proposal can be checked

Lines of code Vulnerability details Impact state function cannot view the state from any proposal except for the latest one. Proof of Concept requireproposalCount = proposalId && proposalId initialProposalId, "GovernorBravo::state: invalid proposal id"; Currently proposalCount needs to be bigger ...

Redeem function can silently fail

Lines of code Vulnerability details Impact During the code review, It has been observed that return value of redeem function is not checked. Redeem operation can silently fail and the protocol can expect It is successfully executed. From compound, the comment can be seen from below. CErc20 / CEth...

zeroswap/UniswapV2Pair.sol Token reserves per lp token can be manipulated due to lack of MINIMUM_LIQUIDITY when minting the first liquidity with migrator

Lines of code Vulnerability details if totalSupply == 0 address migrator = IUniswapV2Factoryfactory.migrator; if msg.sender == migrator liquidity = IMigratormigrator.desiredLiquidity; requireliquidity 0 && liquidity != uint256-1, "Bad desired liquidity"; else requiremigrator == address0, "Must no...

getCashPrior == 0 not checked at the end of redeemFresh

Lines of code CNote.solL332 Vulnerability details Impact To ensure that there are no Note tokens in CNote.sol the getCashPrior value should be exactly 0 at the end of every function that transfers tokens in and out of the contract. In the redeemFresh function, the getCashPior value is not checked...

Queued proposals cannot be executed in GovernorBravoDelegate.sol

Lines of code GovernorBravoDelegate.solL63 Vulnerability details Impact In GovernorBravoDelegate.sol's queue function, the executed value for the proposal to be queued is set to true. The execute function in GovernorBravoDelegate.sol will revert when called since the state of the proposal is...

No checks for the input of borrow cap

Lines of code Vulnerability details Impact due to lack of input validation of borrowcap , it can be borrowcap for a ctoken can be 0 and value of 0 corresponds to unlimited borrowing Proof of Concept Tools Used manual reveiw Recommended Mitigation Steps add input validation while using it in loop...

Oracle may be attacked if an attacker can pump the tokens for the entire block

Lines of code Vulnerability details Impact Attacker may use huge amount of their fund to pump the token in a liquidity pair for one entire block. The oracle will capture the manipulated price as current TWAP implementation may only cover 1 block if timed correctly. First block on every periodSize...

Unnecessary migrate function

Lines of code Vulnerability details Impact There is no need to have a migrate function in zeroswap as there are no liquidity to vampire attack from. This function introduce a significant rug vector. Proof of Concept function migrateuint256 pid public requireaddressmigrator != address0,...

Attacker can frontrun calling _mint_to_Accountant() which mints the entire supply to the caller, and sets them as _accountant and admin

Lines of code Vulnerability details Impact By leaving minttoAccountant with no access control when accountant = address0 it allows an attacker to call the function, mint the entire supply to themselves, and gain the accountant and admin roles. Additionally, the parameter "address...

first user can steal everyone else's tokens

Lines of code Vulnerability details Impact Detailed description of the impact of this finding. A user who joins the systems first stakes first can steal everybody's tokens by sending tokens to the system externally. This attack is possible because you enable staking a small amount of tokens. Proo...

Upgraded Q -> M from 44 [1655746452853]

Judge has assessed an item in Issue 44 as Medium risk. The relevant finding follows: 3. Missing/Invalid validation checks Risk Low Impact Function ForgottenRunesWarriorsMinter.setPhaseTimes setups times for different phases of minting. Phases should be launched one after the other. Missing check:...

Upgraded Q -> M from 225 [1655746320073]

Judge has assessed an item in Issue 225 as Medium risk. The relevant finding follows: C4-005 : Missing sanity check on the timestamps Impact During the code review, It has been observed that all timestamps are missing sanity checks. With the following scenario, that can have serious consequences...

Upgraded Q -> M from 225 [1655746069175]

Judge has assessed an item in Issue 225 as Medium risk. The relevant finding follows: C4-010 : The Dutch Auction Parameters Can be Manipulated By Owner After The Auction Started - LOW Impact - LOW Dutch Auction parameters can be changed by a malicious owner, after It is started. The malicious own...

AssetLogic's _swapAssetOut can fail on the one step approval

Lines of code Vulnerability details swapAssetOut performs one step approval for an arbitrary assetIn before calling pool's swapExactOut. As ERC20 that do not allow approval race condition prohibit setting approval to a new positive value when allowance is positive already, this call will fail if...

Malicious Relayer Could Cause A Router To Provide More Liquidity Than It Should

Lines of code Vulnerability details Proof-of-Concept Assume this is a fast-transfer path and the sequencer has a good reason e.g. some sophisticated liquidity load balancing algorithm to assign 3 routers to provide liquidity for a transfer of 90 DAI Therefore, each of them will provide 30 DAI...

Incorrect check in diamondCut allows update be executed without proposal

Lines of code Vulnerability details The current implementation of LibDiamond.diamondCut allows any diamondCut update to be executed instantly, defeating the purpose of the 7 day update delay mechanism. The issue is this check in LibDiamond.diamondCut: require...

ETH funds accidentally sent to InfinityStaker cannot be retrieved

Lines of code Vulnerability details rescueETH function that aims to retrieve mistakenly sent funds cannot reach contract balance, only sending over the Ether value attached to the current call instead. Setting the severity to medium as the case is a violation of system's auxiliary logic. Also, an...

ETH rescue does not work

Lines of code Vulnerability details Impact Both contracts InfinityExchange and InfinityStaker have a function rescueETH to allow an admin to rescue any ETH accidentally sent to the contracts. However, this ETH rescue functionality does not work. The code expects ETH to be sent to this function an...

Upgraded Q -> M from 36 [1655654413719]

Judge has assessed an item in Issue 36 as Medium risk. The relevant finding follows: Allows owner to mint more tokens than planned There are two ways for the owner of the contract to mint tokens for themselves unrelated to the public sale: through the teamSummon function or by setting the minter ...

function removeRouter() in RouterFacet don't check that router has balance and don't transfer it, it just set router owner and recipient to 0x0 which can cause make router balance in danger or unavailable for router owner

Lines of code Vulnerability details Impact There are some security levels for router, like setting owner and recipient and when removeRouter is called this values set to 0x0 and router address become vulnerable. contract should transfer router balance to recipient before removing it. Proof of...

Rescue ETH at InfinityStaker.sol is incorrectly implemented

Lines of code Vulnerability details Impact Admins are unable to recover any ETH locked in the contract PoC The function rescueETHInfinityStaker.sol is incorrectly implemented. It is using msg.value instead of the balance of the contract /// @dev Admin function to rescue any ETH accidentally sent ...

InfinityExchange owner can grief with high fees

Lines of code Vulnerability details Impact The owner of the InfinityExchange contract can set any high fee values for PROTOCOLFEEBPS via the InfinityExchange.setProtocolFee function. The fee value should be limited to a maximum of 10000. If set to higher values and due to the usage across functio...

Centralization Risk with onlyOwner modifier

Lines of code Vulnerability details Impact During the code review, It has been observed the all currency tokens can be withdraw by owner without timelock. The currency token should not be withdrawn by owner. This poses centralization risk. Proof of Concept 1. Navigate to the following contract...

_transferNFTs() functions doesn't reverts if the transfer item doesn't supports interface for both ERC721 and ERC1155

Lines of code Vulnerability details Impact The transferNFTs function use ERC165 to check if the itemnft supports ERC721 interface or ERC1155 interface and execute transfer accordingly. But if it doesn't supports either, it just exits the functionno revert. Proof of Concept in...

Current implementation of arbitrary call execute failure handler may break some use case for example NFT bridge.

Lines of code Vulnerability details Impact Current implementation of arbitrary call execute failure handler may break some use case for example NFT Bridge. In the case of NFT Bridge, NFT may be lost forever. This is likely to be happened in the case of out of gas. Proof of Concept Relayer receive...

Owner can grief with high gas units

Lines of code Vulnerability details Impact The owner of the InfinityExchange contract can frontrun sellers/buyers e.g InfinityExchange.matchOneToOneOrders, InfinityExchange.matchOneToManyOrders functions and set arbitrarily high gas units with updateWethTranferGas which can either cause an...

The timestamp of the specified duration is reset after the position is added via stake().

Lines of code Vulnerability details Impact Detailed description of the impact of this finding. I noticed that the stake amount change in stake is +=, which means that the project itself defaults to adding positions at the same duration, but the timestamp of the duration is overwritten by =...

Router Owner Could Steal All The Funds Within SponsorVault

Lines of code Vulnerability details Proof-of-Concept Assume the following: For simplity sake, only two 2 routers exist within Connext. Gas, relayer, callback fees and slippage are ignored. An attacker owns Router A. Router A has 1,000,000 oUSDC on Optimism Domain/Chain Router B has only 100 oUSDC...

Upgraded Q -> M from 225 [1655654402923]

Judge has assessed an item in Issue 225 as Medium risk. The relevant finding follows: C4-011 : Centralization Risk On The teamSummon Function - LOW Impact - LOW With the teamSummon function, owner can mint unlimited warriors. This poses a security risk. The max/min limit should be implemented at...

Relayer Will Not Receive Any Fee If execute Reverts

Lines of code Vulnerability details Proof-of-Concept Connext relies on the relayer to trigger the BridgeFacet.execute function on the destination domain to initiate the token transfer and calldata execution processes. Relayers pay for the gas cost to trigger the execute function, and in return fo...

Incorrect calculation of startGasPerOrder will cause fund loss to the buyer of the last order due to mischarged gas cost

Lines of code Vulnerability details uint256 gasCost = startGasPerOrder - gasleft + wethTransferGasUnits tx.gasprice; When the orders are matched by the MATCHEXECUTOR, the gas cost of each order is paid by the buyer in WETH, the amount gasCost is calculated based on startGasPerOrder, gasleft and...

Maker order buyer is forced to reimburse the gas cost at any tx.gasprice

Lines of code Vulnerability details uint256 gasCost = startGasPerOrder - gasleft + wethTransferGasUnits tx.gasprice; // if the execution currency is weth, we can send the protocol fee and gas cost in one transfer to save gas // else we need to send the protocol fee separately in the execution...

Buyer may not agree on gasCost

Lines of code Vulnerability details Impact Gas cost when matching orders is payed by the buyer. Since buyers don't have control on order execution, they may spend more gas than what they are willing to. Examples: periods of high gasPrice, or if NFTs for some reason consume a extra amount of gas...

Wrong gasCost calculation per order

Lines of code Vulnerability details Impact During a match, there's some accounting on how much gas we're spending, so that the executor can be reimbursed. The gas cost is split between multiple orders, computing the difference between the gas at the start and at the end. The gas at the start is...

Protocol can steal WETH founds

Lines of code Vulnerability details Impact The protocol can steal WETH founds with the refunds gas cost mechanism in the functions matchOneToOneOrders, matchOneToManyOrders and matchOrders This functions can call only by the MATCHEXECUTOR but we don't know what is this contract/address according...

Admin can set arbitrarily high WETH_TRANSFER_GAS_UNITS which is a fee

Lines of code Vulnerability details Impact Users can lose their fund by attack from the admins Poc In the function updateWethTranferGasInfinityExchange.sol there is no limit for setting WETHTRANSFERGASUNITS . As this is a kind of fee paid by from the buyers to the contract By definition of the...

InfinityExchange owner can steal user's tokens via front-running

Lines of code Vulnerability details Impact Contract InfinityExchange.sol charges protocol fee through PROTOCOLFEEBPS. The issue is that owner of the contract is able to change protocol fee at any time without any restriction which puts him in a very privileged position and allows him to steal...

getCurrentPrice will be return a higher value than expected due to insufficient check in isTimeValid

Lines of code Vulnerability details The Boolean value will always return true if start price is zero. The condition will be sufficiently fulfilled since a time will be set in the future but no time is set in the past or at current time: sumCurrentPrices uses the returned value of getCurrentPrice...

division rounding error in _handleExecuteLiquidity() and _reconcile() make routerBalances and contract fund balance to get out of sync and cause fund lose

Lines of code Vulnerability details Impact variable routerBalances suppose to keep track of routers balance in contract and routers can withdraw their balance from contract. but because of division rounding error in handleExecuteLiquidity and reconcile contract uses more of its tokens than it...

Wrong implementation of withdrawAdminFees() can cause the adminFees to be charged multiple times and therefore cause users' fund loss

Lines of code Vulnerability details function withdrawAdminFeesSwap storage self, address to internal IERC20 memory pooledTokens = self.pooledTokens; for uint256 i = 0; i pooledTokens.length; i++ IERC20 token = pooledTokensi; uint256 balance = self.adminFeesi; if balance != 0 token.safeTransferto,...

rescueEth does not transfer ether in contract

Lines of code Vulnerability details Impact The InfinityStakerrescueETH and InfinityExchangerescueETH payable functions sends msg.value to the destination and not the ether in the contract, so the fees and accidentally transferred ether is not sent to the destination address Proof of Concept...