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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

MIMOEmptyVault.sol executeOperation() does not transfer the Vault leftover assets to the owner, it is locked in the MIMOEmptyVault

Lines of code Vulnerability details Impact MIMOEmptyVault.sol executeAction is supposed to pay off the debt and return the leftover assets to the owner of the Vault But In fact the emptyVault contract, after executing the executionOperation, only pays back the flash loan, and does not transfer th...

Manager can drain vault by taking flashloan in unexpected currency

Lines of code Vulnerability details Impact Manager can drain vault Proof of Concept address fromCollateral = vaultsData.vaultCollateralTypevaultId; uint256 rebalanceValue = priceFeed.convertFromfromCollateral, rebalanceAmount; The source of the exploit is that the above lines wrongly assume that...

H02: code is not same as intended function description

Lines of code Vulnerability details MIMOManagedAction.sol, 27-39 / @notice Sets a vault management parameters @dev Can only be called by vault owner and can only appoint whitelisting managers as manger @param vaultId Vault id of the vault to be put under management @param mgtParams ManagedVault...

H03: do use default zero to check account exist.

Lines of code Vulnerability details MIMOProxyRegistry.sol, deployFor function deployForaddress owner public override returns IMIMOProxy proxy IMIMOProxy currentProxy = currentProxiesowner; // Do not deploy if the proxy already exists and the owner is the same. if addresscurrentProxy != address0 &...

H01: code is not same as intended function description

Lines of code Vulnerability details MIMOAutoAction.sol, 26-38 / @notice Sets a vault automation parameters @dev Can only be called by vault owner @param vaultId Vault id of the vault to be automated @param autoParams AutomatedVault struct containing all automation parameters / function...

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

Lines of code Vulnerability details Impact For upgradeable 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...

Returned Value Of Function Call Not Verified

Lines of code Vulnerability details When executing a function call, two data are returned success condition and return data. It was observed that after executing the command via a function call, the contract verifies the success condition to ensure that the call did not revert. However, it does n...

NO STORAGE GAP FOR UPGRADEABLE CONTRACT MIGHT LEAD TO STORAGE SLOT COLLISION

Lines of code Vulnerability details NO STORAGE GAP FOR UPGRADEABLE CONTRACT MIGHT LEAD TO STORAGE SLOT COLLISION Impact For upgradeable contracts, there must be storage gap to “allow developers to freely add new state variables in the future without compromising the storage compatibility with...

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

Upgraded Q -> M from 93 [1659526885300]

Judge has assessed an item in Issue 93 as Medium risk. The relevant finding follows: Use call instead of transfer Contract ETHRegistrarController.sol Description It was observed that withdraw function is using transfer function instead of call for transferring ether. This could become a problem i...

Will Not Work If Signatures Are Not Sorted

Lines of code Vulnerability details Commands cannot be executed if the signatures submitted are not stored. Proof-of-Concept Consider the following is submitted to the AxelarAuthWeighted.validateSignatures function. Operators = Alice, Bob, Charles Signatures = Charles's Signature, Bob's Signature...

Anyone Can Become Owner Of XC20Wrapper Contract

Lines of code Vulnerability details Anyone can become the owner of the XC20Wrapper contract by calling the XC20Wrapper.setup function. Proof-of-Concept The XC20Wrapper contract inherits from Upgradable contract. contract XC20Wrapper is AxelarExecutable, Upgradable As such, the XC20Wrapper contrac...

Anyone can steal the ether or the ReceiverImplementation tokens

Lines of code Vulnerability details Impact Anyone can steal the ether or the ReceiverImplementation tokens. Proof of Concept As you can see in the receiveAndSendNative method: function receiveAndSendNative address payable refundAddress, string calldata destinationChain, string calldata...

selfdestruct will be banned after "The Purge" phase of ethereum merge. DepositReceiver may be broken. Any interaction between AxelarDepositService may result in the loss of fund.

Lines of code Vulnerability details Impact selfdestruct will be banned after "The Purge" phase of ethereum merge. DepositReceiver may be broken. Any interaction between AxelarDepositService may result in the loss of fund. Since AxelarDepositService that deal with native token is highly associated...

Previous {Operators/Weights/Threshold} Are Still Able To Sign Off New Commands After Operatorship Is Transferred

Lines of code Vulnerability details The administrator will call AxelarAuthWeighted.transferOperatorship function to transfer the operatorship to a new set of Operators/Weights/Threshold. However, it was observed that after transferring the operatorship to a new set of Operators/Weights/Threshold,...

Unable To Verify Signature If There Are Too Many Signers And Operators

Lines of code Vulnerability details It was observed that there is a nested for loop within the AxelarAuthWeighted.validateSignatures function. If there are a large number of signatures and operators, it will result in an "Out of Gas" error or a "Block Gas Limit" error and the command's signature...

Anyone can self-destruct this contract.

Lines of code Vulnerability details Impact High Unprotected call to a function executing selfdestruct/suicide. Proof of Concept DepositHandler.destroyaddress contracts/DepositHandler.sol28-30 allows anyone to destruct the contract. Once the contract is unlocked through the noReenter modifier,...

Users will be able to receive tokens through _executeWithToken() after RemoveWrapping() by onlyOwner

Lines of code Vulnerability details Impact Whenever owner removes wrapping by calling removeWrapping , it sets the wrappedaxelarToken to 0. This would allow users to be able to call executeWithToken when the function is used in the future by calling a function that calls it i.e if this function i...

Not calling approve(0) before setting a new approval causes the call to revert when used with some tokens

Lines of code Vulnerability details Not calling approve0 before setting a new approval causes the call to revert when used with Tether USDT Impact Some tokens like USDT do not work when changing the allowance from an existing non-zero allowance value it will revert if the current approval is not...

Add cancel and refund option for Transaction Recovery

Lines of code Vulnerability details Add cancel and refund option for Transaction Recovery Impact Transactions could fail or stuck, according to the documentation: Occasionally, transactions can get "stuck" in the pipeline from a source to destination chain e.g. due to one-off issues that arise wi...

Malicious operators within epoch can not be manually invalidated

Lines of code Vulnerability details Impact Messages are verified and validated by a set of operators. Operators their weights and threshold are defined per epoch and stored as a hash. Transferring operatorship which means creating a new set of valid operators creates a new epoch. Operator sets th...

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

Unprotested _setup function in XERC20Wrapper via Upgradable Contract

Lines of code Vulnerability details Impact If a caller calls the setup function and the address within the IMPLEMENTATIONSLOT does not equal zero, the function will call setup. The setup function changes the ownership of the contract. Proof of Concept The Upgradable contract has a setup function...

Potential scenario where an attacker can steal a user's gas refund

Lines of code Vulnerability details Impact Anybody is allowed to add gas for a specific tx. I was trying to figure out whether the following scenarios are possible: 1. Alice sends a public tx where she deposits some gas and calls the gateway contract after 2. Bob, the attacker, catches that...

Refund Address can forcefully receive WETH

Lines of code Vulnerability details Impact In AxelarDepositService.sol , the refundAddress could forcefully receive WETH instead of the refundToken when calling any of the 3 functions refundTokenDeposit , refundNativeDeposit and refundNativeUnwrap. This is possible by due to // Allowing only the...

AxelarDepositService: When wrappedToken is not weth, sendNative may cause users to lose ether.

Lines of code Vulnerability details Impact In the sendNative function of the AxelarDepositService contract, the wrappedToken address is treated as weth-like and the wrappedToken's deposit function is called. If the wrappedToken address is TokenType.External token and is not weth-like and the...

Hash Collision Within The epochForHash Mapping

Lines of code Vulnerability details It is not possible for the administrator to transfer operatorship to a new set of Operators/Weights/Threshold that has been previously configured because a hash collision will occur within the epochForHash mapping. Proof-of-Concept Assuming that OLDKEYRETENTION...

Functions that send Ether to arbitrary destinations

Lines of code Vulnerability details M-1. Functions that send Ether to arbitrary destinations Description Unprotected call to a function that allow a user to refund to another address. Mitigation Ensure that an arbitrary user cannot withdraw unauthorized funds...

XC20Wrapper may lost received token forever if LocalAsset(xc20).mint is reverted indefinitely

Lines of code Vulnerability details Impact XC20Wrapper may lost received token forever if LocalAssetxc20.mint is reverted indefinitely. Similar to ERC20, the spec said that if mitn returns false it means minting is failed. But it is commonly revert instead of returning false which is also a minti...

Approval from non-zero to non-zero amount can lead to DoS

Lines of code Vulnerability details Impact Some tokens like USDT do not work when changing the allowance from an existing non-zero allowance value. They must first be approved by zero and then the actual allowance must be approved. Proof of Concept The following function use an unsafe approvement...

easy to craft input to execute commands that are not SELECTOR_TRANSFER_OPERATORSHIP

Lines of code Vulnerability details Impact For AxelarGateway.execute , the signed messageHash is only used to validateProof for currentOperators , so anyone can craft input to execute commands other than SELECTORTRANSFEROPERATORSHIP. Impact of above would allow anyone to be able to be approved to...

# Refund wont function as intended to refund the desired token in some cases

Lines of code Vulnerability details Refund wont function as intended to refund the desired token in some cases Impact The rationale behind refund is only let the refundAddress to call refund the specific token. But according to current code Anyone else can call the refund function. Other tokens c...

Anyone can receive tokens

Lines of code Vulnerability details Impact As long as there is token balance or WETH in ReceiverImplementation contract, anybody can receive tokens or weth by making a call to AxelarDepositService.nativeUnwrap with their address as recipient Tools Used Manual review Recommended Mitigation Steps...

Approve will fail

Lines of code Vulnerability details Impact Multiple ERC20 tokens require zero address approval before approving the required amount. The contract seems to be missing this. Proof of Concept 1. Observe the receiveAndSendNative function at ReceiverImplementation.sol...