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

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

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

Unsafe casting from int128 can cause wrong accounting of locked amounts

Lines of code Vulnerability details Impact The unsafe casting to int128 variable can cause its value to be different from the correct value. For example in the createLock function, the addition to the locked amount variable is done by locked.amount += int128int256value. In that case, if value is...

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

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

User can't undelegate after he has delegated to someone who have different unlock time.

Lines of code Vulnerability details Impact User can't undelegate after he has delegated to someone who have different unlock time. It will be reverted with "Only delegate to longer lock" Proof of Concept I have created a new testcase to demonstrate this issue. I will upload it to 24 hours after...

Use safetransfer/safetransferFrom consistently instead of transfer/transferFrom

Lines of code Vulnerability details Impact Its a good to add require statement to checks the return value of token transfer or using safetransfer or safetransferFrom on Openzeppelin to ensure the token revert when transfer failure. Failure to do so will cause silent failures of transfer and affec...

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

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

checkpointing of same state values in increaseUnlockTime()

Lines of code Vulnerability details Impact oldLocked and locked referes to same LockedBalance in increaseUnlockTime while calling checkpoint Proof of Concept // See IVotingEscrow for documentation function increaseUnlockTimeuint256 unlockTime ........ LockedBalance memory locked = lockedmsg.sende...

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

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

USE SAFETRANSFER()/SAFETRANSFERFROM() INSTEAD OF TRANSFER()/TRANSFERFROM()

Lines of code Vulnerability details Impact It is a good idea to add a require statement that checks the return value of ERC20 token transfers or to use something like OpenZeppelin’s safeTransfer/safeTransferFrom unless one is sure the given token reverts in case of a failure. Failure to do so wil...

User can have full voting power with virtually no lockup using multiple locks and delegation

Lines of code Vulnerability details Impact User receives voting power of max lock but only locks for a minimal amount of time Proof of Concept In VotingEscrow.solcheckpoint, the duration of the delegatee and the total delegated tokens are used to determine the amount of voting power that a lock...

Malicious creator can create different NFT drop collections with NFTs that will be revealed to correspond to same ipfs token URI

Lines of code Vulnerability details Impact The following createNFTDropCollection function with different name, symbol, and nonce can be called to create different NFT drop collections. function createNFTDropCollection string calldata name, string calldata symbol, string calldata baseURI, bytes32...

Malicious creator can create different NFT collections with NFTs corresponding to same ipfs token URI

Lines of code Vulnerability details Impact A creator can call the following createNFTCollection function with different name, symbol, and nonce to create different NFT collections. function createNFTCollection string calldata name, string calldata symbol, uint256 nonce external returns address...

User Funds are Locked in the VotingEscrow Contract When Delegated User Withdraws

Lines of code Vulnerability details Description There exists an issue when a delegated user attempts to withdraw the locked funds after a lock duration is expired, as a result the funds for the original user who triggered the delegation is lost within the contract. Impact This is an issue because...

Operator can use reentrancy within mintFromFixedPriceSale to bypass limitPerAccount.

Lines of code Vulnerability details Impact An operator can pass a contract as the buyReferrer parameter. When distributeFunds is called it sends ETH to the contracts' receive function. After transferring any NFTs the contract may have to another contract it calls mintFromFixedPriceSale passing a...

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

Upgraded Q -> H from 54 [1660349227136]

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

Upgraded Q -> H from 210 [1660349369322]

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

Take over of MinterRole contract

Lines of code Vulnerability details Impact Any user can call the grantMinter or revokeMinter method and take the mint control of the contract. Proof of Concept The grantMinter and revokeMinter methods are not protected by any type of authentication restriction, when they should have the...

Take over of AdminRole contract

Lines of code Vulnerability details Impact Any user can call the grantAdmin or revokeAdmin method and take control of the contract. Proof of Concept The grantAdmin and revokeAdmin methods are not protected by any type of authentication restriction, when they should have the onlyAdmin modifier. So...

MIMOAutoRebalance#rebalance can be used to drain vault due to reentrancy

Lines of code Vulnerability details Impact Vault is drained instead of rebalanced Proof of Concept The caller of rebalance is allowed to supply arbitrary swapData when calling rebalance. The can be used to swap into a malicious ERC20 token that transfers control to the attacker. This opens the...

Incorrect implementation of access control in MIMOProxy:execute

Lines of code Vulnerability details Description There is a function execute in MIMOProxy smart contract. The function performs a delegate call to the user-specified address with the specified data. As an access control, the function checks that either it was called by the owner or the owner has...

Any permission can be used to execute any code in MimoProxy

Lines of code Vulnerability details Impact If a permission is given in MimoProxy to any function, it can then be abused to whitelist any other msg.sender/function by modifying the permissions mapping. There is a check that the owner was not modified, but no check on permissions which makes sense...

Manager is able to use swapData.dexTxData to Perform Reentrancy

Lines of code Vulnerability details Impact Reentrancy here allows a manager to perform multiple operations on a given pool beyond the max limit. Proof of Concept A Manager, using swapData.dexTxData, can reenter MIMO actions performed on the vault he is managing. The manager deploys a FAKE token a...

Everyone can perform emptyVaultOperation. Everyone can steal leftover par after repaying the loan from any vault. This pattern also applied to other files.

Lines of code Vulnerability details Impact Everyone can perform emptyVaultOperation. Everyone can steal leftover par after repaying the loan from any vault. It is intended to only be able to call from executeOperation through MIMOProxy callback but in fact everyone can execute it. Proof of Concep...

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

MIMOProxy: The transferOwnership function does not update _currentProxies, which causes the core functionality to not work

Lines of code Vulnerability details Impact When the transferOwnership function of the MIMOProxy contract is called, currentProxiesnewOwner is not updated, which causes getCurrentProxynewOwner to return 0 or the wrong MIMOProxy contract address when newOwner executes the function in the current...

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

If a MIMOProxy owner destroys their proxy, they cannot deploy another from the same address

Lines of code Vulnerability details When deploying a new MIMOProxy, the MIMOProxyRegistry first checks whether a proxy exists with the same owner for the given address. If an existing proxy is found, the deployment reverts: MIMOProxyRegistrydeployFor function deployForaddress owner public overrid...

vaultOwner Can Front-Run rebalance() With setAutomation() To Lower Incentives

Lines of code Vulnerability details Impact A vaultOwner who is "not confident enough in ourselves to stay up-to-date with market conditions to know when we should move to less volatile collateral to avoid liquidations." They can open their vault to other users who pay attention to the markets and...

msg.sender lending pool and initiator can be forged in executeOperationin MImoLeverage

Lines of code Vulnerability details Impact Detailed description of the impact of this finding. the function signature is function executeOperation address calldata assets, uint256 calldata amounts, uint256 calldata premiums, address initiator, bytes calldata params external override returns bool...

When a user is given the owner privilege of multiple MIMOProxy through transferOwnership, the user cannot choose which MIMOProxy to use as the current Proxy

Lines of code Vulnerability details Impact currentProxies is only set when the user calls the deployFor function of the MIMOProxyRegistry contract, function deployForaddress owner public override returns IMIMOProxy proxy IMIMOProxy currentProxy = currentProxiesowner; // Do not deploy if the proxy...

Inadequate transferOwnership function prevents new owner from accessing funds

Lines of code Vulnerability details Description There are two sources of truth for identifying the account that owns a given MIMOProxy: 1. MIMOProxy.owner 2. MIMOProxyRegistry.getCurrentProxyaddress The first source of truth is only used within the proxy contract itself, and the second source of...

safeIncreaseAllowance may fail on USDT case

Lines of code Vulnerability details Impact safeIncreaseAllowance may fail on USDT case. If a swap completed with dust fund. Consequence swap will always revert due to non-zero allowance on USDT token. Proof of Concept function safeIncreaseAllowanceIERC20 token, address spender, uint256 value...

Overwriting storage slots in MIMOProxy

Lines of code Vulnerability details Impact The MIMOProxy allows you to delegate a call to another contract from a permission of owner. With a delegate call, the entire storage layout is kept the same as it is on MIMOProxy. It means that if the delegate call will be made for smart contract with...

Malicious targets can manipulate MIMOProxy permissions

Lines of code Vulnerability details The MIMOProxy contract stores per-caller, per-target, per-selector permissions in a nested internal mapping. MIMOProxy.solL21: /// INTERNAL STORAGE /// /// @notice Maps envoys to target contracts to function selectors to boolean flags. mappingaddress =...

[H1] MIMOProxy can be PWNED by malicious delegate call

Lines of code Vulnerability details Impact PBR proxy owner change protection can bypassed / DoS PoC PRBProxy has a protection to prevent malicious delegatecall to overwrite owner. function executeaddress target, bytes calldata data public payable override returns bytes memory response ... ... //...

## [H2] Multicall can be called by a malicious contract after executing a malicious delegatecall

Lines of code Vulnerability details Impact Multicall can be called though a malicious delegate call controlling all function in a vault. PoC This is related to a bug I had reported MIMOProxy can be PWNED by a malicious delegate call where I explain that storage variables can be set in delegate...

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

MIMOProxy.sol can change owner, but not all contracts expect it. Registry.getCurrentProxy() can give wrong outputs.

Lines of code Vulnerability details Impact Registry.getCurrentProxy is expected to return the individual Proxy.sol for a user EOA. But Proxy.sol can change its owner, and Registry will not know it. Thus Registry.getCurrentProxy only show a deployers, and it means nothing for the project. Also,...

QA Report

Prevent possible future storage collisions The target action contracts of MIMOProxy.sol use storage slot 0 for immutable addresses, which doesn't currently pose an issue; however, if in future additional contracts are added which allow this slot in storage to be modified then it could open the...

Registry.sol works bad - it fails to delivere expected functionality

Lines of code Vulnerability details Impact The description of Registry.sol is following: /// Deploys new proxies via the factory and keeps a registry of owners to proxies. Owners can only /// have one proxy at a time. But it is not. There are multiple problems: 1. Proxy owner can change and will...

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

Double spend in execute function from the MIMOProxy

Lines of code Vulnerability details There is batch function in MIMOProxy smart contract. The function is inherited from the BoringBatchable contract. The function accept an array of bytes - call parameters, and do delegate call to addressthis for each of the call parameters. There also is execute...

Malicious manipulation of gas reserve can deny access to MIMOProxy

Lines of code Vulnerability details The MIMOProxy contract defines a minGasReserve value as a storage variable: MIMOProxy.solL18: /// @inheritdoc IMIMOProxy uint256 public override minGasReserve; The execute function uses this minGasReserve value to calculate a gas stipend to provide to the targe...

Unchecked Call Return Value

Lines of code Vulnerability details Impact The return value of a message call is not checked. Execution will resume even if the called contract throws an exception. If the call fails accidentally or an attacker forces the call to fail, this may cause unexpected behaviour in the subsequent program...