MUMUBIT TOKEN security record

The seller can finish the auction with different tokens in the function `finishAuction(string memory symbol_,uint256 _tokenId)`. Consider the following situation: 1. Bob creates an auction by account A. 2. Bob bids the auction with 100e18 `SHIBA` by account B. 3. At this time, Bob finishes the auction with the symbol `USDT`, and then 100e18 `USDT` will be paid to Bob. 4. Bob gets 100e18 `USDT` and pays 100e18 `SHIBA`. He pays less and gets more. ```solidity function finishAuction(string memory symbol_,uint256 _tokenId) public { ... ... IERC20(tokens[symbol_]).transferFrom(address(this),recipientAddr, receipientAmount); IERC20(tokens[symbol_]).transferFrom(address(this),auction.seller,sellerAmount); ... ... } ``` Round 2 **[Metacraft.pro]**: This function of finishAuction without a symbol of the chain, is set the bids with the default symbol of the chain(BNB in BSC, MATIC in Polygon and so on. **[Certik]**: The native token does not support the `transferFrom` function. Consider adding an native symbol and checking it in the function `finishAuction(uint256 _tokenId)`, for example: ```solidity function finishAuction(uint256 _tokenId) public payable { Auction memory auction = tokenIdToAuction[_tokenId]; require( msg.sender == auction.seller, "Should only be called by the seller" ); require(keccak256(abi.encodePacked("Native")) == keccak256(abi.encodePacked(auction.symbol)),"Symbol is not correct"); ... ... ```

The users can bid the auction with different tokens in the function `bidAuction(string memory symbol_,uint256 price_,uint256 _tokenId)`. Consider the following situation: 1. Alice firstly bids the auction with 100e18 `USDT` and she is the highest bidder now. 2. Bob bids the auction with 200e18 `SHIBA` and 200e18 is greater than 100e18. Bob becomes the highest bidder and it will return 100e18 `SHIBA` to Alice. 3. Alice lost his 100e18 `USDT` and only gets 100e18 `SHIBA`. She almost lost all of the money. ```solidity function bidAuction(string memory symbol_,uint256 price_,uint256 _tokenId) public { .... .... if (price_ > highestBid) { ... ... if( highestBid > 0 ) { require(IERC20(tokens[symbol_]).approve(address(this), highestBid), "Approve has failed"); IERC20(tokens[symbol_]).transferFrom(address(this),highestBidder, highestBid); //payable(highestBidder).transfer(highestBid); } ```

The seller can cancel the auction with different tokens in the function `cancelAuction(string memory symbol_,uint256 _tokenId)`. Consider the following situation: 1. Bob creates an auction. 2. Alice firstly bids the auction with 100e18 `USDT`. 3. At this time, Bob cancels the auction with the symbol `SHIBA`, and then 100e18 `SHIBA` will be returned to Alice. 4. Alice pays 100e18 `USDT` and only gets 100e18 `SHIBA`. She pays more and gets less. ```solidity function cancelAuction(string memory symbol_,uint256 _tokenId) public { .... .... // if there are bids refund the last bid if( amount > 0 ) { require(IERC20(tokens[symbol_]).approve(address(this), amount), "Approve has failed"); IERC20(tokens[symbol_]).transferFrom(address(this),bidder,amount); // payable(bidder).transfer(amount); } .... .... } ```

The seller can finish the auction with different tokens in the function `finishAuction(string memory symbol_,uint256 _tokenId)`. Consider the following situation: 1. Bob creates an auction by account A. 2. Bob bids the auction with 100e18 `SHIBA` by account B. 3. At this time, Bob finishes the auction with the symbol `USDT`, and then 100e18 `USDT` will be paid to Bob. 4. Bob gets 100e18 `USDT` and pays 100e18 `SHIBA`. He pays less and gets more. ```solidity function finishAuction(string memory symbol_,uint256 _tokenId) public { ... ... IERC20(tokens[symbol_]).transferFrom(address(this),recipientAddr, receipientAmount); IERC20(tokens[symbol_]).transferFrom(address(this),auction.seller,sellerAmount); ... ... } ```

```solidity require(payable(msg.sender).balance >=msg.value, "insufficient balance"); ``` The above check is meaningless since the native tokens have already been sent to this contract. For example, a user has 1 BNB and the price the user paid to bid the auction is also 1 BNB, then, in the above statement, the value of `payable(msg.sender)` is zero. As a result, the program is probably blocked.

Within the contract, the `totalStakeAmount` variable signifies the current staked amount. However, in the `openStake()` function, there are no constraints on the parameters provided by the user, such as `_itemToken` and `_tokenAmount`. ```solidity function openStake( address _itemToken, uint256 _tokenAmount, uint256 _duration ) public whenNotPaused virtual { ... IERC20(_itemToken).transferFrom(msg.sender, address(this), _tokenAmount); ... totalStakeAmount+=_tokenAmount; stakeAmounts[_duration]+=_tokenAmount; ... } ``` If an attacker utilizes a malicious `itemToken` to execute this function, it could potentially manipulate `totalStakeAmount`, leading to a DoS attack. This situation also occurs in `stakeAmounts`.

In the `upgradeStake()` function, the contract calculates the user's reward using the remaining `tokenAmount` from `oldstake`, without updating the timestamp for reward reception afterward. This situation may allow the user to leverage the remaining `tokenAmount` from the current duration to repeatedly calculate and receive rewards. ```solidity function upgradeStake( address _itemToken, uint256 _tokenAmount, uint256 _oldDuration, uint256 _newDuration ) public whenNotPaused virtual { ... oldstake.tokenAmount -= _tokenAmount; ... payedProfits += stakeProfit; require(IERC20(_itemToken).approve(address(this), stakeProfit), "Approve has failed"); IERC20(_itemToken).transferFrom(address(this), oldstake.staker, stakeProfit); oldstake.reward = 0; // No update to oldstake.timestamp //totalStakeAmount -=_tokenAmount; if(newstake.staker == msg.sender){ ... }else { ... } ... } ``` Two potential attack scenarios arise: 1. Despite no restriction on upgrading to the same duration in `upgradeStake()`, an attacker could exploit this by upgrading to the same duration. In the `newstake`, the remaining tokenAmount from the current duration would be utilized to calculate rewards again. If the timestamp updates after calculating `oldstake`, upgrading to the same duration would not yield any rewards, as `newDuration_in_days` would be 0. 2. Following `upgradeStake()`, the attacker could utilize the `unStake()` function to withdraw the tokenAmount and its corresponding reward. As the duration's timestamp remains unchanged, the reward calculation within `unStake()` would still rely on the old timestamp. This situation may result in improper reward calculations and potentially exploit the system.

In the `withdrawAllFunds()` function, there's a critical flaw where the total invested amount (`investedAmount[tokenAddress_]`) is reduced by the current balance of the token within the contract (`IERC20(tokenAddress_).balanceOf(address(this))`). This operation assumes that the contract's token balance only increases through legitimate investment actions via `investInWithVerify()`. However, an attacker could artificially increase the contract's token balance by directly transferring tokens to it. If such an action occurs, the subtraction operation could lead to an underflow in `investedAmount[tokenAddress_]`, potentially causing legitimate invested tokens to become permanently locked in the contract due to an inaccurate representation of available funds. ```solidity=702 function withdrawAllFunds(address tokenAddress_) external nonReentrant notContract(msg.sender) withdrawAddressOnly() { uint256 balance = IERC20(tokenAddress_).balanceOf(address(this)); require(investCoinAddress == tokenAddress_ || projectCoinAddress == tokenAddress_, "Coin Address Not Correct!"); require(block.timestamp - claimTimestamp >= 3 days, "Withdraw Fund Until 48 hours after Claim "); if(tokenAddress_ == investCoinAddress){ investedAmount[tokenAddress_] -= balance; refundedAmount[tokenAddress_] += balance; emit IDOStatusChange(tokenAddress_, balance, msg.sender, "Manager Withdraw invest Coins"); } else if(tokenAddress_ == projectCoinAddress){ projectCoinAmount[tokenAddress_] = IERC20(tokenAddress_).balanceOf(address(this)); claimedAmount[tokenAddress_] += balance; emit IDOStatusChange(tokenAddress_, balance, msg.sender, "Manager Withdraw invest Coins"); } else revert("withDraw TokenAddress ERROR"); IERC20(tokenAddress_).safeTransfer(msg.sender, balance); } ```

The given symbol is not checked in the function `executeTrade`.

The array `validOwners` stores the current valid owner addresses. The `setOwner()` function either adds or removes valid owners based on the transaction operation. However, when adding a valid owner to the `validOwners` array, it does not check for duplicates, allowing re-addition of elements already present in the array. ```solidity function setOwner(address _owner, uint8 _status) public onlySelf { ... }else if(_status == 1) { validOwners.push(_owner); } } ``` Within the same function, removal of `validOwners` can be initiated based on the transaction. When scanning the `validOwners` array and encountering the target address, it will be removed. However, in the case of duplicate elements in the array, the `break` operation ensures that only the first matching address is removed, leaving subsequent duplicates unaffected in the array. This discrepancy between the length of `validOwners` and the count of valid signers will impact the execution of sign-related functions within the contract. ```solidity function setOwner(address _owner, uint8 _status) public onlySelf { _validOwners[_owner] = _status; if(_status == 0){ uint256 ownerLength = validOwners.length; for (uint256 i = 0; i < ownerLength; i++) { if (validOwners[i] == _owner) { validOwners[i] = validOwners[ownerLength - 1]; validOwners.pop(); break; } } } ... } ```