_rejectBuyout() does not take buys and sells into account until next block, can lead to false effects due to timing

Lines of code Vulnerability details Impact Buyout that should be rejected will be allowed to happen. Proof of Concept Last user to call buy that could have pushed rejectBuyout to reject the buyout wont be accounted for because rejecBuyout is place before the minting of new tokens. Therefore even ...

sell() and buy() should not have whenNotPaused modifier

Lines of code Vulnerability details Impact Impacts are two fold: 1. admin should not be allowed to stop users from selling/cashing out their assets under any circumstance. 2. Due to poor timing or malicious admin activity, it could be the case that buy is put on pause right after initiateBuyout i...

Reentrancy bug in Basket's withdraw multiple tokens function which gives attacker ability to transfer basket ownership and spend it but withdraw all the tokens out of basket

Lines of code Vulnerability details Impact Basket is used for keep multiple tokens in contract and mint one NFT token to represent their ownership. Basket only allows for owner of NFTid=0 to withdraw tokens from Basket address. users can deposit multiple tokens in one Basket and then create a...

Buys/sells will be front-run by MEV bots

Lines of code Vulnerability details Impact Since the value of the vault depends on the Bancor formula, it's important that transactions can't be front-run, because doing so, even for small amounts, will change the valuation price for subsequent buys/sells. The link the sponsor provided describes...

Issue with TWAV calculation.

Lines of code Vulnerability details Impact To calculate time weighted average value, current valuation is used. This is not appropriate way to decide the TWAV while other locations considers only time. This might not be the stable one. Proof of Concept uint256 currentValuation =...

Potential denial of service issues

Lines of code Vulnerability details Impact Detailed description of the impact of this finding. I noticed that the withdrawUnsettledBids and redeem functions return the Ether amount by calling safeTransferETH, but if the to address passed in is a malicious contract address and the receive function...

sell function does not have minimum threshold on the _minAmtOut

Lines of code Vulnerability details Impact During the code review, It has been observed saleReturn value is not resistant to slippage on the minAmtOut. For the protecting value from slippage, the protocol should ensure that minAmtOut is different than zero. Proof of Concept Tools Used Recommended...

Missing notBoughtOut modifier

Lines of code Vulnerability details Impact On the withdrawUnsettledBids function, unsettledBid can be withdraw when buyout is rejected. However, the contract is missing notBoughtOut modifier in the function. From that reason, during buyout period the function can be called. On the redeem function...

NibblVault buyout duration longer than update timelock

Lines of code Vulnerability details Impact User can buy out NFT by initiating the process through initiateBuyout, then he has to wait BUYOUTDURATION which is 5 days and if the buyout will not get rejected he can claim the NFT. During that period bidder cannot cancel the process. The issue is that...

[PNM-004] Calculation of _secondaryReserveRatio can be overflowed

Lines of code Vulnerability details Description uint32 secondaryReserveRatio = uint32msg.value SCALE 1e18 / initialTokenSupply initialTokenPrice; secondaryReserveRatio can be overflowed by setting a relatively small initialTokenSupply and initialTokenPrice. The result will be truncated by uint32,...

DoS with block gas limit--External calls inside a loop might lead to a denial-of-service attack.

Lines of code Vulnerability details Impact --Check: calls-loop --Severity: Medium --Confidence: Medium External calls can fail accidentally or deliberately, which can cause a DoS condition in the contract. Proof of Concept -- --ConsenSys Smart Contract Best Practices -- -- Tools Used...

TWAV can be attacked by flash loan

Lines of code Vulnerability details Impact updateTWAV can be flash loaned. Hacker may pay the flash loan fee for 4 blocks then execute the attack after that. Proof of Concept function updateTWAVuint256 valuation, uint32 blockTimestamp internal uint32 timeElapsed; unchecked timeElapsed =...

Upgraded Q -> H from 94 [1655957384739]

Judge has assessed an item in Issue 94 as High risk. The relevant finding follows: 2. Return values of transfer/transferFrom not checked Not all IERC20 implementations revert when there's a failure in transfer/transferFrom. The function signature has a boolean return value and they indicate error...

Upgraded Q -> H from 285 [1655952312863]

Judge has assessed an item in Issue 285 as High risk. The relevant finding follows: 1. Excess ether sent to FixedPricePassThruGate is lost low passThruGate redirects to a beneficiary only gate.ethCost, requiring that msg.value = gate.ethCost. As there are no other ways to access native tokens hel...

missed valid address check for assetAddress

Lines of code Vulnerability details Impact Malicious Actor can do DoS using Reentrance using createVault and can create vault without depositing any asset Proof of Concept pragma solidity ^0.8.10; import "./NibblVaultFactory.sol"; contract ERC721Attack address curator =...

use ECDSA helper library instead of ecrecover

Lines of code Vulnerability details use ECDSA helper library instead of ecrecover the function permit adds approval for a spender for a certain amount of value use of ecrecover to validate signatures without a nonce will allow signatures to be replayed to add additional approvals due to signature...

Transition notBoughtOut -> boughtOut -> notBoughtOut possible because of updateTWAV

Lines of code Vulnerability details Impact Because rejectBuyout uses the TWAV, which is time-weighted and updateTWAV can be called, it is possible that notBoughtOut is true at first, then boughtOut is true, and then notBoughtOut is true again. See Proof of Concept for how one can construct such a...

initiateBuyout and sell functions on NibbleVault.sol Reentrancy vulnerability

Lines of code Vulnerability details Impact 1. In NibbleVault.sol, it is possible for a contract to prevent the Sell event from being fired after being sent ether. From calling the sell function on line 390, an attacker contract is able to keep the event hidden from being listened to by stopping t...

ERC721 support not checked - NFT loss

Lines of code Vulnerability details Impact It is not checked whether withdrawn token supports ERC721 interface. Also there is no check to see if necessary approval have been set for sending nft The fix need to be applied for all other functions withdrawing ERC721 Proof of Concept 1. Observe the...

ERC1155 support unchecked - NFT can be lost

Lines of code Vulnerability details Impact It is not checked whether withdrawn token supports ERC1155 interface. Also there is no check to see if necessary approval have been set for sending nft The fix need to be applied for all other functions withdrawing ERC1155 Proof of Concept 1. Observe the...

Some users can redeem more reserve tokens after curator redeems accumulated curator fee.

Lines of code Vulnerability details Impact It is possible for some users to redeem more reserved tokens if the curator redeems accummulated curator fee before their redeem action in the case of a boughtOut. This is possible because NibblVault.redeemCuratorFee sets feeAccruedCurator back to 0. So ...

whenNotPaused modifier missing

Lines of code Vulnerability details Impact whenNotPaused modifier is missing in both createBasket function NibblVaultFactory.solL80 and withdrawUnsettledBids function NibblVault.solL424 This means even when contract is in paused state this function will still be operational Note Other impacted...

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

Attackers can call update and grief the users from swapping tokens

Lines of code Vulnerability details Impact Attackers can call update and grief the users from swapping tokens An attacker can specify reserve0 and revsere1 in update function and make it very small number and the user will have to supply zero tokens otherwise the function will fail. mitigation :...

Attacker can steal all tokens from pools

Lines of code Vulnerability details Attacker can steal all tokens from pool 2 issues 1.On deployment totalsupply is zero and reserve0 and reserve1 is zero called minting 103 to address zero And if tokens have supply in them amount0=1018 amount1=1018 liquidity= sqr1e36 - 103 =1015 which is still...

A queued proposal will never be executed

Lines of code Vulnerability details All the relevant fields for a proposal is set in queue but executed is marked as true, even before the proposal is even queued. When it comes time to execute a proposal, this will be impossible since state will always return ProposalState.Executed. The conditio...

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

WETH#approve can be used to steal user balances

Lines of code Vulnerability details Impact In WETH.sol 2nd approve function can be used to update allowance of any user, it can be used to steal users balance Proof of Concept function approveaddress owner, address spender external returnsbool approveowner, spender, balanceOfowner; return true;...

Functions in CNote.sol are internal instead of external/public

Lines of code Vulnerability details Impact Most of the functions in CNote.sol are internal mutability and have no other locations they are being called. This renders most of the functionalities in the contract to be uncallable. Tools Used Manual review Recommended Mitigation Steps Change to the...

No check of return value token transfer

Lines of code Vulnerability details Impact transfer does not check the return , due which transfer may get failed without reverting Proof of Concept Tools Used manual review Recommended Mitigation Steps add checks the for the return value from transfer --- The text was updated successfully, but...

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

MasterChef's emergencyWithdraw can be reentered drawing all lptokens from the contract if pool.lpToken allows for transfer flow control

Lines of code Vulnerability details User's LP funds accounting update in emergencyWithdraw happens after lpToken transfer. If pool.lpToken allows for the control of transfer call flow or can be upgraded to allow it in the future i.e. beforetokentransfer, afterTokenTransfer type of hooks, or an...

newblockchain report: Missing access control in Proposal-Store.sol

Lines of code Vulnerability details Impact Anyone can add proposal. These proposals are used later to be executed by timelock in GovernorBravoDelegate. However, it is unclear whether the code works as intended: see the QA report "Upon queue executed true in GovernorBravoDelegate". Possible...

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

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

Missing zero address check can set treasury to zero address

Lines of code Vulnerability details Impact AccountantDelegate.initialize is missing a zero address check for treasury parameter, which could may allow treasury to be mistakenly set to 0 address. Proof of Concept Tools Used Manual review Recommended Mitigation Steps Add a require check for zero...

_update function on deployment time isnt actually going to be 30 minutes and you can bypass it

Lines of code Vulnerability details Because timeElapsed = blocktimestamp , on deployment block.timestamp -point.timestamp it can be zero if some calls it right after deployment And block.timestamp 1800 which is true even though its not a pass 30 minutes mitigation have check that updated...

getWETHAddress() returns wrong address

Lines of code Vulnerability details Impact The hardcoded return value is the address of COMP token in mainnet not of WETH in getWETHAddress. grantCompInternal function depends on this return value which will cause unintended issues. Proof of Concept function grantCompInternaladdress user, uint...

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

Note: When _initialSupply ! = 0, the _mint_to_Accountant function will fail

Lines of code Vulnerability details Impact In Note contract, if initialSupply ! = 0, totalSupply will overflow when the minttoAccountant function executes mintmsg.sender, typeuint.max constructorstring memory name, string memory symbol, uint256 totalSupply public name = name; symbol = symbol;...

Poor random used in NoteInterest

Lines of code Vulnerability details Impact The random number used in NoteInterest.getBorrowRate is easily bruteforceable by an attacker to generate the address that give the lowest interest rate. Proof of Concept uint rand = uintkeccak256abi.encodePackedmsg.sender % 100; uint ir = 100 -...

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

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

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

WETH.totalSupply() returns wrong result.

Lines of code Vulnerability details Impact WETH.totalSupply 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 WETH.balanceOf just returns a balance of a specific address and totalSupply must ...

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

CNote updates the accounts after sending the funds, allowing for reentrancy

Lines of code Vulnerability details Having no reentrancy control and updating the records after external interactions allows for funds draining by reentrancy. Setting the severity to medium as this is conditional to transfer flow control introduction on future upgrades, but the impact is up to th...

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

CNote: Attacker can frontrun the calling of _setAccountantContract and gain admin priviledges and insert a malicious accountant contract

Lines of code Vulnerability details Impact The function does not have access control before the accountant address is set, allowing anyone to call the function, gain admin privileges, and set the accountant address. Proof of Concept CNote.solL17 Recommended Mitigation Steps Include access control...

Attacker can transfer himself tokens and deplete token0 and token 1 out of funds

Lines of code Vulnerability details Impact Attacker can transfer himself tokens and deplete token0 and token 1 out of funds And there is no check for how much tokens attacker has so attacker Supplies amount0out and amount1out huge number not checked and it gets transferred to the attacker and...