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

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

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

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

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

Delegate call can manipulate minGasReserve and the _permissions mapping.

Lines of code Vulnerability details Impact When the proxy delegatecalls the target contract the target contracts code runs in the proxy’s storage. This means the target code has access to all of proxy’s storage including internal mappings. As a result, the target contracts code can manipulate...

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

the customers can’t just trust the admin

Lines of code Vulnerability details The users can’t just trust the admin eve with multi-sig. So the admin can on anytime change the lenderFee to 100% just by calling replaceLenderFee Recommended Mitigation Steps Add a maximum value for lenderFee to ensure that the admin can’t update the lenderFee...

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

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

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

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

Ownership transfer not correctly handled for flash loans

Lines of code Vulnerability details Impact In response to a flashloan, proxyRegistry.getCurrentProxyowner is used to retrieve the correct user proxy and check that this proxy has initiated the flash loan. This can be problematic after ownership transfers of the proxy, where...

User's may accidentally overpay in register() / renew() and the excess will be paid to the vault creator :-

Lines of code Vulnerability details Vulnerability details Impact User's may accidentally overpay in register / renew and the excess will be paid to the vault creator Proof of Concept 1. File: 2022-08-mimo/contracts/actions/MIMORebalance.sol line 129-132: require...

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

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

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

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

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

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

It should never be possible to change the status of a completed task

Lines of code Vulnerability details High Risk Finding Impact In Project.sol, once a task is set as completed by calling function setComplete, the contract pays the subcontractor. Once in this state, in should not be possible to change the task state back to ACTIVE/INACTIVE, because then the same...

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

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