Use of arbitrary ERC20 tokens could result misaccounting of funds

Lines of code Vulnerability details Vulnerability details If the token is set to a deflationary/fee-on-transfer token then its actual transfer amounts will not be accurately reflected in the protocol accounting given the lack of pre-transfer and post-transfer checks on asset transfers. Impact Som...

delegated value can go to negative value during subtracting with amount. This will cause issue when delegating amount again.

Lines of code Vulnerability details Impact re-delegating / delegating again could not get actual delegated value. Proof of Concept Tools Used Manual code review. VS code Recommended Mitigation Steps update delegated value to zero when it goes to negative value ifdelegated 0 delegated = 0; --- The...

Blocking Through Change of Blocklist Could Trap Tokens

Lines of code Vulnerability details Impact In the normal flow, an account that is blocked is protected from having its funds locked by a call to forceUndelegate, as occurs on line 27 of Blocklist.sol. However, this protection could potentially be circumvented if the value of blocklist is changed ...

VotingEscrow: uninitialized blocklist will DOS the contract

Lines of code Vulnerability details Impact The checkBlocklist modifier calls the isBlocked function for the blocklist address, but the blocklist is not initialized in the constructor, only assigned by the owner in the updateBlocklist function. This means that after the contract is deployed and...

createNFTDropCollectionWithPaymentAddress() doesn't granting the creator to receive royalties and mint payments

Lines of code Vulnerability details Impact createNFTDropCollectionWithPaymentAddress doesn't ensure to receive royalties and mint payments to the address payable paymentAddress Proof of Concept The documentation side Create a new drop collection contract with a custom payment address and in...

Functions quitLock and delegate fundamentally change game theory of VoteEscrow

Lines of code Vulnerability details Impact Without delegation it is not possible to remove voting power before the end of a lock. Function quitLock now makes this possible, but it does not just affect the user who quits the lock. Any votes that are delegated to them are temporarily lost from the...

VotingEscrow Owner Can Give Power to Force Undelegate

Lines of code Vulnerability details Impact As the owner can change the blocklist to anything, a malicious or compromised owner has the ability to set blocklist to an EOA which could directly call forceUndelegate on any number of users. This could be used in bad faith to distort voting. Recommende...

VotingEscrow contract doesn't work for tokens that have decimals more than 18

Lines of code Vulnerability details Impact In the VotingEscrow.sol constructor, the decimals are calculated based on the specified ERC20 tokens decimals. Then on the L:116 a require statement enforces that the max decimals of the ERC20 token is 18. This is not always the case, as some ERC20 token...

Delegators can Avoid Lock Commitments if they can Reliably get Themselves Blocked when Needed

Lines of code Vulnerability details Impact Users can enjoy the voting power of long lock times whilst not committing their tokens. This could cause the entire system to break down as the incentives don't work any more. Exploit Method This exploit only works if a user is able to use the system and...

the 1st creator can get a little amount or 0 creatorShares

Lines of code Vulnerability details Impact On line 513 the MarkerFees.getFees function if there is more than one creatorRecipients it will loop through them and set there payment after that it will subtract creatorRev - totalRoyaltiesDistributed and set it to the the 1st creatorRecipients...

ERROR IN UPDATING **_checkpoint** IN THE **increaseUnlockTime** FUNCTION

Lines of code Vulnerability details Impact The potentiel impact of this error are : Give wrong voting power to a user at a given block. Give wrong total voting power at a given block. Give wrong total voting power. Proof of Concept The error occured in this line : In the increaseUnlockTime functi...

Checkpoint end time is not correct.

Lines of code Vulnerability details Checkpoint end time is not correct. Impact The user's voting power can be significantly decreased. Proof of Concept At L513 of VotingEscrow.sol, when we increase unlock time, oldLocked.end should be oldUnlockTime instead of unlocktime. We can double-check it fr...

Exposure of critical functions

Lines of code Vulnerability details Impact AdminRole mixin exposes critical functions without any restrictions like grantAdmin revokeAdmin Proof of Concept Criticial functions like grantAdmin can be externally accessed changing the critical roles like admin. // for eg: function grantAdminaddress...

VotingEscrow Ownership Management Lacks Basic Protections

Lines of code Vulnerability details Impact A comment saying "Owner should always be a timelock contract" is not sufficient protection for something as critical as ownership. There should be: a zero address check on addr a propose/transfer method of ownership transfer, not a unilateral transfer...

A delegatee can frontrun the delegator's call to increaseUnlockTime to prevent the delegator to withdraw or quitlock

Lines of code Vulnerability details Impact Charlie and Alice both create a lock, with Alice's lock being longer than Charlie's. Charlie then delegates to Alice. At this point, if Charlie wants to unlock his tokens he can call withdraw or quitLock, but not with a delegation in place see 1, 2, so h...

Attackers can abuse the quitLock function to get a very large amount of votes

Lines of code Vulnerability details Impact An attacker can use a flashloan and the quitLock function to achieve a large amount of votes for one transaction. It can, depends on the implementation of the modules that will use this contract, be used to pass malicious proposals or exploit any feature...

Single-step ownership change for contracts is risky

Lines of code Vulnerability details Vulnerability details The owner addresses can be changed in a single-step in VotingEscrow. If set to the wrong address this could lead to loss/lock of funds or allow a malicious smart contract to interact with the protocol. When privileged roles are being...

createLock: User can lose funds by sending a wrong _value parameter

Lines of code Vulnerability details Impact Users can lose funds by sending a wrong value parameter. The problems relays in casting value to int128. Solidity does not check casting, just math operations. If a user tries to lock an amount greater than 2^128, the transaction won't be reverted and...

Add limit on NFT Mining Count

Lines of code Vulnerability details Add limit on NFT Mining Count Context: Description: Mint Function is the most important architectural part of an NFT project. The mint function has no quantity limit. For example, in a 10000 NFT project, 10000 NFT Mints can be triggered at the same time. Howeve...

Wrong values are passed to _checkpoint mess up accounting

Lines of code Vulnerability details Impact The increaseUnlockTime function sends a wrong unlock time to the checkpoint function in the oldLocked variable - the locked variable which is the new LockedBalance is copied into the oldLocked variable which is supposed to be the previous LockedBalance,...

Wrong logic in increaseUnlockTime() function in case undelegated lock and call _checkpoint()

Lines of code Vulnerability details Impact In increaseUnlockTime function, in case it’s undelegated lock, it calls checkpoint for msg.sender with oldLocked and locked. But actually, these 2 locks oldLocked and locked are the same. It makes the logic in checkpoint function works incorrectly. Proof...

Bad access control in AdminRole.sol can lead to all funds being stolen from FoundationTreasury.sol

Lines of code Vulnerability details Proof of concept FoundationTreasury.sol inherits CollateralManagement.sol which has the method function withdrawFundsaddress payable to, uint256 amount external onlyAdmin that can withdraw the whole balance of the treasury to the to address. It looks like it is...

Delegate can force users, who delegate to them, to increase their lock end

Lines of code Vulnerability details Impact If a user has votes delegated to them, they can force all users who delegated to them to increase their lock time with minimal negative effects to themselves. In my PoC, I prove that a user can lock minimal value and manipulate the users who delegated to...

Creator can be incorrectly paid because of array truncate when distributing mint fee.

Lines of code Vulnerability details Impact Detailed description of the impact of this finding. The creatorRecipient and creatorShares can be forcefully trancated to a fixed length. And the truncated creator is not paid. Proof of Concept Provide direct links to all referenced code in GitHub. Add...

mintFromFixedPriceSale for a custom contract can lead to users losing funds

Lines of code Vulnerability details mintFromFixedPriceSale for a custom contract can lead to users losing funds NFTDropMarketFixedPriceSale.createFixedPriceSale allows creators to create a sale drop. A creator can create a drop sale for their custom NFT Contract that adheres to...

The current implementation of the VotingEscrow contract doesn't support fee on transfer tokens

Lines of code Vulnerability details Impact Some ERC20 tokens implemented so a fee is taken when transferring them, for example STA and PAXG. The current implementation of the VotingEscrow contract will mess up the accounting of the locked amounts if token will be a token like that, what will lead...

VotingEscrow: Unsupported fee-on-transfer tokens

Lines of code Vulnerability details Impact In the VotingEscrow contract, if the token is a fee-on-transfer token, the number of tokens received by the contract in the createLock and increaseAmount functions will be less than value. This makes the locked balance recorded by the contract incorrect,...

Bad access control in AdminRole.sol can lead to griefing DoS by front-running when trying to withdraw treasury funds

Lines of code Vulnerability details Proof of concept FoundationTreasury.sol inherits CollateralManagement.sol which has the method function withdrawFundsaddress payable to, uint256 amount external onlyAdmin that can withdraw the whole balance of the treasury to the to address. It can be called on...

The _checkpoint function won't be called for a user which is both a delegator and a delegatee in the increaseUnlockTime function

Lines of code Vulnerability details Impact The virtual balance of a user is calculated using 2 values - the amount that is delegated to that user, and his lock period. When calling the increaseUnlockTime function, we want to checkpoint the user's data as long as he doesn't have any funds. This is...

A malicious delegatee can always block the delegator from undelegating the lock

Lines of code Vulnerability details Impact A user who has delegated his/hers voting power to a delegatee can break his/hers delegate only by submitting a lock with a higher expiration time than the delegatee after a successful call to increaseUnlockTime function. After that, he has to call the...

User cannot withdraw locked fund at all after unlock time has passed if delegated to someone else. Result in huge economics loss.

Lines of code Vulnerability details Impact User cannot withdraw locked fund at all after unlock time has passed if delegated to someone else. Result in huge economics loss as user can't get their underlying token delegated to back. In the document it is said that locks need to be undelegated firs...

Inconsistent logic of increase unlock time to the expired locks

Lines of code Vulnerability details 2022-08-fiatdao Inconsistent logic of increase unlock time to the expired locks Impact Can not prevent expired locks being extended. Proof of Concept Call function function increaseUnlockTime with an expired lock lockedmsg.sender.end block.timestamp Case 1: if...

users could mint NFTs for free

Lines of code Vulnerability details Impact Malicious users could mint NFTs AMAP by the collection saleConfig.limitPerAccount with msg.value == 0 ETH Proof of Concept The malicious users call mintFromFixedPriceSale with 0 ETH. the only check is if msg.value mintCostand there is no check for...

Missing onlyCreator modifier on minting functions

Lines of code Vulnerability details Impact Missing onlyCreator modifier or any access-control to the NFTCollection.sol mint function makes all of the contract's minting functions absolutely open to any user. Proof of Concept All minting functions in the NFTCollection contract: mintAndApprove...

_safeMint() should be used rather than _mint() wherever possible

Lines of code Vulnerability details safeMint should be used rather than mint wherever possible Impact In NFTCollections.sol and NFTDropCollection, eventually it is called ERC721 mint. Calling mint this way does not ensure that the receiver of the NFT is able to accept them, making possible to los...

increaseUnlockTime missing _checkpoint for delegated values

Lines of code Vulnerability details PNM-001 increaseUnlockTime missing checkpoint for delegated values. Links Description In the VotingEscrow contract, users can increase their voting power by: Adding more funds to their delegated valule Increasing the time of their lock Being delegated by anothe...

The toLocked.end >= fromLocked.end in delegate function is inconsistent with design.

Lines of code Vulnerability details Impact The condition of toLocked.end = fromLocked.end in function delegate is inconsistent with design in veFDT Checkpoint Math Proof of Concept In VotingEscrow.sol. L589 requiretoLocked.end = fromLocked.end, "Only delegate to longer lock"; But In veFDT...

Unsafe usage of ERC20 transfer and transferFrom

Lines of code Vulnerability details Impact Some ERC20 tokens functions don't return a boolean, for example USDT, BNB, OMG. So the VotingEscrow contract simply won't work with tokens like that as the token. Proof of Concept The USDT's transfer and transferFrom functions doesn't return a bool, so t...

FETH.sol is susceptible to ERC20 approve functionality front-running exploit

Lines of code Vulnerability details Proof of concept The problem is perfectly described here The tldr; is that if you approved a person to spend 100 tokens and then you want to decrease his allowance to 50, if he spends his 100 tokens allowance before you set his allowance to 50 he will be able t...

Possible lost msg.value

Lines of code Vulnerability details Impact Possible lost value in tryUseFETHBalance function call Proof of Concept In call tryUseFETHBalance can send value and the asset can be an ERC20!= address0. if send as asset ETH== address0 and send more value than totalAmounttotalAmount msg.value, the...

[M1] Incorrect amount of gas sent in _distributeFunds

Lines of code Vulnerability details Impact In case recipients consume more gas than expected the transaction could revert or cost can be too high. Proof of Concept According to the definition of the variable SENDVALUEGASLIMITMULTIPLERECIPIENTS you intend to use all that gas for all recipients. /...

Unsafe typecasting can lead to tokens being locked in the contract

Lines of code Vulnerability details Impact In the createLock function the amount is calculated by casting the uint256 value to int128 in an unsafe way. Specifically the locked.amount is calculated as: locked.amount += int128int256value; could result in a negative value for locked.amount. For...

wrong calculation in MarketFees._getFees

Lines of code Vulnerability details Impact when calculating the royalty the loop start with 1 so creatorShares0 is skipped and not correctly converted his share so he can get more royalty than others creators Proof of Concept Provide direct links to all referenced code in GitHub. Add screenshots,...

Some ERC20 tokens don't return bool values

Lines of code Vulnerability details Impact VotingEscrow will not work for such tokens. Proof of Concept This issue is same as a previous contest. In the implementation of token transfer, the return value is checked after token transfer. But some tokens do not return bool values. After DM, I know...

[H1] Protocol is unusable with some ERC20 tokens

Lines of code Vulnerability details Impact ​ Protocol unusable for USDT and other bad tokens Proof of Concept ​ Some ERC20 tokens do not implement correctly EIP-20 standard. They just do not return a boolean in transfer/transferFrom functions and just return void. Asking them for a boolean will...

No check for nftcollection address leads to the user's loss in NFTDropMarket

Lines of code Vulnerability details Impact There is no check for nftContract address in function createFixedPriceSale of NFTDropMarketFixedPriceSale.sol. The seller can deploy a malicious NFT collection contract, everything is very like the collection produced by NFTCollectionFactory except the...

Users should not use quitLock () function when unlock() is set.

Lines of code Vulnerability details Impact In this case, the quitLock function can be abused as it would allow users to exit the locking mechanism at anytime. As such, the entire objective of a locking mechanism would not be realized. Proof of Concept The quitLock function is used to quit locking...

Using StableMath and SafeCast

Lines of code Vulnerability details Impact Detailed description of the impact of this finding. import "@openzeppelin/contracts/utils/math/SafeCast.sol"; Wrappers over Solidity’s uintXX/intXX casting operators with added overflow checks. Downcasting from uint256/int256 in Solidity does not revert ...

Expired locks can still increaseUnlockTime when the locked_.end != msg.sender

Lines of code Vulnerability details Impact Expired locks can still increase their unlock time and the new unlocktime does not factor in the duration the lock was inactive Proof of Concept Users can increase their unlock time via the increaseUnlockTime function. In the increaseUnlockTime function ...

VotingEscrow: Anyone can call the collectPenalty function

Lines of code Vulnerability details Impact In the VotingEscrow contract, anyone can call the collectPenalty function to send penalty tokens to the penaltyRecipient address. However, if the private key of the penaltyRecipient address is compromised, the attacker can immediately call the...