MarketFees's treasury can have potentially a malicious admin

Lines of code Vulnerability details Impact / Proof of Concept In contracts/FoundationTreasury.sol, an attacker can frontrun a call to initialize to register as an admin. If the address of this treasury is shared or is already shared with NFTDropMarket's constructor line 83, then on line 87,...

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

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

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

Blocklist contract lack an unblock mechanism

Lines of code Vulnerability details Impact In Blocklist contract, the manager can block a contract from accessing VotingEscrow by calling the block function. If the manager calls the block function on a contract due to some misunderstanding, the manager cannot resume the contract's access to...

Users can create an un-bannable contract

Lines of code Vulnerability details Impact Users can create an un-bannable contract by working from a contract's constructor and then self-destructing on each instantiation. Users can also deterministically deploy the contract to the same address every time they want to interact with fiatDAO...

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

Upgraded Q -> H from 275 [1660349121925]

Judge has assessed an item in Issue 275 as High risk. The relevant finding follows: --- The text was updated successfully, but these errors were encountered: All reactions...

Manager is able to charge an excessive fee by taking out a larger than necessary flash loan

Lines of code Vulnerability details Impact The manager's fee is determined by the flash loan amount he takes out. A manager can take out a large flash loan to increase the fee due to him. Proof of Concept Manager needs to rebalance a vault. Manager only needs 50k flash loan. Manager takes out 2m...

[H3] Persisted msg.value in a loop of delegate calls can be used to drain ETH from your proxy

Lines of code Vulnerability details Impact msg.value in a loop can be used to drain proxy funds PoC While BoringBatchable is out of the scope, this bug affects seriously MIMOProxy as it inherits. Some time ago I read a report about an auditor called samczsung . I believe that you are having the...

Caller is able to set management params in a MIMOManagedRebalance proxy he doesn't own

Lines of code Vulnerability details Impact A caller is able to set management params with any MIMOManagedRebalance proxy as long as the caller is a vault owner, the caller has deployed a proxy, and the caller is set as a manager of said vault. Proof of Concept Using the MIMOProxy's multi-call, ca...

The MIMOVaultActions contract cannot withdraw assets to the user

Lines of code Vulnerability details Impact The MIMOVaultActions contract provides users with the functions of depositing, withdrawing and borrowing assets, which is called by the delegatecall of the MIMOProxy contract. But when borrowing or withdrawing assets, there is no function provided to giv...

rbData.toCollateral address can be different than the token address set in swapData.dexTxData.

Lines of code Vulnerability details Impact rbData.toCollateral address can be different than the token address set in swapData.dexTxData. A manager can supply toCollateral address of the collateral within the contract but supply a different address in swapParams.dexTxData such that the contract...

Lack of storage gap for upgradable contracts

Lines of code Vulnerability details Impact Safe usage of upgradable contract should include a storage gap to allow the addition of new state. OpenZeppelin docs Proof of Concept The following contracts on the project are using upgradable contracts from OpenZeppelin. File: contracts/Community.sol...

Project.raiseDispute() doesn't use approvedHashes - meaning users who use contracts can't raise disputes

Lines of code Vulnerability details Impact In case users are using a contract like a multisig wallet to interact with a project, they can't raise a dispute. The sponsors have added the approveHash function to support users who wish to use contracts as builder/GC/SC. However, the...

Builder can reduce his debt more than expected by replaying signature

Lines of code Vulnerability details Impact A builder can, after it convened with the lender and an external agent to reduce his debt through escrow, reuse the signature and pass it again to escrow many times. This allows him to reduce his debt more than expected, leaving the lender at a loss. Pro...

projectCost may be reverted due to out of gas problem if having too many tasks. lendToProject and recoverTokens may always revert because of this.

Lines of code Vulnerability details Impact projectCost may be reverted due to out of gas problem if having too many tasks. lendToProject and recoverTokens may always revert because of this. If lendToProject always revert, community owner won't be able to lends fund to the published project. Tasks...

Project.changeOrder() might reinitialize already completed task.

Lines of code Vulnerability details Impact changeOrder can be used to change subcontractor. But if a project builder approves the signature by fault or the contractor is delegated, a malicious contractor and subcontractor might reinitialize an already completed task and complete again to receive...

New subcontractor can be set for a SCConfirmed task without current subcontractor consent

Lines of code Vulnerability details Malicious builder/contractor can change the subcontractor for any task even if all the terms was agreed upon and work was started/finished, but the task wasn't set to completed yet, i.e. it's SCConfirmed, getAlertstaskID2 == true. This condition is not checked ...

Automation / management can be set for not yet existing vault

Lines of code Vulnerability details Impact & Proof Of Concept vaultOwner returns zero for a non-existing vaultId. Similarly, proxyRegistry.getCurrentProxymsg.sender returns zero when msg.sender has not deployed a proxy yet. Those two facts can be combined to set automation for a vault ID that doe...

DOS on valid signatures

Lines of code Vulnerability details Impact It was observed that signature matching can fail due to incorrect updation of operatorIndex. Each loop iteration should reset operatorIndex but this is not happening causing matches to be skipped as shown in POC Proof of Concept 1. Assume...

AxelarAuthWeighted - Can set operators to same values to override OLD_KEY_RETENTION

Lines of code Vulnerability details Impact It is possible to transfer operatorship to the same operators by simply doubling the values of the newWeights array and newThreshold value. This could be used by newly appointed operators to invalidate all previous operators and thus invalidate the...

Attacker can empty vaults

Lines of code Vulnerability details Impact Both MIMOEmptyVault and MIMOLeverage contracts share same signature/definition, an attacker can gain control of EmptyVault contract, issue a flash loan, and empty the vaults. Proof of Concept See the test below. In below test, EmptyVault contract is...

Wrong arithmetic operation upon filling a bid order, lead to lost amount of ether on user funds

Lines of code Vulnerability details Functions GolomTrader.solfillBid... and GolomTrader.solfillCriteriaBid... used to fill a signed order Golom order.orderType = 1 or = 2 by the buyer after he approved GolomTrader contract some wETH. However, in case that the signed order is of type ERC1155 where...

The function removeDelegation doesn't work

Lines of code Vulnerability details When a delegation is created with the function delegate, the ID of its NFT tokenId is added in the array checkpointstoTokenIdnCheckpoints - 1; where toTokenId is the ID of NFT the user delegates its NFT to. When an user removes the delegation of his NFT with th...

Fees are lost after GOLOM token supply limit has been reached

Lines of code Vulnerability details Impact When the 1 billion supply has been reached for the GOLOM token, addFee in RewardDistributor will automatically return to the start without updating any trades, however will still accept fees being sent from GolomTrader. These fees however cannot be claim...

Attacker can steal all the wrapped tokens, ether or native currency contained in the ReceiverImplementation contract

Lines of code Vulnerability details Impact since all the functions in the ReceiverImplemention are all designed to be called by DepositBase and AxelarDepositService, why not create a simple modifer to check if msg.sender equals either of the two when a delegatecall or call occurs, like so modifie...

Misleading mapping delegates

Lines of code Vulnerability details The mapping delegates in VoteEscrowDelegation can be misleading it is not updated when a delegation is removed. If someone is relying on this value not the case here, because this mapping is never used in the contracts ?, but we can assume that it has / will ha...

Malicious user can cause complete loss of function in AxelarAuthWeighted.sol by matching validators and weights to a previous epoch

Lines of code Vulnerability details Impact Complete loss of function for both AxelarAuthWeighted.sol and AxelarGateway.sol Proof of Concept if epochForHashnewOperatorsHash 0 revert SameOperators; AxelarAuthWeighted.soltransferOperatorship reverts in L76 if epochForHashnewOperatorHash != 0. This...

[PNM-004] An additional domain can be registered for free

Lines of code Vulnerability details Description The ETHRegistrarController added new functionality to support set multiple records while registering a ETH 2LD. It uses the following code to support this functionality. function setRecords address resolver, bytes32 label, bytes calldata data intern...

call() should be used instead of transfer() on an address payable

Lines of code Vulnerability details Impact The use of the deprecated transfer function for an address will inevitably make the transaction fail when the caller is a smart contract and: 1. Does not implement a payable function. 2. Implements a payable fallback which uses more than 2300 gas unit. 3...

[PNM-001] PARENT_CANNOT_CONTROL can be bypassed by maliciously unwrapping parent node

Lines of code Vulnerability details Description By design, for any subdomain, as long as its PARENTCANNOTCONTROL fuse is burnt and does not expire, its parent should not be able to burn its fuses or change its owner. However, this contraint can be bypassed by a parent node maliciously unwrapping...

Incorrect implementation of RRUtils.serialNumberGte

Lines of code Vulnerability details Impact Comparing serial numbers should follow RFC1982 due to the possibility of numbers wrapping around. RRUtils.serialNumberGte tried to follow the RFC but failed to do so, leading to incorrect results in comparison. Proof of Concept For a serial number i1 to ...

Incorrect validation check results with always initial proportion to the liquidator.

Lines of code Vulnerability details Impact The liquidator will get the initial proportion of value irrespective how long the time passed. Proof of Concept if duration == typeuint32.max in above condition check need to be checked with "elapsed" not with "duration" . Since it was mentioned that...

auction.start might be zero after a new auction is created.

Lines of code Vulnerability details Impact Currently several functions consider the auction is valid when auction.start 0 and they don't work properly when auction.start = 0. Proof of Concept In this part, auction.start = 0 when block.timestamp = 232 exactly. Of course, this is an edge case and...

Incorrect amount of Collateral moves for Auction

Lines of code Vulnerability details Impact It was observed that the debt and collateral which moves for Auction is calculated incorrectly. In case where line.proportion is set to small value, chances are art will become lower than min debt. This causes whole collateral to go for auction, which wa...

Auctions can be started for same undercollateralized vault using different Witch contracts, which causes vault collateral loss

Lines of code Vulnerability details Impact Given that there are two deployed Witch contracts, two auctioneers are calling Witch 1 and Witch 2's auction functions for the same vault at similar times. Both transactions are included in the same block for which the governances of these Witch contract...

Vault blacklisting is required

Lines of code Vulnerability details Impact Add a way to blacklist vault from going in Auction. There may be cases where owner does not want a particular Vault to be Auctioned even if it is undercollateralized as shown in POC. Due to lack of any blacklisting all Vault are treated equally and can g...

Reduce the incentive of self liquidation

Lines of code Vulnerability details Reduce the incentive of self liquidation Impact In some situations the borrower may have incentive to self liquidate. For example: May pay less interest Redeem early and reduce the loss or the fee React to abrupt market volitility Other cases to circumvent some...

User can set auctioneer to address(0) to prevent vault from being liquidated

Lines of code Vulnerability details Impact Vault cannot be liquidated Proof of Concept auction can be called with any address as the 'to' address. A majority of ERC20 tokens will revert if a transfer is initiated to address0, notably, including USDC. Since the auctioneer is paid each time a payme...

Upgraded Q -> M from 233 [1657954928095]

Judge has assessed an item in Issue 233 as Medium risk. The relevant finding follows: --- The text was updated successfully, but these errors were encountered: All reactions...

Upgraded Q -> M from 163 [1657895636410]

Judge has assessed an item in Issue 163 as Medium risk. The relevant finding follows: --- The text was updated successfully, but these errors were encountered: All reactions...

dont use transfer ,instead use call

Lines of code Vulnerability details details To withdraw eth it uses transfer, this trnansaction will fail inevitably when : - The withdrwer smart contract does not implement a payable function. Withdrawer smart contract does implement a payable fallback which uses more than 2300 gas unit Thw...

Upgraded Q -> H from 187 [1657760940788]

Judge has assessed an item in Issue 187 as High risk. The relevant finding follows: 1. rescueETH cannot rescue Ether rescueETH sends msg.value to the destination address, which means it requires the caller of rescueETH to provide the Ether to send. Essentially the owner is directly paying the...

Small scalar may make buyoutPrice calculation incorrectly

Lines of code Vulnerability details Impact In Buyout.start function, buyoutPrice is calculated using a scalar = 100. This small scalar and division rouding will reduce the precision of the formula significantly. Proof of Concept 1. Alice call start with msg.value = 1e18 and depositAmount = 4567...

createCollectionFor doesn't transfer ownership to _controller that specified in the parameter.

Lines of code Vulnerability details Impact controller is supplied to the function parameter to be controller of the token and owner of the vault. But actually owner of the vault is VaultRegistry that can't control anything in the Vault. Proof of Concept function createCollectionFor bytes32...

Anyone can call propose() and create a DOS

Lines of code Vulnerability details A denial of service can be initiated by anyone, since there is no check for any address, and also no Ether/ERC20 at stake. Impact Denial of service for vault migration Proof of Concept Contract : Migration.sol, function propose Recommended Mitigation Steps Make...

call() should be used instead of transfer() on address payable

Lines of code Vulnerability details call should be used instead of transfer on address payable Impact The use of the deprecated transfer function for an address wll make the transaction fail when 1. The withdrawer smart contract does not implement a payable function. 2. The withdrawer smart...

ERC20 Incorrect check on returnedAddress in permit() results in unlimited approval of zero address

Lines of code Vulnerability details Impact When creating ERC20.sol from Solmate, a require in permit was converted to a custom error incorrectly. It now reads: if recoveredAddress != address0 && recoveredAddress != owner revert Invalidmsg.sender, owner; So if the recoveredAddress is non-zero and...

Insufficient oracle validation

Lines of code Vulnerability details Impact Oracle my return stale price. Proof of Concept Round completeness and the quoted timestamp are not checked to ensure that the reported price is not stale. roundId, startedAt, updatedAt, and answeredInRound are omitted from the return result of...