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

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

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

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

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

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

Functions transferFrom() and transfer() will fail because of commented code

Lines of code Vulnerability details Impact These 2 functions will fail just because the dev forgot to remove the commented code ex / sender / wich is very bad for users. Proof of Concept Tools Used visual Recommended Mitigation Steps remove the / and /. --- The text was updated successfully, but...

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

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

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

Malicious delegated contractor can block funding tasks or mark tasks as complete

Lines of code Vulnerability details Impact A malicious delegated contractor can add a huge number of tasks or one task with a huge cost. This would then pose problems in allocateFunds as tasks could not be funded. Builder could remove delegation for the contractor but couldn't replace the...

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

Missing upper limit definition in replaceLenderFee() of HomeFi.sol

Lines of code Vulnerability details Missing upper limit definition in replaceLenderFee of HomeFi.sol Impact The admin of the HomeFi contract can set lenderFee to greater than 100%, forcing calls to lendToProject to all projects created in the future to revert. Proof of Concept Using the function...

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

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

Lent principal includes lender fee

Lines of code Vulnerability details Impact A community owner is able to lend funds to a project by calling the function Community.lendToProject. Lending requires the lender to pay a lenderFee, a percentage of the fee a lender has to pay to the HomeFi system. However, the current implementation...

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

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

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

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

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

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

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

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

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

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

Attacker can use the same param to call Community.escrow() multiple times and reduce all the debt

Lines of code Vulnerability details Impact In Community contract, function escrow is used to reduce debt when lender comed in terms with the builder and agent to reduce debt. It checks that all lender, builder and agent are signed the data. But the issue is there is no nonce value in data which...

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

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

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

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

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

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

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

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

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

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

Contract name will point to old implementation

Lines of code Vulnerability details Impact The owner can update implementation of contract Name. However it was observed that the updation is not done correctly and contract fails to update contractAddresscontractName variable which mean contract name will still point to old implementation Proof ...

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

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

Malicious callers can replay change orders

Lines of code Vulnerability details Unlike some of the other signature based operations in the Rigor system, change order signatures do not include a nonce and are vulnerable to replay attacks. A number of exploits are possible using replayed change orders, including subcontractors extracting...

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

Storage variable modifications when the contract is paused

Lines of code Vulnerability details Impact The function addMember can be called to modify the the storage variable community even if the contract is paused. function addMemberbytes calldata data, bytes calldata signature external virtual override // Compute hash from bytes bytes32 hash =...

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

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

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

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

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

Project party can unilaterally change price payed at task completion

Lines of code Vulnerability details Impact If there has been more than a change in a task's cost through mulitple calls to changeOrder, signatures previously passed can be replayed by one party to change the price payed for the task without consent of the other parties by frontrunning call to...