ProxyFactory can circumvent ProxyRegistry

Lines of code Vulnerability details Impact The deployFor function in MIMOProxyFactory.sol can be called directly instead of being called within MIMOProxyRegistry.sol. This results in the ability to create many MIMOProxies that are not registered within the registry. The proxies deployed directly...

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

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

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

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

The lazy update of _currentProxies results in non-owners being able to execute functions that require owner privileges

Lines of code Vulnerability details Impact In the MIMOProxyRegistry contract, the getCurrentProxy function returns the address of the mimoProxy contract corresponding to the owner's address in currentProxies. function getCurrentProxyaddress owner external view override returns IMIMOProxy proxy...

Malicious callers can replay disputes

Lines of code Vulnerability details Unlike some of the other signature based operations in the Rigor system, dispute signatures do not include a nonce and are vulnerable to replay attacks. This is similar to my finding in 339, but lower severity, since it is more of a spamming/griefing vector...

Community's escrow allows for signature replay

Lines of code Vulnerability details checkSignatureValidity verification by signature do not utilize nonces and can be tricked by using owner / builder signatures from earlier calls. Namely, while checkSignatureValidity's approvedHashes based way can used only once as it deletes the corresponding...

Multicall does not check if the owner has changed after calls has been made(msg.sender misuse)

Lines of code Vulnerability details Impact The multicall doesn't check if the owner has changed after call or calls has been made. The transferOwnerShip contracts/proxy/MIMOProxy.sol/ requires that the owner is the msg.sender, before ownership can be changes, which is exactly what multicall can d...

Owner of project NFT has no purpose

Lines of code Vulnerability details Owner of project NFT has no purpose Impact Creating a new project mints a NFT to the sender builder. The builder of a project has special permissions and is required to perform various tasks. However, if the minted NFT is transferred to a different address, the...

Logic error

Lines of code Vulnerability details Impact This could result in ignoring a task from allocating. Also this could cause in incorrect memory of array. Proof of Concept // Local instance of lastAllocatedChangeOrderTask. To save gas. uint256 i = lastAllocatedChangeOrderTask; // Local instance of...

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

It should not submit a project with no total budget. Requires at least one task with cost > 0

Lines of code Vulnerability details Impact When publishing a project, there is still possibility the project doesn't have any task or 0 budget. Proof of Concept According to contest guideline, there is an information says "Note that you cannot submit a project with no total budget. Therefore it...

Calculating project cost is vulnerable to reaching block gas-limit

Lines of code Vulnerability details Impact The function Project.projectCost calculates the project costs by calculating the sum of all project task costs. However, due to the unbound for loop, iterating over a potentially large amount of project tasks, this function can potentially DoS due to...

Possible DOS in lendToProject() and toggleLendingNeeded() function because unbounded loop can run out of gas

Lines of code Vulnerability details Impact In Project contract, the lendToProject function might not be available to be called if there are a lot of Task in tasks list of project. It means that the project cannot be funded by either builder or community owner. This can happen because lendToProjec...

Possible signature replay in updateTaskHash() and updateProjectHash() function

Lines of code Vulnerability details Impact In updateProjectHash function, the data encoded only hash and nonce value but not the projectAddress. In case builder had 2 or more projects, the signature that builder used in updateProjectHash can also be used in other projects by attackers. bytes memo...

Insecure SignatureDecoder.recoverKey method

Lines of code Vulnerability details Impact The SignatureDecoder.recoverKey function can return address0 and incur errors, so it is considered insecure. Proof of Concept For example, in Project.checkSignatureValidity a signature could be accepted as valid if any of the addresses used have not yet...

Can add member and publish project to future community without community owner approval

Lines of code Vulnerability details Impact Anyone can add himself as a member of community for any future community. This can be done due to a combination of facts: Non initialized address storage values are defaulted to address0 addMember doesn't check if community is already created...

updateProjectHash does not check project address

Lines of code Vulnerability details In Project.sol, function updateProjectHash L162, data which is signed by builder and/or contractor does not contain a reference to the project address. In all other external functions of Project.sol, data contains the address of the project, used in this check:...

Community can lose interest because interest was calculated by days instead of seconds

Lines of code Vulnerability details Impact Community's owner lose amount of interest from project up to half of total interest Proof of concept When builder repay any loan amount by function repayLender or community call function lendToProject, function claimInterest in contract Community will be...

Add members to the not yet created community

Lines of code Vulnerability details Impact There is a addMember function in the Community. The function accepts data that should be signed by the community.owner and newMemberAddr. // Compute hash from bytes bytes32 hash = keccak256data; // Decode params from data uint256 communityID, address...

Mathematical impossibility in Modifier

Lines of code Vulnerability details Impact In the resolvable modifier, there is a mathematical check for disputeID disputeCount, and unless this is true, the modifier will revert. The default value of the variable disputeCount, which must be mathematically large, is 0 and its value is never...

Signature replay

Lines of code Vulnerability details Impact Neither the signed content nor the signature are associated with the contract DOMAINSEPARATOR. Therefore, both can be repeated in other contracts that use similar values, usually the same builder or contractor addresses.. Proof of Concept In some areas o...

Builder can pay only half of the agreed upon interest on his debt by exploiting daily interest accrual

Lines of code Vulnerability details Impact Project bulder can get away with paying the community owner only half of the agreed upon interest on his debt. Theoretically, the community owner can defend himself from this exploit, if he's aware of this bug, but chances are that he won’t be aware. Pro...

In Community.sol, the interest calculation formula is not correct.

Lines of code Vulnerability details Impact Currently, it calculates interest using the number of days and builders would pay nearly half or less interest than they should. So lenders wouldn't get the interest as expected and it means builders can steal the interest from lenders. Proof of Concept ...

Upgradeable contract is missing a __gap[50] storage variable to allow for new storage variables in later versions

Lines of code Vulnerability details Upgradeable contract is missing a gap50 storage variable to allow for new storage variables in later versions Impact For upgradeable contracts, there must be storage gap to "allow developers to freely add new state variables in the future without compromising t...

Possible to create projects with currency=address(0)

Lines of code Vulnerability details Impact Sometimes it is possible to create projects with currency=address0. Proof of Concept if you call createProject before the contract has been initialized, some areas of the Project service might be denied. Nowhere is it checked that the address is empty,...

block.timestamp used as time proxy

Lines of code Vulnerability details block.timestamp used as time proxy Summary: Risk of using block.timestamp for time should be considered. Details: block.timestamp is not an ideal proxy for time because of issues with synchronization, miner manipulation and changing block times. This can be...

No storage gap for Upgradable contract might lead to storage slot collision

Lines of code Vulnerability details Impact For Upgradable contracts, there must be storage gap to “allow developers to freely add new state variables in the future without compromising the storage compatibility with existing deployments” quote OpenZeppelin. Otherwise it may be very difficult to...

No check if 'ecrecover()' fails

Lines of code Vulnerability details Impact addMember function makes a call to 'ecrecover' to grab the 'community owner' & 'new member'. The problem is that if 'ecrecover' fails it will return a 0 which is currently not guarded against. This was a problem in the famous Polygon Matic token bug. The...

New owner of user proxy can prevent old owner from using the system

Lines of code Vulnerability details Impact In deployFor, owner is called if there is already an entry for the provided address. This can be exploited by a sophisticated attacker to make the system completely unusable for a user. Proof Of Concept Alice transfers her proxy to Bob through...

Registry not updated when proxy ownership is transferred

Lines of code Vulnerability details Impact When the ownership of a MIMOProxy instance is transferred using the transferOwnership function, the MIMOProxyRegistry contract is not updated, although it is the main entry point of all the workflows, as described in the README.md file. Even after the...

minGasReserve of MIMOProxy can be overwritten

Lines of code Vulnerability details Impact While there is a check that owner is not changed in a delegatecall, such a check is missing for minGasReserve, which means that the variable can be changed either maliciously or accidentally because of a storage slot collision. The consequences of this a...

Vault rebalancing can be exploited if two vaults rebalance into the same vault

Lines of code Vulnerability details Impact User funds stolen Proof of Concept Swap data is completely arbitrary and can be used to swap though malicious ERC20 tokens allowing control transfer. This control transfer would allow the attacker to call rebalance on a second vault and exploit both as...

Anyone can create disputes if contractor is not set

Lines of code Vulnerability details Impact Disputes enable an actor to arbitrate & potentially enforce requested state changes. However, the current implementation does not properly implement authorization, thus anyone is able to create disputes and spam the system with invalid disputes. Proof of...

Old owner can still set automation / management for vaults after ownership transfer

Lines of code Vulnerability details Impact When the ownership of a user proxy is transferred, proxyRegistry.getCurrentProxy does not reflect this and still returns the proxy for the previous owner. This can be exploited in the access check of setAutomation. Because setManagement has the same chec...

Divide before multiply may create unexpected values on interests

Lines of code Vulnerability details Divide before multiply may create unexpected values on interests Impact Solidity integer division might truncate. As a result, performing divide before multiply can sometimes create loss of precision. Details If for example we have an operation: 2/33 The expect...

Lack of check for fee tokens

Lines of code Vulnerability details Impact The lendToProject function will receive the cost as an argument. However, some tokens will charge a fee, which might cause the balance of the token in the contract mismatch the cost. File: contracts/Project.sol /// @inheritdoc IProject function...

Missing EIP-155 replay attack protection

Lines of code Vulnerability details Impact publishProject/addMember/escrow in Community and inviteContractor/updateProjectHash/addTasks/setComplete/changeOrder in Project use ecrecover for signed messages to check access. However, all signed messages that is used to check access do not include an...

PTP-03: Unsafe logic in repayLender() function

Lines of code Vulnerability details PTP-03: Unsafe logic in repayLender function Impact reduceDebt internal function is called before the actual transfer of tokens, meaning that it doesn't matter if the transaction fails or succeeds for any reason, debt will be reduced, causing a loss for the...

Unclaimed interest is 0 within 24 hours since the last time that loan is lent or interest is earned for project

Lines of code Vulnerability details Impact When returnToLender is called, the unclaimed interest is calculated as below. uint256 noOfDays = block.timestamp - communityProject.lastTimestamp / 86400; // 246060 /// Interest formula = principal APR days / 365 1000 // prettier-ignore uint256...

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

the lenderFee must be multiplied with 10

Lines of code Vulnerability details There is no check to ensure if the lenderFee is multiplied by 10 Finding: File: /blob/main/contracts/HomeFi.sol 115: lenderFee = lenderFee; // the percentage must be multiplied with 10 194: lenderFee = newLenderFee; --- The text was updated successfully, but...

Project.raiseDispute() might work with the already completed task.

Lines of code Vulnerability details Impact Project.raiseDispute might work with the already completed task. Already completed tasks can't be changed in any cases and it might bring some unexpected outcome when the dispute is approved by fault. Proof of Concept It's impossible to change anything...

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

Signature Replay Attack - Signatures are reusable

Lines of code Vulnerability details checkSignatureValidity performs a check to verify that the signer is the correct address. In the case of Community this is done on the following data: uint256 communityID, address builder, address lender, address agent, address project, uint256 repayAmount, byt...

Use call() instead of transfer() when transferring ETH in DebtToken

Lines of code Vulnerability details Impact When transferring ETH, use call instead of transfer. The transfer function only allows the recipient to use 2300 gas. If the recipient uses more than that, transfers will fail. In the future gas costs might change increasing the likelihood of that...

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

_postRebalanceChecks doens't properly account for flashloan fees

Lines of code Vulnerability details Impact Rebalances that should fall outside of variance requirement are approved Proof of Concept MIMOAutoBalance.solrebalance and MIMOManagedRebalance.solrebalance pass flData.amount into isVaultVariationAllowed. However, in MIMORebalance.solrebalanceOperation...

Unexpected use of array indexes in HomeFi.sol

Lines of code Vulnerability details Unexpected use of array indexes in HomeFi.sol Impact Elements in array are expected to start from 0 when regular indexes are used, however, cause of the order of operations in the HomeFi.createProject function, the element 0 will never be used as index in...