WEMIX security record
As of 23 Oct 2026, WEMIX has 75 audit reports by 1 security firm on Trustblock; the latest is by CertiK, dated 13 Feb 2025. Those reports list no open critical findings.
Record last updated . It holds audits and findings, contracts and incidents up to Oct 2026. Trustblock does not review code.
Audits75
Newest first
75 audit reports, 72 in CertiK’s retainer series, 3 in CertiK’s series
| Audit | Security firm | Date | Scope | Critical findings | Report |
|---|---|---|---|---|---|
| Wemix Retainer 150 WEMEME | Security firm: CertiK | Date: | Scope: Files named in the report | No open critical findings | Open · skynet.certik.com |
| Wemix Retainer 147 WEMIX Governance | Security firm: CertiK | Date: | Scope: Files named in the report | No open critical findings | Open · skynet.certik.com |
| Wemix Retainer 123 PlayNftMarket | Security firm: CertiK | Date: | Scope: Files named in the report | No open critical findings | Open · skynet.certik.com |
| Wemix Retainer 141 GameServerNFT | Security firm: CertiK | Date: | Scope: Files named in the report | No open critical findings | Open · skynet.certik.com |
| Wemix Retainer 142 - PNIX DEX V2 | Security firm: CertiK | Date: | Scope: Files named in the report | No open critical findings | Open · skynet.certik.com |
| Wemix Retainer 137 HSPFE | Security firm: CertiK | Date: | Scope: Files named in the report | No open critical findings | Open · skynet.certik.com |
| Wemix Retainer 125 WemixPayment Contract | Security firm: CertiK | Date: | Scope: Files named in the report | No open critical findings | Open · skynet.certik.com |
| Wemix Retainer 136 Wemade Phoenix | Security firm: CertiK | Date: | Scope: Files named in the report | No open critical findings | Open · skynet.certik.com |
| Wemix Retainer 129 PlayTokenExchanger | Security firm: CertiK | Date: | Scope: Files named in the report | No open critical findings | Open · skynet.certik.com |
| Wemix Retainer 133 PricePolicy | Security firm: CertiK | Date: | Scope: Files named in the report | No open critical findings | Open · skynet.certik.com |
| Wemix Retainer 131 NightCrowTWAPV3 | Security firm: CertiK | Date: | Scope: Files named in the report | No open critical findings | Open · skynet.certik.com |
| Wemix Retainer 74 (DIOSV3) | Security firm: CertiK | Date: | Scope: Files named in the report | No open critical findings | Open · skynet.certik.com |
| Wemix Retainer 122 TokenPayment | Security firm: CertiK | Date: | Scope: Files named in the report | No open critical findings | Open · skynet.certik.com |
| Wemix Retainer 121 wepublicBank | Security firm: CertiK | Date: | Scope: Files named in the report | No open critical findings | Open · skynet.certik.com |
| Wemix Retainer 119 DIOSV3 (2) | Security firm: CertiK | Date: | Scope: Files named in the report | No open critical findings | Open · skynet.certik.com |
| Wemix Retainer 117 SupportProjectTreasuryV2 | Security firm: CertiK | Date: | Scope: Files named in the report | No open critical findings | Open · skynet.certik.com |
| Wemix Retainer 111 Wonder Staking | Security firm: CertiK | Date: | Scope: Files named in the report | No open critical findings | Open · skynet.certik.com |
| Wemix Retainer 113 Liquid Staking | Security firm: CertiK | Date: | Scope: Files named in the report | No open critical findings | Open · skynet.certik.com |
| Wemix Retainer 103 KLEVA Omni | Security firm: CertiK | Date: | Scope: Files named in the report | No open critical findings | Open · skynet.certik.com |
| Wemix Retainer 102 DEX NFT Service | Security firm: CertiK | Date: | Scope: Files named in the report | No open critical findings | Open · skynet.certik.com |
| Wemix Retainer 94 DEX V3 | Security firm: CertiK | Date: | Scope: Files named in the report | No open critical findings | Open · skynet.certik.com |
| Wemix Retainer 106 DIOS Staking | Security firm: CertiK | Date: | Scope: Files named in the report | No open critical findings | Open · skynet.certik.com |
| Wemix 99 PlayNFTMarket | Security firm: CertiK | Date: | Scope: Files named in the report | No open critical findings | Open · skynet.certik.com |
| Wemix Retainer 100 PlayNFTMarket (hybrid-auction) | Security firm: CertiK | Date: | Scope: Files named in the report | No open critical findings | Open · skynet.certik.com |
| Wemix Retainer 96 UnaMetaHash | Security firm: CertiK | Date: | Scope: Files named in the report | No open critical findings | Open · skynet.certik.com |
Findings646
Statuses are the security firm’s own, from each report.
| Severity | Open | Acknowledged | Fixed |
|---|---|---|---|
| Critical | 0 | 0 | 9 |
| High | 89 | 0 | 23 |
| Medium | 25 | 0 | 83 |
| Low | 180 | 0 | 237 |
CriticalError-Prone Interest Calculation and MisconfigureFixed
Each product has an "interest" parameter which indicates the interest rate for the product. The comment provides an example of the number, which is "535". ```sol=12 struct Product { uint64 maxLeverage; // set to 0 to deactivate product uint64 liquidationThreshold; // in bps. 8000 = 80% uint64 fee; // In sbps (10^6). 0.5% = 5000. 0.025% = 250 uint64 interest; // For 360 days, in bps. 5.35% = 535 } ``` The interest collection is performed in the "_getPnL" function, and the value is calculated as follows: ```sol=490 uint256 _interest = size * interest * (block.timestamp - timestamp) / (UNIT * 10**4 * 360 days); ``` Note that, in the formula, a "UNIT" equal to "10 ** 8" is divided. This indicates that the value for the project's interest is in the format of `value * 10 ** 8`. If the interest is set to a number shown in the comment section, such as 535, the interest will be extremely small (`10 ** 8` times smaller). During an on-chain investigation, it appears that the value is indeed misconfigured in the deployed trading contract.
CriticalIncorrect Price Validation Allows Malicious BiddeFixed
In the `offer` function of the `(C/O/H)HybridAuctionOrder.sol` contracts, they contain the following code that calls the `isValidPrice` function with `_order.price` and the `amount(offer amount)`. ```solidity=104 if (_order.buyer.length == 0) { require(_order.price == amount, "AC: invalid bid price"); } else { require( IGrowthRateManager(growthRateManager).isValidPrice( _order.collection, _order.price, amount, address(_order.payment) == address(0) ? 18 : ERC20(_order.payment).decimals() ), "AC: invalid bid price" ); } ``` The `isValidPrice` in the `GrowthRateManager.sol` is implemented as follows: ```solidity=141 function isValidPrice( address collection, uint price, uint offered, uint decimal ) external view returns (bool isValid) { require( collection.isContract(), "GRM: Invalid collection contract address" ); // Check the validity of the proposed price // Required arguments: collection, price, offered, _auction return IGrowthRate(growthRate).isValidPrice( price, offered, decimal, grVersion[msg.sender] ); } ``` The `_isValidPrice` in the `GrowthRateV1.sol` is implemented as follows: ```solidity=48 function _isValidPrice( uint price, uint offered, uint decimal, uint version ) private pure returns (bool isValid) { if (version == 0) isValid = _nextPriceV1(price, decimal) <= offered && _isValidPriceV1(price, decimal); return isValid; } ``` In the `_isValidPrice` function, the `price` function argument is equal to `_order.price` and the `offered` function argument is equal to the `amount` placed by the bidder. The `_isValidPriceV1(price, decimal);` checks if the `order.price` is a valid price. The auditor believes the check `_isValidPriceV1(price, decimal);` is problematic and allows someone to DOS (Denial of Service) the auction process.
CriticalLogical Issue In Function `unstake()`Fixed
The function `unstake()` is to retrieve the tokens staked by the user, but the function does not call `_claim()`, `_unstake()`, and `_afterUnstake()` of the parent contract internally, so the user will not be able to retrieve the assets. In addition, the function `unstake()` has no privileges and can be called by anyone.
CriticalMissing Access ControlFixed
The current smart contract implementation provides unrestricted access to functions `addZeroFeeAddr`, `addBlackListAddr`, `removeZeroFeeAddr`, and `removeBlackListAddr`. These functions are marked as external, which means they can be called by any account. ```solidity=11 function addZeroFeeAddr(address account) external { require(!zeroFeeList[account], "BridgeAccessControl: Already exists address"); zeroFeeList[account] = true; } function addBlackListAddr(address account) external { require(!blackList[account], "BridgeAccessControl: Already exists address"); blackList[account] = true; } function removeZeroFeeAddr(address account) external { require(zeroFeeList[account], "BridgeAccessControl: not registered address"); zeroFeeList[account] = false; } function removeBlackListAddr(address account) external { require(blackList[account], "BridgeAccessControl: not registered address"); blackList[account] = false; } ``` Any external user can add or remove addresses from the `zeroFeeList` and `blackList` arrays. This implies that any user could arbitrarily set another account as a zero fee account or blacklist an account, without any checks or controls in place to prevent unauthorized modifications.
CriticalMissing Check For `seller` And `amount`Fixed
The function `trade()` supports trade between buyer and seller, but the parameters `seller` and `amount` are not checked for malicious modifications, and if the data is modified, then all of the buyer's funds may be transferred. The parameters `names`, `accounts`, `tradeFee` and `feels` of the `addExchange()` function and the `tradeIDs` of the `settle()` function are also not checked. ### Attack Scenario 1. Bob somehow obtains the transaction data for the unexecuted transaction. 2. Bob modifies the parameters `seller` and `amount` and then sends the transaction.
CriticalMissing Checks On Signatures ReuseFixed
The signature verification process doesn't check for signature reuse. A malicious validator or an attacker-compromised validator could bypass the `validatedsignum` check by using the same signatures repeatedly to reach the required signature amount. ```solidity uint sigNum = SafeMath.div(signatures.length, 65); uint validatedSigNum; bytes32 id_ = id; address ca_ = address(this); address tokenAddr_ = tokenAddr; address from_ = from; address payable to_ = to; bytes32 fromChain_ = fromChain; bytes32 txHash_ = txHash; uint amount_ = amount; // multi sig validation for(uint i = 0; i < sigNum; i++){ bytes calldata sig = signatures[i*65:(i+1)*65]; bytes memory message = abi.encodePacked(id_, ca_, tokenAddr_, from_, to_, fromChain_, txHash_, amount_); if(_validatorRole.isValidatorSig(message, sig) == true){ validatedSigNum++; } } ``` ***Exploit Scenario:*** Let's say there are 5 validators (A-E) in total. The attacker manages to compromise validator A and forge a signature `s`. Then he sets the @param `signatures` as 3 duplicated `s`. The layout of @param `signatures` is like the following: ```rust s||s||s ``` The forged signatures can still pass the valid signatures number check in L303, although there is only one valid signature. ```solidity=303 require(validatedSigNum > SafeMath.div(_validatorRole.numberValidators(),2), "WemixBridge: fail to satisfy sufficient number of valid sig"); ```
CriticalMissing checks on the function parameter `from`Fixed
There are no checks in the function `exit()` that can make sure @param `from` is the `msg.sender`. Function caller can set @param `from` as arbitrary addresses. Since users have to approve the bridge contract for exchanging tokens, anyone can call this function to burn/transfer tokens out of the user addresses: ```solidity= if(Tokens.Crypto(_wemixBridgeStorage.getTokenCryptoType(id)) == Tokens.Crypto.COIN) { require(msg.value == SafeMath.add(amount,feeAmount), "WemixBridge: not enoungh coin"); _payCost(service, address(0), feeAmount, from, _INCOME_EXCHANGE); }else{ if(_wemixBridgeStorage.getTokenMintable(id)){ require(IWemixBridgeERC20(_wemixBridgeStorage.getTokenAddr(id)).burnFrom(from, amount), "WemixBridge: failed to burn token"); //burn }else{ require(IERC20(_wemixBridgeStorage.getTokenAddr(id)).transferFrom(from, address(this), amount), "WemixBridge: failed to transferFrom token"); //deposit to this contract } _payCost(service, tokenAddr, feeAmount, from, _INCOME_EXCHANGE); } ``` Missing checks on @param `from` may cause loss of users' assets and other severe consequences.
CriticalMissing Validation of Position Ownership Allows fFixed
The `NonfungiblePositionHelper` contract acts as a router contract for users to manage their NFT-based liquidity positions. For the contract to operate on behalf of the user, the user must grant NFT approval to `NonfungiblePositionHelper`. The vulnerability arises in the `easyDecreaseLiquidity***()`, `easyCollect***()`, and `easyStrategyChange***()` functions, which are designed to decrease liquidity and collect assets for a given NFT position. The issue here is the lack of access control mechanisms to validate the caller's permissions to operate on the `tokenId` specified. Once a user (User A) grants approval to `NonfungiblePositionHelper`, another user (User B) could exploit this vulnerability by calling `easyDecreaseLiquidity()` or `easyCollect()` with A's position NFT `tokenId` to remove liquidities/claim fees that are owned by User A. ```solidity=578 function easyDecreaseLiquidityCollectAll( EasyDecreaseLiquidityCollectAllParams calldata params ) external payable override notInDenylist nonReentrant sendTokensByTokenIdAfterAction(params.tokenId) returns (uint256 amount0, uint256 amount1) { return _decreaseLiquidityCollectAll( DecreaseLiquidityCollectAllParams({ tokenId: params.tokenId, liquidity: params.liquidity, amount0LiquidityMin: params.amount0LiquidityMin, amount1LiquidityMin: params.amount1LiquidityMin, deadline: params.deadline, recipient: address(this) }) ); } function _decreaseLiquidityCollectAll( DecreaseLiquidityCollectAllParams memory params ) internal returns (uint256 amount0, uint256 amount1) { INonfungiblePositionManager(manager).decreaseLiquidity( INonfungiblePositionManager.DecreaseLiquidityParams({ tokenId: params.tokenId, liquidity: params.liquidity, amount0Min: params.amount0LiquidityMin, amount1Min: params.amount1LiquidityMin, deadline: params.deadline }) ); (amount0, amount1) = INonfungiblePositionManager(manager).collect( INonfungiblePositionManager.CollectParams({ tokenId: params.tokenId, recipient: params.recipient, amount0Max: uint128(-1), amount1Max: uint128(-1) }) ); } ``` Impact: Due to the missing access control and `tokenId` ownership validation, this opens up a critical security risk where an unauthorized contract or user can remove liquidity and collect owed tokens from rightful position owners.
CriticalRevoked roles retain sensitive privilegesFixed
The `RoleManager` contract, which inherits from `AccessControlUpgradeable`, is responsible for managing access roles within the protocol. The `RoleManager::revokeRole()` function intends to revoke a role from an account by removing the account from the role list `_adminList[role]`, it is missing to invoke `super._revokeRole()` from the inherited `AccessControlUpgradeable` contract. This oversight results in the revoked account retaining its permissions, as the role membership in the `AccessControlUpgradeable` is not updated to reflect the revocation. ```solidity=60 function revokeRole(bytes32 role, address account) public override checkRole(DEFAULT_ADMIN_ROLE) { require(_adminList[role].contains(account), "RM: Invalid Account"); _adminList[role].remove(account); // Missing: super._revokeRole } ```
HighLack of Transfer Restriction Before Complete CoulWemix Retainer 150 WEMEME · CertiK · 13 Feb 2025Fixed
The `WEMEME` token inherits the standard `ERC20` contract, which does not have any transfer restriction. If a user purchases the `MEME` token via the bonding curve, he could create a token pair via `playSwapRouter` between the `MEME` token and Wemix using any arbitrary ratio between the two tokens, even before the bonding curve's wemix reserves and token holder count reach their thresholds. This effectively determines the price between the two tokens on the `playSwapRouter`. When the wemix reseves and token holder count do reach the threshold, the `_complete()` function would add liquidity to the token pair at the price determined by the `playSwapRouter` and the corresponding token pair contract, which could be very different from the price of the bonding curve (i.e. the price of `WEMEME` token could be higher via `playSwapRouter` than via the bonding curve). With the price discrepancy, the original liquidity provider of the token pair, or other buyers of the `WEMEME` token via the bonding curve, could potentially sell large amount of `WEMEME` token immediately after the token is "complete" on the bonding curve and profit off of the price discrepancy and extract a large portion of WEMIX token from the token pair.
Open the reportShow 10 more
HighMissing access control in `cancel()` function allWemix Retainer 123 PlayNftMarket · CertiK · 16 Dec 2024Fixed
The `cancel()` function in the `PlayMarketOfferBiddingOrder` contract is intended to be called by the market to allow users to cancel their orders. However, this function is missing the `onlyMarket` access control modifier. This vulnerability allows malicious actors to directly call the `cancel()` function in the `PlayMarketOfferBiddingOrder` contract, enabling them to cancel orders on behalf of other users without their permission.
Open the reportHighUnable to buy orders due to inconsistent usage ofWemix Retainer 123 PlayNftMarket · CertiK · 16 Dec 2024Fixed
In `offerSellOrder()` function, the meaning of the `amount` parameter varying based on `order.orderStrategyType`: - For `order.orderStrategyType == Type.OrderStrategyType.SELL`, buyers need to pay `orderPrice = _order.price * amount`, where `amount` represents the number of NFTs to purchase (`PlayMarketFixedPriceOrder#L52`). - For `order.orderStrategyType == Type.OrderStrategyType.OFFER_BIDDING`, buyers need to pay `amount` of payment tokens, where `amount` represents the order price. To ensure sufficient payment in the second case, the function performs the following check: ```solidity=277 require(amount >= _order.price, "MB: insufficient amount" ); ``` However, this check breaks the first case where `amount` represents the number of NFTs, resulting in buyers not being able to call the `offerSellOrder()` function to buy NFTs if `order.orderStrategyType == Type.OrderStrategyType.SELL`. Additionally, for `OFFER_BIDDING` orders, buyers should be allowed to increase their offered price by any amount, rather than incrementing by `order.price`.
Open the reportHighUnrestricted Metadata ModificationWemix Retainer 141 GameServerNFT · CertiK · 15 Nov 2024Fixed
The `setTokenURI` function is external and lacks access control, enabling any user to change the metadata URI of any token. ```solidity=72 function setTokenURI(uint256 tokenId, string memory metadataURI) external { _setTokenURI(tokenId, metadataURI); } ``` Attackers can alter the metadata of any token, leading to misinformation or inappropriate content.
Open the reportHighFaulty liquidity calculations can result in usersWemix Retainer 103 KLEVA Omni · CertiK · 2 Apr 2024Fixed
In the `EarnRouter` contract, the outcome of `getConvertedFeeRate` rises over time, which means that after a year, the `protocolShare` will represent an equivalent portion of the `protocolFeeRate` within the `totalShare`. As the `protocolShare` is a component of the `totalShare`, the latter also grows in tandem. ```solidity=234 function getProtocolShare(uint256 pid_) public view override returns (uint256) { PoolInfo memory poolInfo_ = _poolInfos[pid_]; if (block.timestamp <= poolInfo_.lastUpdateTime) { return poolInfo_.protocolShareStored; } uint256 timeElapse_ = block.timestamp - poolInfo_.lastUpdateTime; uint256 protocolShare_ = poolInfo_.protocolShareStored + (timeElapse_ * getConvertedFeeRate(pid_) * (poolInfo_.totalShareStored - poolInfo_.protocolShareStored)) / 1e18; return protocolShare_; } ``` When the user calls the `withdrawAll()` function, the protocol fee will be taken in the `userBalance_` due to the increment of the `totalShare_`. ```solidity=346 function withdrawAll(string calldata poolName_) external override nonReentrant { ... uint256 userBalance_ = getBalance(msg.sender, pid_); _withdrawInternal(pid_, userBalance_); } function getBalance(address account_, uint256 pid_) public view override returns (uint256) { if (_poolInfos[pid_].totalShareStored == 0) return 0; uint256 totalBalance_ = getTotalBalance(pid_); uint256 totalShare_ = getTotalShare(pid_); return (totalBalance_ * _userShare[account_][pid_]) / totalShare_; } ``` The `EarnRouter` will execute a withdrawal using less liquidity than what was initially deposited(deduct fee), and the shares converted from this smaller amount of liquidity might still surpass the user's recorded `_userShare`. As a result, the `_userShare` is updated to 0 and the remaining liquidity value is still in the `KlevaDistributor`. Additionally, it allows users to repeatedly claim rewards from the KlevaDistributor due to the presence of residual liquidity. ```solidity=407 function _withdrawInternal(uint256 pid_, uint256 withdrawAmount_) private { ... if (_userShare[msg.sender][pid_] < share_ + 1000) { share_ = _userShare[msg.sender][pid_]; _userPidSet[msg.sender].remove(pid_); } ... _userShare[msg.sender][pid_] -= share_; ... IKlevaDistributor(klevaDistributor).notifyWithdraw( msg.sender, poolInfo.distributorId, removedLiquidity_ ); } ```
Open the reportHighOwnership can be regained after having been renouWemix Retainer 103 KLEVA Omni · CertiK · 2 Apr 2024Fixed
In the contract `Ownable2StepUpgradeable`, if an owner wants to transfer ownership to another, the owner calls the function `transferOwnership()` to update the `_pendingOwner` to the new owner. Then the new owner calls the function `acceptOwnership()` to become the new owner. However, the `_pendingOwner` is not updated to `address(0)`. If the new owner renounces ownership, they can still regain ownership using the `acceptOwnership()` function.
Open the reportHighUsers can withdraw all balances due to the lack oWemix Retainer 103 KLEVA Omni · CertiK · 2 Apr 2024Fixed
A protocol fee will be applied when the user deposits funds into the `EarnRouter`. ```solidity function getBalance(address account_, uint256 pid_) public view override returns (uint256) { if (_poolInfos[pid_].totalShareStored == 0) return 0; uint256 totalBalance_ = getTotalBalance(pid_); uint256 totalShare_ = getTotalShare(pid_); return (totalBalance_ * _userShare[account_][pid_]) / totalShare_; } ``` The existing check within the `EarnRouter` does not prevent an user from withdrawing all of their liquidity, ie. including the fee. ```solidity function _withdrawInternal(uint256 pid_, uint256 withdrawAmount_) private { PoolInfo storage poolInfo = _poolInfos[pid_]; bytes memory data_ = abi.encode(poolInfo.target, withdrawAmount_); bytes memory res_ = _delegate(poolInfo.implementation, Selector.IMPL_WITHDRAW, data_); uint256 removedLiquidity_ = abi.decode(res_, (uint256)); uint256 share_ = _liquidityToShare(pid_, removedLiquidity_); if (_userShare[msg.sender][pid_] < share_ + 1000) { share_ = _userShare[msg.sender][pid_]; _userPidSet[msg.sender].remove(pid_); } poolInfo.totalLiquidity -= removedLiquidity_; poolInfo.totalShareStored -= share_; _userShare[msg.sender][pid_] -= share_; _transferOut(poolInfo.baseToken, msg.sender, withdrawAmount_); IKlevaDistributor(klevaDistributor).notifyWithdraw( msg.sender, poolInfo.distributorId, removedLiquidity_//CertiK: check exists here ); emit Withdrawn(pid_, poolInfo.baseToken, withdrawAmount_, removedLiquidity_); } ``` The check is located in the `KlevaDistributor`; users must make sure that the `removedLiquidity_` does not exceed the previously `addedLiquidity_`. Users can enter the calculated maximum profit amount when calling the `withdraw()` function to bypass the protocol fee.
Open the reportHigh`remainedCoin` Value Not Being Reset Could CauseFixed
In the `_swapDollarToToken()` function, the variable `remainedCoin` is declared outside of the `for` loop starting in line 603. Inside the `for` loop, the value of `remainedCoin` is calculated as `msg.value - fusedBalance` if the token is `WWEMIX` and `msg.value > fusedBalance`, and this amount is sent back to the `_from` address in line 644. The issue is that the value of `remainedCoin` is never reset to 0 after line 644, so as the `for` loop continues, if there are other non `WWEMIX` tokens, line 644 could be reached again when line 624 evaluates to `false`, resulting in sending additional `WEMIX` coin to the `_from` address. If this contract runs out of `WEMIX` balance, then the whole transaction would revert due to the check in line 645.
HighA malicious user can prevent the claiming of an aFixed
When an auction ends, the winner is responsible for claiming the NFT and refunding funds to the other bidders. The `PlayMarketBaseV2` contract allows the use of the native token as the payment token. If the payment token of an auction is the native token and a bidder refuses to receive native tokens, the transaction will be reverted because the `PlayMarketOrderBase._transferToken()` function will ensure a successful transfer of the token. ```solidity=55 if (token == address(0)) { (bool success,) = to.call{value:amount}(""); require(success, "OB: coin not transferred"); ```
HighAny Whitelisted Address Can Drain `_wemixDollar`Fixed
Any whitelisted and not blacklisted address can call the `fission()` and `restoreFission()` functions of the `ReflectRouter` with arbitrary function input. Note that the `bridgedDollar` variable is only a function argument of the `restoreFission()` function, and not a part of the function argument of the `fission()` function. In the `ReflectVault` contract, the `bridgedDollar` variable is not checked against the `_tempVault` variable, and in line 410-412, the `bridgedDollar` amount of `_wemixDollar` is transferred to the `from` address. Given the above, any whitelisted address can extract any amount of `_wemixDollar` from the `ReflectVault` contract by simply calling `fission()` immediately followed by `restoreFission()` function. The only cost is that in the `getFissionClaim()` function, the `claim` being stored in the `_tempVault` variable is 95% of the original amount, in other words, there's a 5% fee for the caller. However, the caller can set a very low `amount`, such that the 5% fee is negligible relative to the amount of `_wemixDollar` stored in the `ReflectVault` contract.
HighIncorrect calculation of `depositAmountB`Fixed
Based on the logic in `getExpectedUproportionalLPBought()`, the calculation `depositAmountB = _amountB + amountToSwap;` is incorrect because tokenB is supposed to be swapped to tokenA. So the correct calculation should be `depositAmountB = _amountB - amountToSwap;`.
Show 10 more
HighIncorrect total supplyFixed
Users receive tokens in proportion to their stake ratio in the Station stage of the DAO. The `dtTotalSupply` represents the total supply of the `DAOToken`. If the DAO reopens, the `dtTotalSupply` will increase because the `ReOpen` mints tokens to the `DAOToken`. This means that users may receive more tokens than expected. However, users who participate in the `ReOpen` will not receive tokens.
HighInitial Token DistributionFixed
In the `AwayMintBurnableERC20` contract, the amount of `_initialSupply * 10 ** _decimals` of the `AwayMintBurnableERC20` tokens are sent to the `bridge.owner()`. This is a centralization risk because the account can distribute tokens without obtaining the consensus of the community. Any compromise to the account may allow a hacker to steal and sell tokens on the market, resulting in severe damage to the project.
HighInitial Token DistributionFixed
All WEMIX tokens are sent to the contract deployer when deploying the contract. This is a potential centralization risk as the deployer can distribute WEMIX tokens without the consensus of the community.
HighLack of access control in `grantRole()` and `revoFixed
The `grantRole()` and `revokeRole()` functions override the functions in the `AccessControl` contract but do not include any access control mechanisms. This could allow any actor to assign or remove any role to any address, potentially leading to misuse or malicious activity.
HighLack of input validation on `_swapFrom`Fixed
Based on the logic in the contract, parameter `_swapFrom` must be either token0 or token1. Otherwise, the calculation and token swapping in `_swap()/_partialSwap()` are incorrect and make no sense.
HighMissing Check in changeQuorum() FunctionFixed
According to the code logic, line 35 is the requirement that assigns a value to `quorum` in the `constructor` function. Notably, quorum needs to be at least 2/3 of the total number of owners. ```solidity line35 require(_quorum <= _owners.length && _quorum >= _owners.length*2/3, "Treasury: Number of confirmations does not satisfy quorum."); ``` Line 310 is a modifier that limits the value requirement for `quorum`. The `changeQuorum()` function in line 208 enforces this restriction. Note that there is no requirement that quorum remains at least 2/3 of the total number of owners. ```solidity line 310 modifier validRequirement(uint256 _ownerCount, uint256 _quorum) { require( _ownerCount <= MAX_OWNER_COUNT && _quorum <= _ownerCount && _quorum != 0 && _ownerCount != 0, "Treasury: Invalid Requirement." ); _; } ``` As a result, it is possible to set a low quorum number below the minimum threshold of 2/3 of total owners, which should not be allowed.
HighMissing Check in the RemoveOwner() FunctionFixed
Line 34 enforces there needs to be at least 3 owners. ```solidity=34 require(_owners.length > 2, "Treasury: Number of owners must be greater than 2 to guarantee it is a voting system."); ``` However, in the `removeOwner()` function, there is no check that after removing the owner, the condition above is still satisfied. This could result in a low owner count of 2 or even 1.
HighMissing Logic in the Fusion FunctionFixed
The `fusion()` function of the `ReflectVaultV3` contract should synthesize game tokens into RFT token. There are two key components that are missing in the current implementation of the function: 1) The `rft` token is never minted or transferred to the `from` address 2) To the extent that `pWemixDollar` is needed to complete the fusion due to insufficient game token, the necessary `pWemixDollar` is never transferred from the `from` address to the `ReflectVaultV3` contract itself. Instead, `pWemixDollar` in the `ReflectVaultV3` contract itself is used to swap to the game token needed, as seen in line 610-611 Additionally, the current implementation allows any caller to drain `_pWemixDollar` from the `ReflectVaultV3` contract. The caller can set `rftAmount` to 0 such that the code in the "try" block would revert due to the `requirie` statement in line 561, and in the "catch" block, any arbitrary `dollarAmount` of `_pWemixDollar` would be transferred from the `ReflectVaultV3` contract to the `from` address.
HighRenounced role is not removed from admin listFixed
The `RoleManager` inherits `AccessControlUpgradeable` and maintains an admin list (`_adminList`) to track granted roles. The `AccessControlUpgradeable::renounceRole()` function, which is responsible for revoking a role from an account, has not been overridden in the `RoleManager` contract to update the internal admin list (`_adminList`). As a result, when an account revokes a role using `renounceRole()`, the account is not removed from the `_adminList`, leading to an inconsistency between the internal state and the expected behavior. Consequently, relying on the `getAdminList()` function to retrieve the current list of administrators would yield inaccurate information, including accounts that have renounced their roles. This could mislead other components relying on this function for making critical access control decisions.
HighrestoreFusion and Fission Functions Could Be UsedFixed
In the `restoreFusion()` function, there's no validation that the function arguments correspond to a previous successful `fusion()` transaction. The `restoreFusion()` transaction should only occur when there is a successful fusion transaction to restore. Otherwise, the `restoreFusion()` function can be used to drain all game tokens from other successful `fusion()` transactions. Specifically, a malicious user could first query the `balance` of each game token in the `ReflectVaultV3` contract, and then calculate the `rftAmount` and `fusionRatio` such that the `fusedBalance` calculated in line 247 equals the `balance`. Then in the foor loop, the entire balance of the game token would be transferred to the `from` address. The malicious could do this one by one for each game token, or combine a few tokens together if their balance at the `ReflectVaultV3` contract are the same. Separately, there's no verification of the `userHash` in the function argument. Any previously unused `userHash` would pass the check. In the `fission()` function, there's no `rft` token being transferred from the `from` address to `address(this)` and there's no burning of the Reflect Tokens as the comment in line 269 indicates. With no effective input validation, all game tokens could be transferred to the `from` address in the `_transferUser()` function when `isDollarReturn` is set to `false`. Note that the behavior of the `Validator` role is unknown and is considered out of scope for this audit.
Show 10 more
HighResumeSwap Function Does Not Unpause SwapsFixed
In the contract `DollarInAndOutStabilizerGame.sol`, the function `ResumeSwap` should call `IWeswapBreaker.unpause` function, rather than `IWeswapBreaker.pause`. The current implementation would result in a permanent blockage of swaps.
HighThe `marketMinPrice` mapping is used but not setFixed
The smart contract contains a mapping for the minimum price per market type (`marketMinPrice`) which is used within the contract to determine price validity and value. However, it has been observed that there is no function within the contract that allows for the modification of `marketMinPrice` values. This oversight leads to incorrect price calculations when the `validStartPrice` function is called, as it relies on static values that do not reflect changes in market conditions or administrative updates. ```solidity=39 mapping(GrmType.MarketType => uint) public marketMinPrice; // marketType => minimum price ``` ```solidity=113 function validStartPrice( address collection, uint price ) external view returns (bool isValid) { require( collection.isContract(), "GRM: Invalid collection contract address" ); // 시작 가격 유효성 체크 // 필요 인자 : CA, price, decimal, minPrice return IGrowthRate(growthRate).validStartPrice( price, minPrices[collection][market[msg.sender]] == 0 ? marketMinPrice[market[msg.sender]] : minPrices[collection][market[msg.sender]], grVersion[msg.sender] ); } ```
MediumIncompatible Payment Token Handling in `PlayMarkeWemix Retainer 123 PlayNftMarket · CertiK · 16 Dec 2024Fixed
The `PlayMarketFixedPriceOrderV2` contract has been updated to inherit from `PlayMarketOrderBaseV3`, which mandates using WWEMIX as the payment token for non-ERC20 payment orders. In the `offer` function of `PlayMarketFixedPriceOrderV2`, the contract checks if the payment token is `address(0)`, implying a transfer of Ether: ```solidity=53 if (_order.payment == address(0)) { require(msg.value == orderPrice, "FP : Invalid value"); } else { IERC20(_order.payment).safeTransferFrom(payer, address(this), orderPrice); } ``` The check does not appropriately handle WWEMIX, as WWEMIX is a wrapped version of WEMIX and should not be recognized as `address(0)`. Furthermore, the internal functions `_nftOffer()` and `_bundleOffer()` fail to unwrap WWEMIX before invoking `_transferToken()`. Consequently, orders with WWEMIX payment cannot be processed correctly, leading to payment failures.
Open the reportMediumIncorrect Token Balance Check for WWEMIX TokensWemix Retainer 123 PlayNftMarket · CertiK · 16 Dec 2024Fixed
In the process of wrapping WEMIX tokens to WWEMIX tokens, the `Checker::checkBalance()` function attempts to verify the strategy contract's token balance. This function checks the balance for a specified token in order to ensure the contract received the correct amount. As shown in the code snippet below, if the token is denoted by `address(0)`, the function checks the ETH balance of the contract using `address(this).balance`: ```solidity=59 function checkBalance(address token, uint value) internal view returns (bool) { uint balance; if (token == address(0)) { balance = address(this).balance; } else { balance = IERC20(token).balanceOf(address(this)); } return balance >= value; } ``` However, since the strategy contract receives WWEMIX tokens rather than the native ETH in the new commit a4c547d, checking the balance of `address(this).balance` when the payment token is `address(0)` will not reflect the WWEMIX token balance, leading to a transaction revert.
Open the reportMediumUnintended cancellation of accepted offersWemix Retainer 123 PlayNftMarket · CertiK · 16 Dec 2024Fixed
Users can cancel their own unaccepted offers to refund their payments. However, this can unintentionally lead to the cancellation of an accepted offer in the following scenario: 1. A seller accepts an offer, updating the `order.orderStatus` to `ACCEPTED`. 2. The owner of the accepted offer does not claim the order immediately, leaving the order status as `ACCEPTED`. 3. Another user cancels their own unaccepted offer, triggering the following code snippet: ```solidity=250 if (_order.orderStatus == Type.OrderStatus.ACCEPTED) { _cancelAccept(_order); } ``` This logic unintentionally cancels the accepted offer when it should only affect the unaccepted offer being cancelled.
Open the reportMediumUsage of slot0 is Easy to ManipulateWemix Retainer 74 (DIOSV3) · CertiK · 16 Aug 2024Fixed
slot0 returns the current tick of the price position of the pool, which can be manipulated easily in the same block if the liquidity in the pool is low or through flash loans. ```solidity=390 (uint160 sqrtPriceX96, int24 tick, , , , , ) = pair.slot0(); ```
Open the reportMediumRisk of supporting "arbitrary" tokensWemix Retainer 122 TokenPayment · CertiK · 13 Aug 2024Fixed
According to the current implementation, the `pay` function plans to support arbitrary types of ERC20 tokens. The `pay` function takes the user input `token`, and the contract performs multiple external calls to the token address. One of the most dangerous behaviors in smart contracts is performing an external call to an untrusted contract. An attacker can deploy a contract and implement the contract logic whatever they want. When the contract performs the external call to a malicious contract, the attacker-defined code will be executed.
Open the reportMediumIncorrect accounting for `currentSupporterNumber`Wemix Retainer 117 SupportProjectTreasuryV2 · CertiK · 28 May 2024Fixed
The state variable `currentSupporterNumber` is designed to keep track of the total number of supporters. The logic implemented to increment and decrement this variable is flawed, leading to incorrect tracking. The problems are identified in two functions: 1. The `support()` function increments `currentSupporterNumber` each time it is called by a supporter. There is no condition to check if the caller has already been accounted for, which results in multiple increments by the same supporter. 2. The `cancelProject()` function attempts to decrement `currentSupporterNumber` under a condition that cannot be fulfilled as the variables `wemixCancelAmounts[i]` and `wepublicPointCancelAmounts[i]` are never set to zero within the function. This leads to a situation where `currentSupporterNumber` does not decrease as intended. Due to these issues, `currentSupporterNumber` does not reflect the true number of unique supporters, and as a result, the contract's status cannot be set to `Cancelled` when it should be: ```solidity=364 if (currentSupporterNumber == 0) { // <= never happen require( totalLockedWemix == 0 && totalLockedWepublicPoint == 0, "SupportProjectTreasuryV2: invalid total amount for a refund" ); status = SupportProjectStatus.Cancelled; } ```
Open the reportMediumSignatures can be reusedWemix Retainer 117 SupportProjectTreasuryV2 · CertiK · 28 May 2024Fixed
The contract functions `cancelProject()` and `settleProject()` lack the necessary protection against signature replay. This is due to the absence of the `onlyUnusedSignature` modifier on these functions. Without this modifier, there is no mechanism to prevent the same signature from being used more than once to authorize these actions. As a result, the `projectOwner` has the capability to replay signatures, allowing them to execute the `cancelProject()` function multiple times with the same signature.
Open the reportMediumDepositors cannot withdraw all their deposits dueWemix Retainer 111 Wonder Staking · CertiK · 15 Apr 2024Fixed
In the function `withdrawRequest()`, there is no check to ensure that the input parameter `amount` or the caller's deposited amount is greater than 0. Therefore, a malicious user can call the function `withdrawRequest()` with `amount = 0` multiple times to reduce `totalDepositors` of the pool to 0. When a depositor attempts to withdraw all tokens from the pool, the `totalDepositors` will underflow and the function will revert. ```solidity=1096 if (user.amount == 0) { pool.totalDepositors--; } ```
Open the reportShow 10 more
MediumIncorrect user account addressWemix Retainer 111 Wonder Staking · CertiK · 15 Apr 2024Fixed
The Staking contract calls the function `ncpDeposit()` to deposit tokens to the pool for `to` account address. The depositor is `to` address. Then the `to` address withdraw their tokens from the pool through the Staking contract using the function `ncpWithdraw()`. Before withdrawing tokens, the function `_updateMP()` is invoked to update reward variables of the given pool. However, The input parameter `account` of the function `_updateMP()` is set as the caller instead of the depositor. This incorrect user account leads to the user receiving fewer rewards.
Open the reportMediumStakers will not be able to withdraw when the pooWemix Retainer 111 Wonder Staking · CertiK · 15 Apr 2024Fixed
A pool can be removed without sanity validation in the function `removePool()`. Any withdrawal attempts after pool removal will revert. In this case, users will lose unclaimed stakings and rewards.
Open the reportMedium`pendingMigrationAmount` is not updated to 0Wemix Retainer 113 Liquid Staking · CertiK · 15 Apr 2024Fixed
In the functions `wonderUpdate()` and `wonderMigrateFromExitPid()`, the `pendingMigrationAmount` tokens are deposited into the new staking pool, but the `pendingMigrationAmount` itself is not updated to 0. If the owner calls the function `wemixWithdraw()` to withdraw `wemix`, it will revert due to the `pendingMigrationAmount` not being 0. ```solidity function wemixWithdraw() external onlyOwner { require( pendingMigrationAmount == 0, "StWEMIXV2::wemixWithdraw: cannot withdraw in migration period." ); (bool success, ) = _msgSender().call{value: address(this).balance}(""); require(success, "StWEMIXV2::wemixWithdraw."); } ```
Open the reportMediumFront-running RiskWemix Retainer 113 Liquid Staking · CertiK · 15 Apr 2024Fixed
The function `initialize()` is used to set the staking address and pid and deposit the contract's balance to the new staking address. This function can only be called once and should be called immediately after deployment by the deployer. However, the chain does not support executing multiple transactions together. Thus, a hacker may front-run the `initialize()` call and set the staking address and withdraw all tokens of this contract.
Open the reportMediumMissing Value Assignment Sentences In State VariaWemix Retainer 113 Liquid Staking · CertiK · 15 Apr 2024Fixed
This function is declared for updating a state variable with passed-in arguments, but no value assignment sentence exists. The function fails to implement its intended function of updating the state variable.
Open the reportMediumPotential Incorrect Staking AddressWemix Retainer 113 Liquid Staking · CertiK · 15 Apr 2024Fixed
The owner calls the function `wonderMigrate()` to update the pending staking address and pending pid, and to claim rewards from the pid pool followed by a request to withdraw from the pid pool. However, the parameter `wonder` represents the pending staking address. If the `wonder` is not equal to `staking`, the contract cannot claim any rewards from `wonder` and may fail to execute the function `withdrawRequest()`.
Open the reportMediumThe function `initialize()` may revertWemix Retainer 113 Liquid Staking · CertiK · 15 Apr 2024Fixed
The function `initialize()` uses the modifier `reinitializer()` to re-initialize. However, the function `initialize()` calls the function `_phaseShift()`, which requires `phaseTwo` to be `false`. If `phaseTwo` was updated in the previous upgrade, the function `initialize()` will revert and will not be able to update the new staking address and `pid` variable.
Open the reportMediumThe function `withdraw()` of the `staking` contraWemix Retainer 113 Liquid Staking · CertiK · 15 Apr 2024Fixed
In the function `wonderMigrateFromExitPid()`, the contract claims rewards from the pid pool and requests withdrawal from the pid pool. However, the function does not invoke the function `withdraw()` of the staking contract to withdraw its deposited amount. The `pendingMigrationAmount` is the sum of the deposit amount and the rewards. Therefore, the function `deposit()` will revert due to insufficient balance.
Open the reportMediumPause token might impact the distributor contractWemix Retainer 103 KLEVA Omni · CertiK · 2 Apr 2024Fixed
The function `harvest()` relies on the `KlevaToken` to mint reward tokens to the `KlevaDistributor`. However, the `KlevaToken` can only mint tokens when the contract is not paused.
Open the reportMediumPotential No withdrawals if the `KlevaToken` mintWemix Retainer 103 KLEVA Omni · CertiK · 2 Apr 2024Fixed
Users deposit their ERC20 tokens or native tokens to the `EarnRouter`, which then transfers them to another pool(such as Wemixfi) to receive liquidity value and converts them to shares for recording. Then, it calls `notifyDeposit()` to deposit the liquidity value to the `KlevaDistributor` to receive rewards. In the `KlevaDistributor` contract, the function `harvest()` is invoked when depositing, withdrawing or claiming rewards, causing the `KlevaToken` to be minted. However, the total supply cannot exceed the cap. Once the cap is exceeded, users cannot withdraw their tokens if the `MinterRole` does not call the function `burn()`. ```solidity function mint( address account_, uint256 amount_ ) external virtual whenNotPaused onlyMinterRole returns (bool) { require(totalSupply() + amount_ <= cap(), "KlevaToken: Cap exceeded"); _mint(account_, amount_); mintedBy[msg.sender] += int256(amount_); return true; } ```
Open the reportShow 10 more
Medium`_safeMint()` should be used rather than `_mint()Wemix Retainer 102 DEX NFT Service · CertiK · 19 Mar 2024Fixed
The `_executeGacha` function uses `mint` function, which does not check that contract recipients are aware of the ERC721 protocol to prevent tokens from being forever locked. `_mint()` is [discouraged](https://github.com/OpenZeppelin/openzeppelin-contracts/blob/d4d8d2ed9798cc3383912a23b5e8d5cb602f7d4b/contracts/token/ERC721/ERC721.sol#L271)in favor of `_safeMint()` which ensures that the recipient is either an EOA or implements `IERC721Receiver`.
Open the reportMediumInherited Contracts Not Initialized In InitializeWemix Retainer 106 DIOS Staking · CertiK · 18 Mar 2024Fixed
Contract StakingV10 extends the `OwnableUpgradeable`, and the `ReentrancyGuardUpgradeable` contracts, but the extended contract is not initialized by the current contract. Generally, the initializer function of a contract should always call all the initializer functions of the contracts that it extends. ------------- Missing the calling of __Ownable_init(). ```solidity /// @custom:storage-location erc7201:openzeppelin.storage.Ownable struct OwnableStorage { address _owner; } /** * @dev Initializes the contract setting the address provided by the deployer as the initial owner. */ function __Ownable_init(address initialOwner) internal onlyInitializing { __Ownable_init_unchained(initialOwner); } function __Ownable_init_unchained(address initialOwner) internal onlyInitializing { if (initialOwner == address(0)) { revert OwnableInvalidOwner(address(0)); } _transferOwnership(initialOwner); } function _transferOwnership(address newOwner) internal virtual { OwnableStorage storage $ = _getOwnableStorage(); address oldOwner = $._owner; $._owner = newOwner; emit OwnershipTransferred(oldOwner, newOwner); } ``` ------------- Missing the calling of __ReentrancyGuard_init(). ```solidity /// @custom:storage-location erc7201:openzeppelin.storage.ReentrancyGuard struct ReentrancyGuardStorage { uint256 _status; } function __ReentrancyGuard_init() internal onlyInitializing { __ReentrancyGuard_init_unchained(); } function __ReentrancyGuard_init_unchained() internal onlyInitializing { ReentrancyGuardStorage storage $ = _getReentrancyGuardStorage(); $._status = NOT_ENTERED; } ```
Open the reportMediumInconsistent payment token supportWemix 99 PlayNFTMarket · CertiK · 1 Mar 2024Fixed
The contract exhibits an inconsistency in its support for native tokens as a form of payment. The function `addNFTPayment()` is designed to add supported payment tokens, but it explicitly disallows the native token by requiring the payment token address not to be zero: ```solidity=243 // addNFTPayment() require(payments[i] != address(0), "MB : payment is zero"); ``` ```solidity=262 // removeNFTPayment() require(collection != address(0) && payment != address(0), "MB: zero address request"); ``` On the other hand, the market contract's `registerBuyOrder()` function suggests that native tokens are intended to be supported as it checks for an address of zero (indicative of native tokens) and processes the payment accordingly: ```solidity=97 // PlayMarketBaseV4::registerBuyOrder() if(_order.payment == address(0)){ require(msg.value == orderPrice, "MB: invalid value"); (result, order) = IOrder(payable(strategy)).open{value:msg.value}(_order); } else { ``` This discrepancy indicates that while the market contract is set up to handle native tokens, the `addNFTPayment()` function prevents their use, which could be a design flaw or oversight.
Open the reportMediumThe first bid in a Dutch auction can be lower thaWemix Retainer 100 PlayNFTMarket (hybrid-auction) · CertiK · 1 Mar 2024Fixed
The contract allows the first bid to be made at either the initial `_order.price` or the next decreased price calculated by `nextDecPrice()`. This does not align with the typical Dutch auction model, where the price should only decrease after a specified period (close delay) has elapsed from the auction's start time. ```solidity=71 require(_order.price == amount || (amount >= nextDecPrice(_order.payment, _order.price) && isValidPrice(_order.payment, amount)), "HA : first bid should be start price or bigger"); ``` Consequently, bidders can opt to bid on a lower price, and the order start price is not consistent with the logic in `orderStartPrice()` function.
Open the reportMedium`TempVault` can be misusedFixed
`TempVault` struct data is created by functions `fusion()/fission()` and used by functions `restoreFusion()/restoreFission()/finalFusion()/finalFission()`. But inside `TempVault` there is flag to indicate whether it is created by `fusion()` or `fission()`. So it is possible that a `TempVault` struct data created by `fusion()` is later used by `restoreFission()/finalFission()` and vice versa, which is obviously incorrect and will lead to incorrect calculation and contract malfunction.
MediumCannot Disable Cool TimeFixed
If the current block number reaches the end of cool time, for example, `stakedBlock(account).add(stakeCoolTime)` equals `block.number`, the `remainXXXCoolTime` function will return zero. However, the `isPassedXXXCoolTime` function returns false, indicating that the account has not yet passed the cool time and will be charged extra fees. Due to the inconsistency between these two cool time view functions, users are unable to stake, claim, or unstake multiple times in a block, even if the cool time is set to zero.
MediumChecks Effects Interaction Pattern Not UsedFixed
The checks effects interactions pattern is not followed in multiple locations in the code base. As an example, in the `_unstake()` function in the `SPBase` contract, the user's `stakedAmount` is decremented after the `st` token is transferred to the user's `account` address. While most ERC20 tokens do not contain a callback functions, ERC777 tokens that are backward compatible with ERC20 tokens do contain callbacks, e.g. https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC777/ERC777.sol If the `st` or `rt` tokens contain any callback function similar to those in ERC777, then there could be reentrancy attack due to not following the checks-effects-interactions pattern and the lack of any reentrancy protection mechanism such as the `nonReentrant` modifier. A reentrancy attack can occur when the contract creates a function that makes an external call to another untrusted contract before resolving any effects. If the attacker can control the untrusted contract, they can make a recursive call back to the original function, repeating interactions that would have otherwise not run after the external call resolved the effects. ```solidity=255 function _unstake(address account, uint amount) internal virtual { uint totalFee; bytes32[] memory columns = _unstakeFeePercents(account); if(columns.length > 0) { if(stVault != address(this)) { require(IERC20(st).transferFrom(stVault, address(this), amount), "SPBase: failed to transferFrom ST from vault"); } for(uint i = 0; i < columns.length; i++) { uint fee = amount.mul(feePercent[columns[i]]).div(DENOMINATOR); _addIncome(service, st, columns[i], fee); totalFee = totalFee.add(fee); emit EnforcedWithFee(account, st, columns[i], amount, fee); } amount = amount.sub(totalFee, "SPBase: total fee is over amount"); require(IERC20(st).transfer(account, amount), "SPBase: failed to unstake"); } else { if(stVault != address(this)) { require(IERC20(st).transferFrom(stVault, account, amount), "SPBase: failed to unstake"); } else { require(IERC20(st).transfer(account, amount), "SPBase: failed to unstake"); } } _userInfo[account].unstakedBlock = block.number; _userInfo[account].unstakeFee = _userInfo[account].unstakeFee.add(totalFee); _userInfo[account].unstakedAmount = _userInfo[account].unstakedAmount.add(amount); _userInfo[account].stakedAmount = _userInfo[account].stakedAmount.sub(amount.add(totalFee), "SPBase: exeeds staked amount"); totalUnstakeFee = totalUnstakeFee.add(totalFee); totalUnstakedAmount = totalUnstakedAmount.add(amount); totalStakedAmount = totalStakedAmount.sub(amount.add(totalFee), "SPBase: exeeds staked amount"); emit Unstaked(account, st, amount, totalFee); } ```
MediumCollection address is not properly savedFixed
Editors can register ERC721 or ERC1155 collection addresses that are tradable on the market. However, the `Collection.collectionAddress` is not being updated with the registered collection address, which causes the registered collection to be unmodifiable and inaccessible.
MediumCompile ErrorFixed
The linked codes are not compatible with solc version v0.6.0 ```solidity= function enter( bytes32 id, address tokenAddr, address from, address payable to, bytes32 fromChain, bytes32 txHash, uint amount, bytes calldata signatures ) ``` The contracts cited have an unlocked compiler version. An unlocked compiler version in the source code of the contract permits the user to compile it at or above version v0.6.0. This, in turn, leads to differences in the generated bytecode between compilations due to differing compiler version numbers. This can lead to ambiguity when debugging, as compiler-specific bugs may occur in the codebase that would be hard to identify over a span of multiple compiler versions rather than a specific one. ***Proof of concept:*** Following is the error message when we try to use compiler version 0.6.0: ```Solidity TypeError: Data location must be "memory" for parameter in function, but "calldata" was given. ```
MediumDangerous Usage of `tx.origin` in `Dispatcher.solFixed
In the current implementation of the contract `Dispatcher.sol`, `tx.origin` is used as the `sender` of the `transferFrom` call. ```solidity= require(IERC20(fromToken).transferFrom(tx.origin, address(this), transferAmount), "Dispatcher: fail to transferFrom for request swap"); ``` However, `tx.origin` check has been deprecated after EIP-3074. EIP3074 introduces two EVM instructions AUTH and AUTHCALL. The first sets a context variable authorized based on an ECDSA signature. The second sends a call as the authorized. This essentially delegates control of the EOA to [a] smart contract. This means there will be a way for smart contracts to send transactions in the context of an Externally Owned Account, thus bypassing this check. **Also tx.origin is widely discourage as there are possible phishing attempts.** For a contract, the `tx.origin` is not transparent. In a simple call chain A->B->C->D, inside D msg.sender will be C, and tx.origin will be A. If the origin is really desired in D, then each of the functions in the contracts B, C, D could take an extra parameter to propagate the origin: A would pass its address (this) to B, B would pass the value to C, and C would pass it to D. `tx.origin` based protection can be abused by a malicious contract if a legitimate user interacts with the malicious contract. Hence it's not recommended to use `tx.origin` for authorization in solidity documentation. Refer to: https://docs.soliditylang.org/en/v0.6.12/security-considerations.html#tx-origin
Show 10 more
MediumFailed to Deploy `FanTokenMinter` for Multiple `FFixed
The `createFanTokenMinter()` function utilizes the `create2` to deploy a new `FanTokenMinter` contract associated with a given `fanToken` address. However, there is a flaw in the current code logic that could lead to the failure of the `createFanTokenMinter` function. This failure occurs because the `FanTokenMinter` contract address is consistently the same during deployment. **Create2.sol** ```solidity=54 /** * @dev Returns the address where a contract will be stored if deployed via {deploy} from a contract located at * `deployer`. If `deployer` is this contract's address, returns the same value as {computeAddress}. */ function computeAddress( bytes32 salt, bytes32 bytecodeHash, address deployer ) internal pure returns (address) { bytes32 _data = keccak256(abi.encodePacked(bytes1(0xff), deployer, salt, bytecodeHash)); return address(uint160(uint256(_data))); } ``` Deploying contract through `Create2` library, the contract address is derived from `keccak256(abi.encodePacked(bytes1(0xff), deployer, salt, bytecodeHash));`. ```solidity=63 //Use when FanToken is created function createFanTokenMinter(address fanToken, address creator) external onlyEditor { require(fanToken.isContract() && creator != address(0), "FTF : address is invalid"); if(!isRegisteredFanToken[fanToken]){ allFanToken.push(fanToken); } isRegisteredFanToken[fanToken] = true; //minter create bytes32 salt = keccak256(abi.encodePacked()); //constructor paramters bytes memory bytecode = type(FanTokenMinter).creationCode; //contract bytecode address minterAddr = Create2.deploy(0, salt, bytecode); IFanTokenMinter(minterAddr).setFanTokenRouter(fanTokenRouter); ... ``` While executing statements from L63 to L64, the local variables `salt` and `bytecode` are consistently assigned identical values. Furthermore, it is noteworthy that the `FanTokenFactory` contract serves as the deployer for the new `FanTokenMinter` contract. Given these circumstances, the address of the `FanTokenMinter` contract, derived from the expression `keccak256(abi.encodePacked(bytes1(0xff), deployer, salt, bytecodeHash));`, will consistently remain unchanged. Consequently, the deployment of the contract would be failure due to address collision.
MediumFunction `removeMinter` Is BrokenFixed
In the current implementation of contract `MinterRoleV2`, @param `bearersArray` is not updated in function `addMinter()`. While in the function `removeMinter`, operation `Array.pop` is performed on the empty `bearersArray`, which causes an exception. The following error will be shown when the function is invoked: ```bash VMError (if there is one): VMError { │status false Transaction mined but execution failed │transaction hash 0x78cd181cfe80530c035a111d5e933a7e344d54e567701891b33f33a46d2fd12c │from 0x5B38Da6a701c568545dCfcB03FcB875f56beddC4 │to MinterRoleV2.removeMinter(address) 0xd9145CCE52D386f254917e481eB44e9943F39138 │gas 3000000 gas │transaction cost 3000000 gas │execution cost 3000000 gas │input 0x309...eddc4 │decoded input { │ "address account": "0x5B38Da6a701c568545dCfcB03FcB875f56beddC4" │} │decoded output {} │logs [] │val 0 wei │transact to MinterRoleV2.removeMinter errored: VM error: invalid opcode. │ │invalid opcode │ │ The execution might have thrown. │} ``` The error will lead to the function `removeMinter()` being inaccessible.
MediumInaccurate Interest Calculation Due to Stale TimeFixed
The smart contract in question exhibits a flaw in the mechanism of interest collection. When a user initiates an open position, the `position.timestamp` is set accordingly. However, the `position.timestamp` is not updated during subsequent increases or decreases in the position size. The interest (`_interest`) calculation, which relies on the elapsed time from `position.timestamp` during the close position, is computed as `size * interest rate * (block.timestamp - timestamp)`. The lack of timestamp updates on position adjustments leads to inaccuracies in interest calculation.
MediumIncompatibility With Deflationary TokensFixed
When transferring standard ERC20 deflationary tokens, the input amount may not be equal to the received amount due to the charged transaction fee. As a result, an inconsistency in the amount will occur and the transaction may fail due to the validation checks.
MediumInconsistent authorization checkingFixed
In function `isAuthorized()`, only whitelisted addresses and token creator are considered authorized. But in modifier `onlyAuthorized()`, the owner of the vault is also considered authorized. Authorized users can withdraw fan tokens from the vault. It is better to have a consistent authorization checking.
MediumIncorrect `Receiver` of the `bridge.exit` callFixed
In the current design of the contract, the `bridge.exit()` function sets its receiver to `_swapInInfo.user`. This `user` is expected to match the `tx.origin` from the source chain. However, an issue may arise in situations where the user's address on the destination chain does not match the address on the source chain. This could happen if the `user` is a smart contract. The PPP contract does not currently account for this scenario, which could lead to the output asset being sent to the wrong address. ```solidity=322 IWemixBridge(CA(PPPConfigs.playBridge)).exit( _swapInInfo.path[_swapInInfo.path.length - 1], address(this), _swapInInfo.user, _swapInInfo.ids[0], _swapInInfo.service, _swapInInfo.toChains[0], SafeMath.sub(_amountOut, _swapInInfo.feeAmounts[0]), _swapInInfo.feeAmounts[0], uint(0) ); ```
MediumIncorrect `totalObligation` updateFixed
After the user creates a new agenda, the `totalObligation` increases and the number of available votes decreases. If the current status is allowed or rejected, `totalObligation` will be decreased. In the function `_reject()`, if the agenda is rejected in the consensus state, the number of creating the agenda will be returned. Otherwise, only the amount of obligations in the consensus state will be returned. However, if the agenda is rejected in the proposal state, the function `_permit()` already returns the amount of obligations in the consensus state when the agenda state is changed from consensus to proposal. Thus, the function `_reject()` should return the amount of obligations in the proposal state, but it currently returns the amount of obligations in the consensus state again. Due to this error, `totalObligation` is being updated incorrectly, and getAvailableNum() is returning an erroneous result. More seriously, the functions `vote()` and `cancel()` may revert if the amount of obligations in the consensus state is greater than the amount of obligations in the proposal state. ```solidity=363 function _reject(uint daoId, uint agendaId) private { Agenda storage agenda = _agendas[daoId][agendaId]; totalObligation[daoId][agenda.info.creator] -= agenda.board[uint(Status.CONSENSUS)].obligationAmount; if(agenda.status == Status.CONSENSUS) { totalObligation[daoId][agenda.info.creator] -= agenda.board[uint(Status.PROPOSAL)].obligationAmount; } agenda.status = Status.REJECT; emit Rejected(daoId, agendaId); } ``` ```solidity=335 function _permit(uint daoId, uint agendaId) private { Agenda storage agenda = _agendas[daoId][agendaId]; uint spellType = agenda.info.spellType; Status status = agenda.status; totalObligation[daoId][agenda.info.creator] -= agenda.board[uint(status)].obligationAmount; ... } ```
MediumIncorrect condition checkFixed
The function `lockToken()` requires the token to be unlocked, and accordingly, the function `unlockToken()` should require the token to be locked. ```solidity=193 require(_wemixBridgeStorage.getTokenLock(id) == Tokens.Lock.UNLOCK, "WemixBridge: token is already unlocked"); ```
MediumIncorrect ERC-20 InterfaceFixed
Incorrect return values for ERC-20 functions. A contract compiled with Solidity > 0.4.22 interacting with these functions will fail to execute them, as the return value is missing. IERC20 (src/lending/IERC20.sol#3-26) has incorrect ERC20 function interface:IERC20.transfer(address,uint256) (src/lending/IERC20.sol#15) ```solidity=15 function transfer(address recipient, uint256 amount) external; ``` --- IERC20 (src/lending/IERC20.sol#3-26) has incorrect ERC20 function interface:IERC20.transferFrom(address,address,uint256) (src/lending/IERC20.sol#17-21) ```solidity=17 function transferFrom( ```
MediumIncorrect token amount parameter value for `getLiFixed
The token amount parameters for `getLiquidity()` are incorrect. For the first `getLiquidity()`, token0 amount should be `_amount - amountToSwap`, not `amounts[0]`; for the second `getLiquidity()`, token1 amount should be `_amount - amountToSwap`, not `amounts[0]`. This bug will lead to incorrect calculation of lp token amount.
Show 10 more
MediumIncorrect Usage of `transferFrom()` FunctionFixed
Within the `initAddLiquidity()` function, the liquidity amount leftovers would refund to `editor` role account. The current code logic causes the failure of assets leftovers refund because the ERC20 token `fanToken/WEMIXDollar` holders cannot transfer from themselves. Before executing codes at L139/L141, no code logic for approving `fanToken/WEMIXDollar` allowance to the `FanTokenRouter` contract itself. **FanTokenRouter.sol** ```solidity=138 if(liquidityAmount > fanTokenLiquidity) { IERC20(fanToken).transferFrom(address(this), msg.sender, liquidityAmount - fanTokenLiquidity); } else if(liquidityAmount > wemixDollarLiquidity) { IERC20(WEMIXDollar).transferFrom(address(this), msg.sender, liquidityAmount - wemixDollarLiquidity); } ``` When code at L343, which belongs to `transferFrom()` function call path, is executed, the boolean expression `currentAllowance >= amount` returns false because `_allowances[FanTokenRouter][FanTokenRouter]` is zero. At the end, the transaction of calling `initAndLiquidity()` is reverted. **ERC20.sol** ```solidity=336 function _spendAllowance( address owner, address spender, uint256 amount ) internal virtual { uint256 currentAllowance = allowance(owner, spender); if (currentAllowance != type(uint256).max) { require(currentAllowance >= amount, "ERC20: insufficient allowance"); unchecked { _approve(owner, spender, currentAllowance - amount); } } } ```
MediumIncorrect use of `msg.sender`Fixed
In the `repayBorrowFresh()` function, if the borrower has no outstanding borrowing balance and the account balance is zero, the borrower exits the market. However, if the sender is not the borrower and they repay the borrower's loan through the `repayBorrowBehalf()` function, the sender will be incorrectly removed from the market.
MediumIneffective `swapFee` and Incorrect CalculationFixed
Both the `_swapToToken()` and the `_swapToDollar()` functions calculate a `swapFee` at the end of the function and returns its value. The `swapFee` is then emitted in events but does not affect the amount of tokens that users receive. As such, its role appear to be ineffective. Additionally, the denominator in line 286 is `10000` but the denominator in line 790 is `9975`. The latter is incorrect if the fee is 0.25% as the comment above indicates.
MediumInherits the `EditorRole` contract to use the `edFixed
The `claim()` function can be invoked by either the `editor` role or the `topBidder` to claim tokens. The `EditorRole::isEditor()` function is utilized to verify the legitimacy of the claimer as a valid `editor`. However, contracts that omit the inheritance of the `EditorRole` contract result in a compilation error.
MediumInsufficient check on `token` parameter in functiFixed
If the `token` parameter does not exist or is invalid, `_tokenIndex[token]` will be zero, which is a valid index. Then `_getToken()` will always return the first token information for any non-existent/invalid `token`. And `_getToken()` is called by `restoreFusion()/getTokenInfo()`, which will result in malfunctioning and incorrect calculation.
MediumInsufficient check on `tokens` parameter in functFixed
The `tokens` parameter may contain redundant token addresses, which will cause incorrect calculation and token transfer in function `restoreFusion()`.
MediumInvalid TWAP time standard checksFixed
The token TWAP price log (`priceLog[twapIndex]`) and its time log (`blockTimeLog[twapIndex]`) are updated when the current time satisfies the following condition: ```solidity=326 require( blockTimeLog[twapIndex] == 0 || block.timestamp >= blockTimeLog[twapIndex] + 1 days, "FTM : Too ealry call, TWAP time standard is 24 hour." ); ``` It is worth noting that `twapIndex` will be changed at the end of the `_calculateTWAP()` function. ```solidity=362 if (twapIndex == MAX_INDEX - 1) { twapIndex = 0; } else { twapIndex++; } ``` After the value of `twapIndex` is changed, - If `twapIndex` is reset to zero, the value of `blockTimeLog[twapIndex]` is significantly smaller than the current time. - If `twapIndex++`, the value of `blockTimeLog[twapIndex]` is zero. Therefore, the check in L326 will be bypassed, and the restriction "TWAP time standard is 24 hour" is never enforced. Furthermore, the statement at L343 is unreachable because `timeStamp` is surely greater than `blockTimeLog[twapIndex]` when the condition `blockTimeLog[twapIndex] != 0` returns true. ```solidity=340 uint twapGrade; if (blockTimeLog[twapIndex] != 0) { if(timeStamp - blockTimeLog[twapIndex] == 0){ twapGrade = highestPriceGrade; } else { // (currentprice - lastprice) / (current time stamp - last time stamp) uint twap = (cumulativeLast - priceLog[twapIndex]) / ( timeStamp - blockTimeLog[twapIndex] ); twapGrade = twap / divider; lastRecordedTwap = twap; } } else { twapGrade = highestPriceGrade; } ```
MediumLack of "_vault" value validationFixed
The `NEITHifyNext` function retrieves the `_vault` value from the `vault[collection]` mapping. If the `vault[collection]` is not set, it will return a 0 address. The funds will end up being sent to the zero address and can never be recovered. ```solidity address _vault = vault[collection]; .... _withdraw( _collection, nextCovenantInfo.covenantType, nextCovenantInfo.claimableAmount, _vault ); ```
MediumLack of `tradeID` Existence CheckFixed
The function `trade()` supports trade between buyer and seller, but does not check if the `tradeID` already exists, if it exists, then the data `_receiptInfo[_tradeID]` will be overwritten and the seller will not receive the funds.
MediumLack Of Access ControlFixed
The function `setSchedule()` sets the schedule of mining governance tokens. The `Schedule` array will be used in calculating the number of governance tokens mined during the period in function `calcMintedGT`. This can be called by anyone as it has no access restriction. The schedule can be updated by any user and will affect its validity, which might cause confusion among users. It will cause much more severe impacts on project operations if the reward schedule is used in reward distribution. Users could set the `reward` of the schedule to an arbitrary amount and get a large number of reward tokens than the original schedule if the governance token has already been minted ahead of the schedule.
Show 10 more
MediumLack of access control on `MYRTLEMinter::myrtleMiFixed
The `MYRTLEMinter::myrtleMigration()` function lacks access control, and the `MYRTLEMinter` will be the minter of the market (`onlyMinter`). Consequently, anyone can call the function `FanTokenMarket::setTokenPriceAmount()` with an arbitrary `newHighestPrice` to set `ledger` and `priceBook`, which makes all funds in the vault available for sale.
MediumLack of access control over `distributionEventEmiFixed
If a trade order is completed, the `PlayMarketTokenManager.distributionEventEmit()` function will be called, and the `PlayMarketRecipient` contract will emit a `DistributionEvent` to track fee charge information. However, as the `PlayMarketTokenManager.distributionEventEmit()` function does not restrict the caller, malicious users can call it to emit a fake `DistributionEvent`.
MediumLack of balance check in the biddingFixed
When users participate in an auction by calling the `PlayMarketEnglishAuctionOrder.offer()` function to bid for an NFT, the function escrows a certain `diff` token amount in the `PlayMarketEnglishAuctionOrder` contract. However, if the payment token is a deflationary token, the escrowed token amount may end up being less than the required `diff`, which in turn prevents the auction winner from claiming the purchased NFT. This occurs due to an insufficient token balance in the strategy contract.
MediumLack of input validation on ` _swapFrom`/`_fromToFixed
Based on the calculation code, `_swapFrom`/`_fromToken` must be either token0/tokenA or token1/tokenB. Otherwise all the calculation in those identified functions will be incorrect and those functions will return incorrect token amount which may cause more subsequent bugs.
MediumLack of input validation on `_fromToken` for funcFixed
Based on the function `_swap()` which called by `zapIn()`, the parameter `_fromToken` must be either token0 or token1 in `_pair`. Otherwise `_swap()` will not handle it properly and perform incorrect calculation.
MediumLack of input validation on `_swapFrom`Fixed
Based on the logic in `getPartialSwapInAmount()`, `_swapFrom` must be either `_tokenA` or `_tokenB`. Otherwise, the token swapping calculation is incorrect.
MediumLack of input validation on `_swapFrom`, `_token0Fixed
Based on the contract logic, `_swapFrom` must be either token0 or token1. And it is possible that parameter `_token0` is actually the token1 in pair; parameter `_token1` is actually the token0 in pair. So if those parameters are not checked properly, all the calculations in `getSwapInAmount()` are incorrect.
MediumLack of input validation on `_tokenA` and `_tokenFixed
Based on the logic in `_performUnproportionalZapIn()`, `_tokenA` and `_tokenB` must be different and must be token0 or token1 in `_pair`. Otherwise, the calculation and token swapping in `_performUnproportionalZapIn()` are incorrect.
MediumLack of input validation on `_tokenA` and `_tokenFixed
Based on functions `_performUnproportionalZapIn()` and `_partialSwap()` which are called by `unproportionalZapIn()`, `_tokenA` must be token0 and `_tokenB` must be token1 in `_pair` respectively. Otherwise `_partialSwap()` will perform incorrect calculation and malfunction.
MediumLack of input validation on `toToken`Fixed
Based on the logic in `getExpectedZapOut()` and `_swapTokens()`, the parameter `toToken` must be either token0 or token1 in pair `fromPoolAddress`. Otherwise, `_swapTokens()` is not consistent with `getExpectedZapOut()` and return values `swappedToken/swappedAmount` are incorrect because both token0 and token1 are swapped to `toToken`.
Show 10 more
MediumLack of input validation on `toToken` and `pair`Fixed
Based on function `getExpectedZapOut()` and `_swapTokens()`, the parameter `toToken` should be either token0 or token1 in `pair`. Otherwise, `_swapTokens()` will not work properly since the returned `swappedToken` can not be token0 and token1 at the same time.
MediumLack of Reentrancy Protection When Sending NativeFixed
The `finalFission()`, `restoreFusion()`, and `restoreFission()` functions in the `ReflectRouter` contract do not contain the `nonReentrant` modifier that several other functions have. The lack of the `nonReentrant` modifier, combined with the deviation from the "checks-effects-interactions" pattern in the related functions in the `ReflectVault` contract, could potentially allow the `from` address receive WEMIX coin multiple times that exceeds the `t.claim` or `t.fusedBalance` amount. As an example, the `finalFission()` function in the `ReflectRouter` contract calls the the `finalFission()` function in the `ReflectVault` contract. In the latter function, when `isDollarReturn` is `false` and `t.tokens[i] == WWEMIX`, `t.claim` amount of WEMIX coin is sent to the `from` address via a low level `call`. If the `from` address is a contract that has a `fallback()` or `receive()` function, it could potentially re-enters the original `finalFission()` function (if whitelisted or if it can trigger the whitelisted address to call the function) and receives `t.claim` amount of WEMIX coin again in line 335. Even if the `from` address is not whitelisted and cannot reenters the above functions, it still is best practice to adhere to the checks-effects-interactions pattern. Note that the "effect" is not executed till line 344-350 in the `finalFission()` function in the `ReflectVault` contract, after the "interaction" which is sending the WEMIX coin in line 335. The same pattern applies to the `restoreFusion()` and `restoreFission()` functions.
MediumLack of Storage Gap in Upgradeable ContractFixed
There is no storage gap preserved in the linked logic contracts. Any logic contract that acts as a base contract that needs to be inherited by other upgradeable children should have a reasonable size of storage gap preserved for the new state variable introduced by the future upgrades.
MediumLack of Storage Gap in Upgradeable ContractFixed
There is no storage gap preserved in the logic contract. Any logic contract that acts as a base contract that needs to be inherited by other upgradeable child should have a reasonable size of storage gap preserved for the new state variable introduced by the future upgrades.
MediumMinimum Quorum RequirementFixed
The minimum quorum requirement set in the `GovernanceSpell` contract for base policy is 0.01% of total governance token supply, and the minimum quorum for sub policy is 1% higher than the base policy. If a quorum is set to be too low, then the voting outcome can be potentially manipulated by a single user with a high voting power, if other voters are not actively voting during the voting window. This would potentially jeopardize the security of the governance system.
MediumMissing Refund The Dollar Payment LeftoversFixed
When invoking the `buyFanTokenWithDollars()` function to purchase FanTokens, the payment, represented by `_costMax`, is transferred from the buyer to the `fanTokenMarket` contract. The existing code has a flaw: it fails to refund any excess payment left over to the buyer. ```solidity=175 function buyFanTokenWithDollars(address _fanToken, uint _cost, uint _costMax, uint _amount) external onlyNotBlockedBuyToken(_fanToken) { //fanToken market require(IFanTokenFactory(fanTokenFactory).isRegisteredFanToken(_fanToken), "FTR : not registered fan token"); IERC20(WEMIXDollar).transferFrom(msg.sender, fanTokenMarket, _costMax); (uint usedDollar,uint concludedAmount) = IFanTokenMarket(fanTokenMarket).confirmTransferByWD(_fanToken, msg.sender, _cost, _costMax, _amount); emit FanTokenBuy(_fanToken, usedDollar, concludedAmount); } ``` The statements at L177-L179 calculate the actual cost of purchasing the exact amount of FanToken. While the actual cost might be less than the buyer's payment denoted by `cost_max`, there is currently no code logic in place to refund any remaining payment to the buyer. ```solidity=176 function confirmTransferByWD(address token, address user, uint cost, uint cost_max, uint amount) external override onlyRouter returns(uint, uint) { require(token.isContract(), "FTMarket: FanToken not CA"); require(user != address(0), "FTMarket: user zero"); require(cost > 0 && amount > 0, "FTMarket: cost, amount zero"); // check a user has enough wemixdollar(cost) for purchasing FanToken(amount or more than amount) uint actualAmount; uint actualCost; (actualAmount, actualCost) = getEstimatedCost(token, amount); // actual cost that user should pay for purchasing FanToken require(actualCost >= cost && actualCost <= cost_max && amount <= actualAmount, "FTMarket: invalid order"); // update FanToken ledger _updateLedger(token, amount); // get FanToken from vault uint vaultBalance = IERC20(token).balanceOf(fanTokenVault); require(vaultBalance >= amount, "FTMarket: FanTokenVault balance not enough"); // transfer FanToken to user, wemixDollar to creator uint marketBalance = IERC20(wemixDollar).balanceOf(address(this)); require(marketBalance >= actualCost, "FTMarket: FanTokenMarket balance not enough"); IERC20(token).safeTransferFrom(fanTokenVault, user, amount); address creator = _readLedger(token).creator; IERC20(wemixDollar).safeTransfer(creator, actualCost); emit TransferConfirmed(token, user, creator, actualCost, actualAmount, 0); return (actualCost, amount); } ```
MediumMissing updating states in the `insertSettleDexInFixed
The `insertSettleDexInfo()` function adds DEX settlement information manually. While it updates `_dv[ct].lastBalance[token]` in the `_getDexIncome()`, it does not update `_dv[ct].lastSettled[token]` to `amount`. Resulting in an incorrect settlement amount in the next settlement process.
MediumNo Minimum QuorumFixed
The modifier `WemixDollarTreasury.validRequirements` checks for `_quorum` to be in the range [0, `_ownerCount`], however not for a majority percentage. If the value of `_quorum` was 1, any user would be able to confirm and execute their transaction without consensus for any other `owners` address. Similarly, in the contract constructor, there is only a minimum required quorum of 2 users.
MediumNo Upper Limit in **setParams** functionFixed
There are no upper boundaries for **setParams** which is used to **set the "withdrawFee"**. It is possible to set the total fee rate up to any arbitrary amount.
MediumPotential cannot cancel the voteFixed
The modifier `whenNotPaused()` should be entered with the parameter `id` instead of constant '8'. If the DAO with id 8 is set to pause, the user cannot cancel voted items.
Show 10 more
MediumPotential Front-Running Risk in `sqrtPriceX96` OvFixed
The function `initialize()` can be invoked again if the initial pool price `slot0.sqrtPriceX96` is already set but the pool has no liquidity (both balance0() and balance1() are zero). This behavior presents a risk of front-running: an attacker who spots an incoming transaction that mints initial liquidity can preemptively front-run that transaction with an `initialize()` call to modify the initial pool price and the corresponding `tick`. ```solidity=367 require( slot0.sqrtPriceX96 == 0 || (balance0() == 0 && balance1() == 0), "AI" ); ``` This vulnerability will disrupt the pool initialization and the initial liquidity minting process.
MediumPotential Griefing Attack of User Fusion and FissFixed
The `fusionFrom()`, `finalFusion()`, `fissionFrom()`, `finalFission()`, `restoreFusion()`, `restoreFission()` functions of the `ReflectRouter` contract allows any whitelisted caller to provide any arbitrary `from` address and `userHash` as function argument. Furthermore, the `tempTxCompleted` and `txCompleted` mappings in the `ReflectVault` contract only takes the `userHash` as the mapping key. The methodology to generate `userHash` is unknown and is outside the scope of this audit. The check in the code base only checks if the `userHash` has been used, but does not decompose or otherwise verify the validity of the `userHash` being provided. Due to the lack of input verification, several griefing attack scenarios could be possible.
MediumPotential Imcompletely Withdraw in `zapOut`Fixed
A user could withdraw his LP token and receive amounts of `toToken` in function `zapOut` following this process: 1. First, Remove liquidity from the PlaySwapPair by the function `_removeLiquidity`. 2. Then, the tokens are swapped to `toToken` by the function `_swapTokens` 3. Finaly, `tokensRec` is transferred to user But, internal the function `_swapTokens`, contract PlaySwapZapOut could only process the swap when `toToken` is in the pair and set the`tokenRec`, otherwise, `tokensRec` is 0. Consider that there're many ERC20 token implemetes `_transfer` without revert then the transferring amount = 0, the `zapOut` would be executed without any token withdrew. ```solidity=124 //swap token0 to toToken if (token0 == toToken) { tokensBought = tokensBought.add(amount0); swappedToken = token1; swappedAmount = amount1; } else { //swap token using swap tokensBought = tokensBought.add(_token2Token(token0, toToken, amount0)); swappedToken = token0; swappedAmount = amount0; } //swap token1 to toToken if (token1 == toToken) { tokensBought = tokensBought.add(amount1); swappedToken = token0; swappedAmount = amount0; } else { //swap token using swap tokensBought = tokensBought.add(_token2Token(token1, toToken, amount1)); swappedToken = token1; swappedAmount = amount1; } ``` ## Impact A user zapOut with a toToken not in the PlaySwap pair and set minTokenRec as 0, he could not get the token received. The achieved tokens by removing liqulity will stay in the contract.
MediumPotential Reentrancy AttackFixed
A reentrancy attack can occur when the contract creates a function that makes an external call to another untrusted contract before resolving any effects. If the attacker can control the untrusted contract, they can make a recursive call back to the original function, repeating interactions that would have otherwise not run after the external call resolved the effects. ### External call(s) ```solidity=648 _transferFund(id, "Liquidate", tokenlist[i], user, userAmount); ``` ### State variables written after the call(s) ```solidity=651 IDAOToken(dtAddress).transferFrom(user, address(this), userDTAmount); IDAOToken(dtAddress).transfer(DEAD_WALLET, userDTAmount); ```
MediumPotential replay attack on function `tradeSeed()`Fixed
The signature in the parameter does not cover anti-replay data like nonce and deadline. And the function `_tradeSeed()` may accept the same parameters `_serialSeed` and `_serialCurrency` multiple times since the corresponding seed or currency may not be closed.
MediumPotential Sandwich AttacksFixed
A sandwich attack may happen when an attacker observes a transaction swapping tokens or adding liquidity without setting restrictions on slippage or minimum output amount. The attacker can manipulate the exchange rate by frontrunning (executing before the target) a transaction to purchase one of the assets and make profits by backrunning (executing after the target) a transaction to sell the asset. The function `addLiquidity()` is called without setting restrictions on slippage or minimum output amount, so transactions triggering these functions are vulnerable to sandwich attacks, especially when the input amount is large.
MediumRisk of Supporting Arbitrary TokensFixed
One of the most dangerous behaviors in smart contracts is to perform an external call to an untrusted contract. An attacker can deploy a contract and implement the contract whatever they want. When the `ReflectRouterV2` contract performs the external call, the attacker-defined code will be executed.
MediumSafeTransfer should be used in place of transferFixed
SafeTransfer should be used in place of Transfer for Solidity contracts to ensure robust security and error handling. Unlike the basic Transfer function, SafeTransfer incorporates safeguards against potential smart contract vulnerabilities, such as unexpected token loss. By automatically validating the recipient's ability to receive tokens and reverting transactions in case of failures.
MediumSigned content is not encoded properlyFixed
Based on EIP\-712 standard, the parameter `extraData` of type `bytes[]` must be encoded before signing. See https://eips.ethereum.org/EIPS/eip-712#definition-of-encodedata
MediumThe `totalClaimFee` Is Ignored in The Total RewarFixed
In the `SPMineRevenue` contract, the value of the variable `_totalRevenue` is calculated by the expression `currentBalance.add(totalClaimedAmount)`, where the `totalClaimedAmount` is the total amount of rewards claimed by users, which does not contain claim fees. The total claim fee is stored in the variable `totalClaimFee`. Therefore, the variable `_totalRevenue` cannot represent the total amount of minted reward tokens. Consequently, the calculated `K` is less than expected and users receive fewer rewards. In the `SPBase` contract, the `remainRT()` function is designed to retrieve the total amount of remaining reward tokens. It subtracts `totalClaimedAmount` from `totalMinedRT`, but does not subtract `totalClaimFee` and therefore returns an incorrect number.
Show 10 more
MediumToken transfer return value is not checkedFixed
```solidity=36 try _token.transfer(_to, _amount) returns (bool) { return (true, ""); } catch Error(string memory _reason) { return (false, bytes(_reason)); } catch Panic(uint _errorCode) { return (false, abi.encodePacked(_errorCode)); } catch (bytes memory _lowLevelData) { return (false, _lowLevelData); } ``` The return value of token transfer in some token contracts may return false to indicate failure of the transfer. So it is possible that `_give()` returns true even if token transfer fails. Consequently, the contract might incorrectly believe the tokens have been successfully transferred, leading to erroneous bridging out of tokens.
MediumUnable to Pause ContractFixed
The `PoET` contract inherits the `PausableUpgradeable` contract, and the `claim()` function requires that the contract is not paused. However, the contract does not contain a public or external function to pause / unpause the contract, rendering the `whenNotPaused` modifier a no-op that always passes.
MediumUpdates of `userLastClaimBlockNumber` and `_usersFixed
The update of the `userLastClaimBlockNumber` variable fails to consider that an NFT representing a position might have been transferred from one user to another. Specifically, if User A transfers the position NFT to User B, the `userLastClaimBlockNumber[UserB]` will be initialized to 0, which does not reflect the actual last claim block number that should have been carried over from `userLastClaimBlockNumber[UserA]`. ```solidity=648 if (tokensOwed0 == 0 && tokensOwed1 == 0) { position.collectBlockNumber = block.number; userLastClaimBlockNumber[recipient] = 0; } else { userLastClaimBlockNumber[recipient] = block.number; } ``` Besides, the current design of `userLastClaimBlockNumber` does not consider the case when a recipient has multiple position NFTs. Let's say user A holds two positions: NFT1 for pool M and NFT2 for pool N. Then he calls the `collect()` function for both positions. Collecting owed tokens for NFT2 will override the `userLastClaimBlockNumber` timestamp of NFT1. Similarly, the current contract logic for updating `_users` and `_poolUsers` mappings only considers scenarios where tokens are minted but don't handle manual transfers between accounts. For example, if User A mints an NFT and then manually transfers it to User B, the mappings will still consider User A as the user, leading to inconsistencies and potentially incorrect state management. ```solidity=334 if (!_users[params.recipient]) { _users[params.recipient] = true; totalUsers += 1; } if (!_poolUsers[poolId][params.recipient]) { _poolUsers[poolId][params.recipient] = true; totalPoolUsers[poolId] += 1; } ```
MediumWithdraw doubled value from swap vaultFixed
```solidity=363 (uint amountA, uint value) = IPlaySwapRouter(psr).removeLiquidity( tokenA, wemixD, amount, 0, 0, _tv[SettleType.SWAP].account, _deadline() ); value = value.add(getValueOut(tokenA, amountA)); require(IERC20(tokenA).transferFrom(_tv[SettleType.SWAP].account, address(this), amountA)); require(IERC20(wemixD).transferFrom(_tv[SettleType.SWAP].account, address(this), value)); ``` The `value` represents the amount of `wemixD` received after removing liquidity. It adds the value of `tokenA` to `value`, and then attempts to withdraw the doubled `value` from the vault, causing the transaction to fail.
MediumWrong Function Call in Function `getAmountIn`Fixed
The function `getAmountIn` should give required input amount depending on the output amount. However contract calls wrong function which is `getAmountOut`. ```solidity=375 function getAmountIn(uint amountOut, uint reserveIn, uint reserveOut) public pure override returns (uint amountIn) { return PlaySwapLibrary.getAmountOut(amountOut, reserveIn, reserveOut); } ```
LowChecks Effects Interactions Pattern Not UsedWemix Retainer 150 WEMEME · CertiK · 13 Feb 2025Fixed
The `buy`, `sell`, and `complete` functions of the `WEMEME` contract do not follow the checks-effects-interactions pattern, as token transfers including native token transfers take place before the relevant state variables are updated. In normal circumstances, these functions are called via the `Router` contract which contains the `nonReentrant` modifier, which offers the necessary reentrancy protection. However, the `router` address could theoretically be changed via the `WEMEMEpolicy` contract. If a new `router` contract does not contain reentrancy protection, the deviation from the checks-effects-interactions pattern could become a vulnerability. For example, the creator fee recipient could potentially reenter the `complete` function and receives fee multiple times.
Open the reportLowEvent Not IndexedWemix Retainer 150 WEMEME · CertiK · 13 Feb 2025Fixed
If an event is not indexed in a smart contract, it means that the event's parameters are not tagged with the `indexed` keyword. This has implications for how the event data can be searched and filtered when looking through blockchain logs. Without indexing, the event will still emit the data as part of the transaction log, but users won't be able to query for these events using the parameters. They'll have to retrieve the entire set of logs and manually sift through them to find events with the specific data. This can be less efficient and more time-consuming, especially on a blockchain with a high volume of transactions and events.
Open the reportLowMissing Emit EventsWemix Retainer 150 WEMEME · CertiK · 13 Feb 2025Fixed
There should always be events emitted in the sensitive functions that are controlled by centralization roles.
Open the reportLowMissing Input ValidationsWemix Retainer 150 WEMEME · CertiK · 13 Feb 2025Fixed
The variables `wemixThreshold`, `listingFee`, and `creatorIncentive` should satisfy the condition that `wemixThreshold >= listingFee >= creatorIncentive` (or `>`). However, only the `setCreatorIncentive()` function contains input validation, and the `setListingFee()` and `setWemixThreshold()` functions do not. If these parameters are not set correctly, the `_complete()` function of `WEMEME` could potentially always revert due to underflow.
Open the reportLowMultiple Inheritance of Ownable ContractWemix Retainer 150 WEMEME · CertiK · 13 Feb 2025Fixed
The `WEMEME` contract inherits both `Ownable` and `WEMEMEBase` contracts. The `WEMEMEBase` contract inherits `WEMEMERole` contract which also inherits `Ownable` contract.
Open the reportShow 10 more
LowPotentially Stuck Native TokensWemix Retainer 150 WEMEME · CertiK · 13 Feb 2025Fixed
The `WEMEME` and `WEMEMERouter` contracts contain payable `receive()` and `fallback()` functions, but other than fee payment and liquidity provision which have their own accounting records, no other mechanism to retrieve native tokens sent to the contract.
Open the reportLowConcerns about `PlayNFTMarketOracle`Wemix Retainer 123 PlayNftMarket · CertiK · 16 Dec 2024Fixed
If `priceOracle` is not a zero address, the `offerBuyOrder()` function will store NFT price of the completed order. However, the `claimOrder()` function, which marks the `OFFER_BIDDING` order as completed, does not save the NFT price to the `priceOracle` contract.
Open the reportLowInconsistent usage of `_registeredOnOfferBidding`Wemix Retainer 123 PlayNftMarket · CertiK · 16 Dec 2024Fixed
The `_registeredOnOfferBidding` mapping tracks `OFFER_BIDDING` orders and prevents the registration of other orders for the same NFT when it is registered for an `OFFER_BIDDING` order. However, there are inconsistent issues regarding the usage of this mapping: - The mapping also tracks `SELL` and `AUCTION` orders. - Provide offer for `SELL` orders will not remove the order ID from `_registeredOnOfferBidding` mapping. - Claim order for `AUCTION` orders will not remove the order ID from `_registeredOnOfferBidding` mapping.
Open the reportLowRisk of Losing Admin Access If The Input New AdmiWemix Retainer 141 GameServerNFT · CertiK · 15 Nov 2024Fixed
If the address `newSuperAdmin` to be set as the admin role is the same as the current admin `msg.sender`, it will result in the admin role losing its administration authorization over the contract. ```solidity=100 function changeSuperAdmin(address newSuperAdmin) external { require(newSuperAdmin != address(0), "GameServerNFT: New super admin cannot be the zero address"); require(isSuperAdmin(msg.sender), "GameServerNFT: Invalid super admin or unauthorized"); grantRole(GAME_SERVER_SUPER_ADMIN, newSuperAdmin); revokeRole(GAME_SERVER_SUPER_ADMIN, msg.sender); emit SuperAdminRoleChanged(msg.sender, newSuperAdmin); } ``` This vulnerability arises because the function first grants the admin role to the new owner's address and then revokes the admin role from the current admin's address. However, if the new owner's address is the same as the current one without any proper validation, the admin role will be revoked prematurely, leading to the admin losing the authorization for contract administration.
Open the reportLowIncorrect `getLastIndex()` Return Value When `hspWemix Retainer 137 HSPFE · CertiK · 31 Oct 2024Fixed
In the `getLastIndex()` function, the value of `index` is not assigned when `hspCount == 0` is `false`, and as a result would return `0` when the correct return value is greater than 0.
Open the reportLowLack of enforcement that `st` and `rt` tokens areWemix Retainer 137 HSPFE · CertiK · 31 Oct 2024Fixed
In the `deploy()` function of `SPFactory`, there's no enforcement that the `goddra` and `hydra` token addresses are different. If they are the same, and insufficient `goddra` token is provided by the project team as the `rt` (reward) token, then it is possible that then user A's staked token might be paid out as reward to user B when user B claims reward, and user A would be unable to unstake its tokens due to insufficient token balance.
Open the reportLowMissing Emit EventsWemix Retainer 137 HSPFE · CertiK · 31 Oct 2024Fixed
There should always be events emitted in the sensitive functions that are controlled by centralization roles.
Open the reportLowUnderscore Prefix For Non-External VariablesWemix Retainer 137 HSPFE · CertiK · 31 Oct 2024Fixed
The current contract doesn't follow the naming convention specified by [Solidity DOC](https://docs.soliditylang.org/en/v0.8.20/style-guide.html#underscore-prefix-for-non-external-functions-and-variables): The state variable `variable` is used as `private` or `internal` and is not exposed publicly. It should have `an underscore prefix` like `_varaible`. Leading underscores allow you to immediately recognize the intent of such functions, but more importantly, if you change a function from non-external to external (including public) and rename it accordingly, this forces you to review every call site while renaming. This can be an important manual check against unintended external functions and a common source of security vulnerabilities (avoid find-replace-all tooling for this change).
Open the reportLowIncorrect Code Comment for unexecutable Mapping iWemix Retainer 125 WemixPayment Contract · CertiK · 16 Oct 2024Fixed
The code comments for the mapping `unexecutable` states true stands for support, false stands for unsupport. However, in the modifier `onlyServiceSupport`, in case unexecutable returns true, the modifier will revert a message `NotSupport`, which indicates the code comments is incorrect. The code comments associated with the `unexecutable` mapping indicate that a value of true means support, while a value of false indicates no support. However, this documentation is misleading in the context of the `onlyServiceSupport` modifier. The modifier is designed to revert with the message NotSupport if `unexecutable` returns true, which contradicts the provided comments.
Open the reportLowMissing Sanity Check in `_initCore` for `_router`Wemix Retainer 125 WemixPayment Contract · CertiK · 16 Oct 2024Fixed
In the contract, the `setRouter` function includes a check to ensure that the provided router parameter is a valid contract address before proceeding. This check is crucial for maintaining the integrity and security of the contract. However, in the `_initCore` function, the internal `_setRouter` function is called directly with the `_router` parameter without any sanity checks to confirm that it is a valid contract address. This oversight could potentially allow an invalid or malicious address to be set as the router, leading to unintended behavior or vulnerabilities within the contract.
Open the reportShow 10 more
LowMissing Validation for Token PathWemix Retainer 125 WemixPayment Contract · CertiK · 16 Oct 2024Fixed
The `swapPay()` function is designed to swap a token from the provided path[0] to payInfo.token, indicating that the source token should not be the same as the destination token (payInfo.token). However, the function currently lacks validation to ensure that path[0] is not equal to payInfo.token. Without this check, if path[0] is accidentally set to payInfo.token, the swap operation could lead to an unintended outcome where no effective token swap occurs, or worse, introduce inefficiencies or unnecessary operations in the contract logic. This missing validation can also introduce security risks if not caught during execution.
Open the reportLowError Message Mismatch in `withdraw` FunctionWemix Retainer 129 PlayTokenExchanger · CertiK · 9 Sep 2024Fixed
In the `withdraw` function, an internal call to the mint function of the `playToken` contract is made to process asset withdrawals. When the mint function call fails, the error message returned is `PlayTokenExchangerByMint: burn fail`. This message is misleading as it suggests a failure in a `burn` operation, which is inconsistent with the actual `mint` function being called. ```solidity=13 (bool status,) = playToken.call(abi.encodeWithSelector(MINT_FUNC_SELECTOR, msg.sender, amount)); require(status, "PlayTokenExchangerByMint: burn fail"); ``` ```solidity=22 (bool status,) = playToken.call(abi.encodeWithSelector(MINT_FUNC_SELECTOR, msg.sender, amount)); require(status, "PlayTokenExchangerByMint: burn fail"); ```
Open the reportLowInconsistent Code Logic and CommentsWemix Retainer 133 PricePolicy · CertiK · 9 Sep 2024Fixed
The `checkMintable()` function contains a discrepancy between its code logic and the accompanying comments. The comment suggests that the function checks if "the timestamp of the TWAP information is more than the current timestamp." However, the actual implementation calls the `isMintableTime()` function, which verifies if the current timestamp is greater than the cached TWAP price timestamp: ```solidity=152 function isMintableTime(uint start) public view returns (bool) { return block.timestamp > start; } ```
Open the reportLow`cachedPrice[gameTokenMinter]` not cleared when rWemix Retainer 131 NightCrowTWAPV3 · CertiK · 22 Aug 2024Fixed
Function `removeGameToken` is used for removing tokens from all mappings. However, mapping `cachedPrice` is not cleared. Note that the cached price cannot be fetched using getter functions implemented in the contract, but it can still be fetched by directly accessing the mapping `cachedPrice`.
Open the reportLowPotential issue after changing game token minterWemix Retainer 131 NightCrowTWAPV3 · CertiK · 22 Aug 2024Fixed
After the editor calls `changeGameTokenMinter`, mapping `lockPrice` and `freezePeriod` remains unchanged. This will cause two potential issues: 1. The new minter may not have record in mapping `lockPrice` and `freezePeriod`. 2. `lockPrice` and `freezePeriod` value for the previous minter cannot be removed, as `removeGameToken` cannot fetch the minter via `tokenToTwap[gameToken]`. The first issue can be mitigated by calling `changeTargetInfo` subsequently, but this requires off-chain operations that falls out of the scope of the audit.
Open the reportLowHardcoded Fee AmountWemix Retainer 74 (DIOSV3) · CertiK · 16 Aug 2024Fixed
The smart contract has hardcoded the fee amount as `WEMIX_FEE_AMOUNT = 0.08 ether`. This design choice lacks flexibility and does not allow for dynamic adjustments based on changing network conditions or economic factors. Furthermore, the constant nature of this variable implies that once the smart contract is deployed, this fee cannot be changed. ```solidity=26 // The fixed fee amount specific to WEMIX, set at 0.08 ether. uint256 public constant override WEMIX_FEE_AMOUNT = 0.08 ether; ```
Open the reportLowMissing Zero Address ValidationWemix Retainer 74 (DIOSV3) · CertiK · 16 Aug 2024Fixed
Addresses are not validated before assignment or external calls, potentially allowing the use of zero addresses and leading to unexpected behavior or vulnerabilities. For example, transferring `operatorManager` to a zero address can result in a permanent loss of access to the privileged functions. ```solidity=66 operatorManager = _newOperatorManager; ``` - `_newOperatorManager` is not zero-checked before being used.
Open the reportLowUnnecessary call of `_distributeReward()` when reWemix Retainer 74 (DIOSV3) · CertiK · 16 Aug 2024Fixed
The function `_distributeReward()` is unconditionally called even when rewardAmount is explicitly set to zero. The scenario under which this occurs is when `wemixDfeeAmount` is greater than the difference between `finalWemixDBalance` and `initialWemixDBalance`. In such a case, the variable rewardAmount is explicitly set to zero, but the function `_distributeReward(rewardAmount)` is still executed. Every unnecessary call to `_distributeReward()` consumes more gas than needed, leading to a financial cost for the callers. ```solidity=212 uint256 rewardAmount; if (wemixDfeeAmount > finalWemixDBalance - initialWemixDBalance) { _transferFee(finalWemixDBalance - initialWemixDBalance, path); rewardAmount = 0; } else { _transferFee(wemixDfeeAmount, path); rewardAmount = finalWemixDBalance - initialWemixDBalance - wemixDfeeAmount; } _distributeReward(rewardAmount); ```
Open the reportLow`path`, `amountInMax` and `charge` arguments areWemix Retainer 122 TokenPayment · CertiK · 13 Aug 2024Fixed
The `abi.encodePacked()` function encodes the given arguments into bytes using tightly packed, contiguous encoding. However, the arguments `path`, `amountInMax` and `charge` are not included as part of the signature message, leading to insufficient checks on these arguments. Additionally, the `path` should be checked to ensure its length is greater than 1.
Open the reportLowRedundant CommentWemix Retainer 122 TokenPayment · CertiK · 13 Aug 2024Fixed
The comments from lines 110 to 120 are the same as those from lines 121 to 131. ```solidity=110 /** * @dev External function to pay token * Allow the user to pay for the product by spending the token they have * @param token address of token contract be sepend * @param service service name of registered platform * @param product product for payment * @param column column for payment * @param user user address * @param amount product price * @param refundable status of refund (true: refundable, false: non-refundable) */ /** * @dev External function to pay token * Allow the user to pay for the product by spending the token they have * @param token address of token contract be sepend * @param service service name of registered platform * @param product product for payment * @param column column for payment * @param user user address * @param amount product price * @param refundable status of refund (true: refundable, false: non-refundable) */ ```
Open the reportShow 10 more
LowSafeTransfer should be used in place of transferWemix Retainer 122 TokenPayment · CertiK · 13 Aug 2024Fixed
SafeTransfer should be used in place of Transfer for Solidity contracts to ensure robust security and error handling. Unlike the basic Transfer function, SafeTransfer incorporates safeguards against potential smart contract vulnerabilities, such as unexpected token loss. By automatically validating the recipient's ability to receive tokens and reverting transactions in case of failures.
Open the reportLowModifier Sequence OptimizationWemix Retainer 121 wepublicBank · CertiK · 2 Aug 2024Fixed
The sequence of checks within the `cancelProject` and `settleProject` function can be optimized to save gas. Specifically, the check for `msg.sender` should be arranged before the signature validation. This is because the `msg.sender` check is simpler and less computationally intensive compared to the signature validation. By performing this check first, unnecessary computationally expensive operations can be avoided if the `msg.sender` check fails. ```solidity=560 require( RbacSignatureValidator.isValidSignature( wepublicPoint, keccak256(packedData), signature, CANCEL_FUNDING_PROJECT_ROLE ), "FundingProjectTreasury: invalid signature" ); require( projectTreasury == address(this), "FundingProjectTreasury: invalid project treasury" ); require( msg.sender == projectOwner, "FundingProjectTreasury: invalid project owner" ); ```
Open the reportLowSender does not transfer any tokens to the treasuWemix Retainer 121 wepublicBank · CertiK · 2 Aug 2024Fixed
In the function `fund()`, if the current type is `Wemix`, the sender will pay the native token to this contract and then transfer it to the project owner. However, if the current type is `WepublicPoint`, the `wepublicPoint` contract mints tokens to this contract but does not reduce any tokens from the sender. This could potentially allow users to fund projects without actually spending their own tokens, leading to "funding out of thin air." ```solidity=361 if (targetCurrencyType == CurrencyType.Wemix) { require( msg.value == fundAmount, "FundingProjectTreasury: invalid msg.value for Wemix" ); payable(projectOwner).transfer(fundAmount); } else if (targetCurrencyType == CurrencyType.WepublicPoint) { require( msg.value == 0, "FundingProjectTreasury: invalid msg.value for WP" ); IWepublicPointV2(wepublicPoint).mint(fundAmount); ```
Open the reportLowState Variables `rewardQuantityOfFunder` and `_reWemix Retainer 121 wepublicBank · CertiK · 2 Aug 2024Fixed
In the function `_cancelProject()`, the `fundedAmounts` of the funders and `totalLockedBalance` are updated. However, the `rewardQuantityOfFunder` and `_rewardInfos` are not updated. If the `REWARD_DISTRIBUTOR_ROLE` role mistakenly distributes the removed funders, they can still receive the NFT reward even though their fund amount is 0. ```solidity=929 cancelAmounts[i] = fundedAmount; accumulatedBalance += fundedAmount; _totalLockedBalance -= fundedAmount; fundedAmounts[funder] = 0; //@audit: `rewardQuantityOfFunder` and `_rewardInfos` not updated here ``` Additionally, the function `distributeReward` should decrease the `_rewardInfos[rewardId].totalQuantity`. ```solidity=714 for (uint i = 0; i < infos.length;) { DistributeInfo memory info = infos[i]; --rewardQuantityOfFunder[info.to][rewardId]; //@audit: _rewardInfos[rewardId].totalQuantity not updated here IERC721Upgradeable(_reward.tokenAddress).safeTransferFrom(vault, info.to, info.tokenId); unchecked { ++i; } } ```
Open the reportLowInconsistency Between Code and CommentWemix Retainer 119 DIOSV3 (2) · CertiK · 8 Jul 2024Fixed
The comment in line 21 says "The fixed fee amount specific to WEMIX, set at 0.8 ether.". However the fee amount was hardcoded as 0.08 ether in line 22. ```solidity=21 // The fixed fee amount specific to WEMIX, set at 0.8 ether. uint256 public constant WEMIX_FEE_AMOUNT = 0.08 ether; ``` Besides, the smart contract has hardcoded the fee amount as 0.08 ether. This design choice lacks flexibility and does not allow for dynamic adjustments based on changing network conditions or economic factors.
Open the reportLowUnnecessary call of `_distributeReward()` when reWemix Retainer 119 DIOSV3 (2) · CertiK · 8 Jul 2024Fixed
The function `_distributeReward()` is unconditionally called even when rewardAmount is explicitly set to zero. The scenario under which this occurs is when `wemixDfeeAmount` is greater than the difference between `finalWemixDBalance` and `initialWemixDBalance`. In such a case, the variable rewardAmount is explicitly set to zero, but the function `_distributeReward(rewardAmount)` is still executed. Every unnecessary call to `_distributeReward()` consumes more gas than needed, leading to a financial cost for the callers. ```solidity=180 if(msg.sender.balance < WEMIX_FEE_AMOUNT){ address[] memory path = new address[](2); path[0] = WEMIXD; path[1] = WWEMIX; uint256 wemixDfeeAmount = weswapRouter.getAmountsIn( WEMIX_FEE_AMOUNT, path )[0]; if (wemixDfeeAmount > rewardAmount) { _transferFee(rewardAmount, path); rewardAmount = 0; } else { _transferFee(wemixDfeeAmount, path); rewardAmount = rewardAmount - wemixDfeeAmount; } } _distributeReward(rewardAmount); ```
Open the reportLow`totalLockedWemix` Not Reset to Zero After ProjecWemix Retainer 117 SupportProjectTreasuryV2 · CertiK · 28 May 2024Fixed
In the `settleProject()` function, the project is settled by transferring the `totalLockedWepublicPoint` amount to the project owner and setting its value to zero. However, the state variable `totalLockedWemix` is not reset to zero after the project is settled. This can lead to confusion, as the view function `totalLockedWemix()` still returns the total supported WEMIX amount, even though the project has been settled and all WEMIX coins have already been transferred to the project owner.
Open the reportLowThe signer role for `deactivateProject()` probablWemix Retainer 117 SupportProjectTreasuryV2 · CertiK · 28 May 2024Fixed
There is an inconsistency between the comment and the actual implementation in the `deactivateProject()` function. The comment indicates that the signed account must possess the `DEACTIVATE_SUPPORT_PROJECT` role. However, the implemented code checks for the `CANCEL_SUPPORT_PROJECT_ROLE` instead. This discrepancy can lead to confusion about the required permissions for signing signatures for the `deactivateProject()` function. There is no role defined as `DEACTIVATE_SUPPORT_PROJECT` within the contract.
Open the reportLowUpgradeable Contracts Should Not Inherit Non-UpgrWemix Retainer 117 SupportProjectTreasuryV2 · CertiK · 28 May 2024Fixed
All upgradeable contracts in an inheritance chain must follow the same rules for writing upgradeable contracts: One should avoid using non-upgradeable contracts as base contracts in an upgradeable contract. However, the current contract inherits a non-upgradeable base contract `ReentrancyGuard`, while an upgradeable equivalent is available in OpenZeppelin's `contracts-upgradeable` repository. Reference: - https://docs.openzeppelin.com/upgrades-plugins/1.x/writing-upgradeable#use-upgradeable-libraries.
Open the reportLowConstant token IdWemix Retainer 111 Wonder Staking · CertiK · 15 Apr 2024Fixed
In the functions `emergencyTransfer()` and `emergencyChange()`, the `tokenId` is constant. This implies that these functions are specifically designed for particular token IDs. If the token ID changes, these functions become obsolete. ```solidity=158 uint256 tokenId = 1334; ``` ```solidity=162 tokenId = 1335; ``` ```solidity=170 uint256 tokenId = 1373; ```
Open the reportShow 10 more
LowFunction `setRewardFeeRatio()` can be invoked witWemix Retainer 111 Wonder Staking · CertiK · 15 Apr 2024Fixed
If the owner of the rewarder wants to set the reward fee ratio, they must first call the function `setRewardFeeRatioRequest()` to create a request. After waiting for `feeRatioRequestDelay` blocks, they can call the function `setRewardFeeRatio()` to update the fee ratio of the pool and delete the request. However, the current inspection conditions are not stringent enough, allowing the owner to set the fee ratio to 0 for the specified pool, thereby bypassing critical checks. ```solidity=415 require( feeRatioRequests[pid].requestBlockNumber <= block.number, "STAKING: FeeRatio request is not ready." ); require( feeRatioRequests[pid].ratio >= 0 && feeRatioRequests[pid].ratio <= 10000, "fee Ratio is invalid" ); ```
Open the reportLowMissing Zero Address ValidationWemix Retainer 111 Wonder Staking · CertiK · 15 Apr 2024Fixed
Addresses are not validated before assignment or external calls, potentially allowing the use of zero addresses and leading to unexpected behavior or vulnerabilities. For example, transferring tokens to a zero address can result in a permanent loss of those tokens. ```solidity=131 registry = _registry; ``` - `_registry` is not zero-checked before being used. --- ```solidity=135 platformFeeCollector = _platformFeeCollector; ``` - `_platformFeeCollector` is not zero-checked before being used. --- ```solidity=45 govStaking = _govStaking; ``` - `_govStaking` is not zero-checked before being used. --- ```solidity=46 staking = _staking; ``` - `_staking` is not zero-checked before being used. --- ```solidity=49 ncp = _ncp; ``` - `_ncp` is not zero-checked before being used. --- ```solidity=68 to.transfer(rewardValue); ``` - `to` is not zero-checked before being used. --- ```solidity=69 feeCollector.transfer(rewardFee); ``` - `feeCollector` is not zero-checked before being used. --- ```solidity=70 platform.transfer(platformFee); ``` - `platform` is not zero-checked before being used. --- ```solidity=126 ncp = _ncp; ``` - `_ncp` is not zero-checked before being used.
Open the reportLowParameter `to` is unusedWemix Retainer 111 Wonder Staking · CertiK · 15 Apr 2024Fixed
In the function `withdraw()`, the parameter `to` is intended to receive tokens. However, the withdrawal amount is directly transferred to the `msg.sender` instead of using the `to` parameter. ```solidity=1215 payable(address((msg.sender))).sendValue(withdrawalRequestInfo.amount); ```
Open the reportLowPotential Divide by ZeroWemix Retainer 111 Wonder Staking · CertiK · 15 Apr 2024Fixed
Performing division by zero would raise an error and revert the transaction. ```solidity=901 uint256 reductionMP = (mpInfo.staked * amount) / user.amount; ``` The expression `(mpInfo.staked * amount) / user.amount` may divide by zero. Its divisor has has estimated interval [-115792089237316195423570985008687907853269984665640564039457584007913129639935, 231584178474632390847141970017375815706539969331281128078915168015826259279870]. --- ```solidity=1089 uint256 reductionMP = (mpInfo.staked * amount) / user.amount; ``` The expression `(mpInfo.staked * amount) / user.amount` may divide by zero. Its divisor has has estimated interval [-115792089237316195423570985008687907853269984665640564039457584007913129639935, 231584178474632390847141970017375815706539969331281128078915168015826259279870].
Open the reportLowThe different addresses map to the same rewarderWemix Retainer 111 Wonder Staking · CertiK · 15 Apr 2024Fixed
In the function `initialize()`, the `ncp` address is mapped to the `rewarderCount` `1`. when the owner calls the function `addRewarder()`, the `rewarderCount` increases by 1, and the new `_rewarder` maps to the new `rewarderCount`. However, the function `setNCP()` directly updates the `rewarderCount` to the `ncp` address. This means that the `ncp` address and the last `_rewarder` have the same `rewarderCount` because the `rewarderCount` does not increase by 1. Additionally, the `rewarderCount` of old `ncp` is not updated to 0. ```solidity=125 function setNCP(address _ncp) external onlyOwner { ncp = _ncp; isRewarder[ncp] = rewarderCount; } ```
Open the reportLowThe function `_updateMP()` is not invoked beforeWemix Retainer 111 Wonder Staking · CertiK · 15 Apr 2024Fixed
The calculation of the `rewardDebt` relies on the `user.amount` and `mpInfo.staked`. However, the function `claim()` does not invoke the function `_updateMP()` to update the `mpInfo.staked` before updating the `rewardDebt`, resulting in the `rewardDebt` not being updated in time. ```solidity user.rewardDebt = ((user.amount + mpInfo.staked) * pool.accRewardPerShare) / ACC_REWARD_PRECISION; ```
Open the reportLowUsage of `transfer`/`send` for Sending Native TokWemix Retainer 111 Wonder Staking · CertiK · 15 Apr 2024Fixed
It is not recommended to use Solidity's `transfer()` and `send()` functions for transferring native tokens, since some contracts may not be able to receive the funds. Those functions forward only a fixed amount of gas (2300 specifically) and the receiving contracts may run out of gas before finishing the transfer. Also, EVM instructions' gas costs may increase in the future. Thus, some contracts that can receive now may stop working in the future due to the gas limitation. ```solidity=68 to.transfer(rewardValue); ``` - `Rewarder.onReward` uses `transfer()`.
Open the reportLowWithdraw requests that have not reached the claimWemix Retainer 111 Wonder Staking · CertiK · 15 Apr 2024Fixed
The users call the function `withdrawRequest()` to request withdrawal from the given pool and the claimable time is updated to the current block number plus the unbound time. In the function `withdraw()`, if the `tokenId` is 0, it is updated to the first withdrawable token ID of the user. However, if the `tokenId` is not 0 and the withdrawal request is obtained from the function `getWithdrawalRequestInfo()`, there is no check to verify if the claimable time of the request is less than or equal to the current block number. Therefore, the withdraw requests that have not reached the claimable time can be withdrawn.
Open the reportLowMissing Check staking amount of this contractWemix Retainer 113 Liquid Staking · CertiK · 15 Apr 2024Fixed
In the function `wonderMigrate()`, there is no check to ensure that the staking amount of this contract is greater than 0. If the owner has already invoked the function `wonderMigrate()` to withdraw all deposits but did not execute the function `wonderUpdate()`, then calling the function `wonderMigrate()` again will override the `pendingTokenId`, `pendingStaking`, `pendingPid` and `pendingMigrationAmount` if the `withdrawRequest()` can receive a 0 amount. As a result, the owner will not be able to withdraw the contract's deposits.
Open the reportLowMissing Zero Address ValidationWemix Retainer 113 Liquid Staking · CertiK · 15 Apr 2024Fixed
Addresses are not validated before assignment or external calls, potentially allowing the use of zero addresses and leading to unexpected behavior or vulnerabilities. For example, transferring tokens to a zero address can result in a permanent loss of those tokens. ```solidity=64 staking = wonder_; ``` - `wonder_` is not zero-checked before being used. --- ```solidity=105 pendingStaking = wonder; ``` - `wonder` is not zero-checked before being used.
Open the reportShow 10 more
LowRedundant CodeWemix Retainer 113 Liquid Staking · CertiK · 15 Apr 2024Fixed
The logic from lines 56 to 65 is redundant because it has already been implemented in the function `_phaseShift()`. ```solidity INCPStaking(wonder_).deposit{value: address(this).balance}( wonderPid_, address(this).balance, payable(address(this)), false, false ); staking = wonder_; pid = wonderPid_; ```
Open the reportLowThe `withdraw()` function does not match the oneWemix Retainer 113 Liquid Staking · CertiK · 15 Apr 2024Fixed
The Interface `withdraw` defines four parameters: `pid`, `amount`, `to` and `claimReward`. However, the `withdraw()` function in the `staking` contract only accepts three parameters: `pid`, `tokenId` and `to`. This mismatch in function parameters can lead to invocation failures. ```solidity=79 IStaking(staking).withdraw(pid, 0, payable(address(this)), true); ```
Open the reportLowCheck Effect Interaction Pattern ViolatedWemix Retainer 103 KLEVA Omni · CertiK · 2 Apr 2024Fixed
A reentrancy attack can occur when the contract creates a function that makes an external call to another untrusted contract before resolving any effects. If the attacker can control the untrusted contract, they can make a recursive call back to the original function, repeating interactions that would have otherwise not run after the external call resolved the effects.
Open the reportLowDeprecated Usage Of `_setupRole()`Wemix Retainer 103 KLEVA Omni · CertiK · 2 Apr 2024Fixed
As mentioned above the function in Openzeppelin's contract is `AccessControl`. The function `_setupRole()` is deprecated in favor of `_grantRole`. The contract `KlevaConfig` is using the deprecated `_setupRole()` function. Reference: [OpenZeppelin Documentation](https://docs.openzeppelin.com/contracts/4.x/api/access#AccessControl-_setupRole-bytes32-address-)
Open the reportLowfunction may return true even if nothing is execuWemix Retainer 103 KLEVA Omni · CertiK · 2 Apr 2024Fixed
Low level call on EOA or non-existing address will always succeed even if no smart contract code is executed at all. If parameter is EOA or non-existing address, the function will malfunction: it returns true even if no contract code is executed. See https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable/blob/v4.8.3/contracts/utils/AddressUpgradeable.sol#L180
Open the reportLowInconsistent Access ControlWemix Retainer 103 KLEVA Omni · CertiK · 2 Apr 2024Fixed
In the contract `EarnRouter`, the function `sync()` can only be called by the contract `KlevaConfig`. However, the function `sync()` in the contracts `KlevaLocker`, `KlevaDistributor`, and `Dashboard` can be called by anyone as it has no access restriction. ```solidity =94 function sync() external { require(msg.sender == klevaConfig, "EarnRouter: Only klevaConfig"); klevaDistributor = IKlevaConfig(klevaConfig).getKlevaDistributor(); feeBox = IKlevaConfig(klevaConfig).getFeeBox(); } ``` ```solidity =43 function sync() external { klevaToken = IKlevaConfig(klevaConfig).getKlevaToken(); } ```
Open the reportLowInconsistent address checkingWemix Retainer 103 KLEVA Omni · CertiK · 2 Apr 2024Fixed
In the contract `KlevaConfig`, the function `setFeeBox()` checks that the input address is not `address(0)`. However, the other functions check whether the input address is a contract address. ```solidity=110 function setFeeBox(address feeBox_) external onlyOwner { require(feeBox_ != address(0), "KlevaConfig: Invalid feeBox address"); _feeBox = feeBox_; } ```
Open the reportLowMissing Check that `distributorId_` is greater tWemix Retainer 103 KLEVA Omni · CertiK · 2 Apr 2024Fixed
In the function `addPool()`, the input parameter `distributorId_` only checks that it should be less than or equal to the `lastPoolId` of the contract `KlevaDistributor`. However, in the contract `KlevaDistributor`, the minimum pool id is 1. If the `distributorId_` is 0, the check will pass and the correct pool cannot be found in the contract `KlevaDistributor`.
Open the reportLowNo way to retrieve ETH from the contractWemix Retainer 103 KLEVA Omni · CertiK · 2 Apr 2024Fixed
The following contracts contain the `receive()` function, yet the contract does not utilize forwarded ETH, and the contract is missing functionality to withdraw ETH from the contract. This means that funds may become trapped in the contract indefinitely.
Open the reportLowRedundant Code ComponentsWemix Retainer 103 KLEVA Omni · CertiK · 2 Apr 2024Fixed
In the function `addPool()`, there are two incorrect `require` statements: ```solidity require( target_ == address(0) || AddressUpgradeable.isContract(target_), "EarnRouter: Invalid target" ); require( implementation_ == address(0) || AddressUpgradeable.isContract(implementation_), "EarnRouter: Invalid implementation" ); ``` If `target_ == address(0)`, the code in L132 will revert. The same issue exists in the next `require` statement.
Open the reportShow 10 more
LowSafeTransfer should be used in place of transferWemix Retainer 103 KLEVA Omni · CertiK · 2 Apr 2024Fixed
SafeTransfer should be used in place of Transfer for Solidity contracts to ensure robust security and error handling. Unlike the basic Transfer function, SafeTransfer incorporates safeguards against potential smart contract vulnerabilities, such as unexpected token loss. By automatically validating the recipient's ability to receive tokens and reverting transactions in case of failures.
Open the reportLowIncorrect Naming Convention UtilizationWemix Retainer 102 DEX NFT Service · CertiK · 19 Mar 2024Fixed
The `s.balances[_to]-1` equals the index of the new-minted token, not the length of the `_ownedTokens[to]`. Naming this temp variable as `length` is not suitable. ```solidity uint256 length = s.balances[_to] - 1; ```
Open the reportLowRisks of `abi.encodePacked()` in User-ControlledWemix Retainer 102 DEX NFT Service · CertiK · 19 Mar 2024Fixed
Encoding through `abi.encodePacked()` is used with at least two dynamic type inputs controlled by the user. This type of encoding is susceptible to malleability when used with strings, bytes, or other dynamic types. As such, it is not a good choice for signature verification with the input used, since users can change the input and potentially get the same bytes output from `abi.encodePacked`. The susceptible malleable result can impact the global state variable `GachaStorage.seed`. ```solidity struct GachaStorage { uint256 seed; ... } function gachaRecycle(uint256 _gachaID, uint256[] calldata _payment) external { ... _executeGacha(_gs, _cs, _gachaID, _gacha, abi.encodePacked(_gachaID, _payment)); } function newSeed(GachaStorage storage _gs, bytes memory _salt) internal returns (uint256) { uint256 _seed = uint256(keccak256(abi.encodePacked(_gs.seed, _salt, msg.sender, block.timestamp, gasleft()))); _gs.seed = _seed; return _seed; } ```
Open the reportLowSafeTransfer should be used in place of transferWemix Retainer 102 DEX NFT Service · CertiK · 19 Mar 2024Fixed
SafeTransfer should be used in place of Transfer for Solidity contracts to ensure robust security and error handling. Unlike the basic Transfer function, SafeTransfer incorporates safeguards against potential smart contract vulnerabilities, such as unexpected token loss. By automatically validating the recipient's ability to receive tokens and reverting transactions in case of failures. ```solidity if (!IERC20(_gacha.payType).transferFrom(msg.sender, _gs.dev, _price)) revert GachaErrFailToBill(); ``` The standard function `transferFrom` in OZ ERC20 returns true forever, which should not be used as a check for the result of the transferFrom.
Open the reportLowIncorrect Error MessageWemix Retainer 106 DIOS Staking · CertiK · 18 Mar 2024Fixed
The above-mentioned error messages in the `claim()`, `claimWithSwap()`, and `compound()` functions of `StakingV10` contract are not correct. These functions have different usages than the `deposit()` function.
Open the reportLow`PlayMarketTokenManagerV3` imports non-upgradableWemix 99 PlayNFTMarket · CertiK · 1 Mar 2024Fixed
The upgradable contract `PlayMarketTokenManagerV3` imports the non-upgradable contract `Ownable`, which is not utilized within the contract.
Open the reportLowComment InconsistencyWemix Retainer 100 PlayNFTMarket (hybrid-auction) · CertiK · 1 Mar 2024Fixed
```solidity=35 // setCloseDelay() * Requirements * - caller must have Editor role of the contract ``` - Instead of the `Editor` role, sellers can call the `setCloseDelay()` function. --- Incorrect Error Message ```solidity=42 require(delayTime >= minimumDelayTime, "AOB: minimun delay time is 1 hour"); ``` - The `minimumDelayTime` is not constantly 1 hour. ```solidity=50 function setCollectionCloseDelay(address _collection, uint _delay) external onlyEditor { require(_delay >= minimumDelayTime, "AOB: minimum delay time is 1 hour"); //minimun = 1hour ``` - The `minimumDelayTime` is not constantly 1 hour. The error message and comment both are inaccurate. ```solidity=39 // PlayMarketEnglishAuctionOrderV4 require(order.concludedTime >= minimumDelayTime, "AC : minimun delay time is 1 hour"); ``` - The `minimumDelayTime` is not constantly 1 hour. ```solidity=39 // PlayMarketHybridAuctionOrder require(order.concludedTime >= minimumDelayTime, "HA : minimum delay time is 1 hour"); ``` - The `minimumDelayTime` is not constantly 1 hour. Furthermore, there is a typo in messages that "minimun" should be "minimum".
Open the reportLowGet the length of a numberWemix Retainer 100 PlayNFTMarket (hybrid-auction) · CertiK · 1 Mar 2024Fixed
```solidity=275 uint digit = bytes(Strings.toString(current)).length; ``` The approach to obtaining the length of a number can be optimized.
Open the reportLowInadequate checking in `isAuctionStarted()` functWemix Retainer 100 PlayNFTMarket (hybrid-auction) · CertiK · 1 Mar 2024Fixed
The `isAuctionStarted()` function incorrectly returns `true` for an order that does not exist. This is due to the function checking whether `_order.startTime <= block.timestamp`. Since `_order.startTime` defaults to zero for a non-existing order, the comparison `0 <= block.timestamp` is always true, leading to the incorrect conclusion that the auction has started.
Open the reportLowSellers can set close delay for an ongoing auctioWemix Retainer 100 PlayNFTMarket (hybrid-auction) · CertiK · 1 Mar 2024Fixed
The `PlayMarketAuctionOrderBase::setCloseDelay()` function is intended to restrict the modification of an order's close delay to a period before the auction starts. However, the function uses an incorrect conditional check to determine if the auction has started: ```solidity=40 require(_endTime[orderID] < type(uint).max, "AOB: set order's close delay is only available before auction start"); ``` This check is flawed because `_endTime[orderID]` is initially set to `type(uint).max` when an order is open (PlayMarketAuctionOrderBase, L155), and it is updated to a smaller value when a bidder places a bid: ```solidity=89 // PlayMarketEnglishAuctionOrderV4::offer() _endTime[orderID] = current + _closeDelay[_order.orderID]; ``` As a result, the condition allows sellers to change the order's close delay even after a bid has been made, contradicting the error message and the intended functionality.
Open the reportShow 10 more
LowDiscussion about the `verifyAndCreateMetaHash` fuWemix Retainer 96 UnaMetaHash · CertiK · 22 Feb 2024Fixed
In the function `verifyAndCreateMetaHash()`, when the parameter `sender` exists in the `validator`, the returned `metaHash` will be another input parameter `inputMetaHash`. ```solidity if (IUnaMetaHash(unaMetaHash).verifySender(sender)) { metaHash = inputMetaHash; } else { metaHash = IUnaMetaHash(unaMetaHash).createMetaHash(); } ``` This approach requires the caller to pass the correct `metaHash`.
Open the reportLow`arbitrageSwapRatio` should be less than `pauseSwFixed
In the function `setArbitrageSwapRatio` and `setMaxArbitrageSwapRatio`, it only guarantees the `arbitrageSwapRatio` is less than `MAX_ARBITRGE_SWAP_RATIO`, which may cause the `arbitrageSwapRatio` is greater than `pauseSwapRatio`, that violates the contract logic.
Low`checkUserNonce` not invokedFixed
Most of the methods in the contract `GovernanceAccrue` call the function `checkUserNonce`, but the `create` and `drop` functions do not call this function.
Low`deadline` Argument for `IPlaySwapRouter` FunctioFixed
Calls to `IPlaySwapRouter` functions are executed immediately within the same block. Setting `deadline` argument as `0xf000000000000000000000000000000000000000000000000000000000000000` is redundant and misleading.
Low`pause/unpause` functionalities not implementedFixed
The contract inherits `Pausable` and uses extensively the `whenNotPaused` modifier to prevent many functions from being called if the contract is paused. However, the contract does not implement any function allowing to set `_paused` as true. Therefore, the contract is not pausable.
Low`PPPLiquidity` does not support adding liquidityFixed
In the current implementation, function `callAddLiqudity` in `Dispatcher.sol` does not support adding liquidity with native token. If one of the `swapTokens` is `address(0)`, the transaction will revert. ```solidity=121 require(IERC20(swapTokens[0]).transferFrom(user, address(this), amounts[0]), "Dispatcher: fail to transfer token to router"); require(IERC20(swapTokens[1]).transferFrom(user, address(this), amounts[1]), "Dispatcher: fail to transfer token to router"); ``` Besides, `PPPSwap` does not support swap ERC-20 tokens with native token either.
Low`totalRemainAmount` is not updatedFixed
In the function `_confirm()` of the contract `Station`, the `totalRemainAmount` is updated after transferring to treasury when the DAO succeeds, but the function `_confirm()` of the contract `ReOpen` is not.
Low`wwemix` is not updatedFixed
In the constructor(), `wwemix` address is obtauned by the function `WWEMIX()` of the `_weswapRouter`. However, the function `changeWeswapRouter()` changes the `_weswapRouter` address, but does not get the new `wwemix` address.
LowAddress `_playBridge` should be a zero address ifFixed
If `_isWrappedNFT` is set to false when deploying NFTs, no NFTs will be minted by the `_playBridge` role. Therefore, the `_playBridge` role should be set to a zero address.
LowBalance of `WemixDollarTreasury`Fixed
The contract `AuthorizedMintAccess` address `WemixDollarTreasury` is responsible for holding the `WEMIX$` token. However, in the contract `WemixDollarTreasury`, no direct mechanism is present to guarantee a minimum balance, resulting in users being unable to redeem their `BYLO` tokens.
Show 10 more
LowBuy token amount and cost can be optimizedFixed
In the declaration of the `buyFanToken(address _fanToken, uint _cost, uint _amount)` function: - `_cost` represents the amount of WEMIXDollar that the user pays. - `_amount` represents the quantity of fan tokens that the user is purchasing. The `FanTokenMarket::_checkOrder()` function ensures that the `_cost` is sufficient to cover the payment for the specified `_amount` of fan tokens. However, since the `priceBook[token][price]` stores the available token amount and token price in the market, it is possible to derive either the `_amount` or the `_cost` from each other.
LowCollections support both ERC721 and ERC1155 standFixed
The NFT market currently does not support the NFT token that supports both ERC721 and ERC1155 standards. Any ERC721 token transfer is regarded as an ERC1155 token transfer with only one item transferred, for example, Sandbox's ASSETs. As the first “if” statement is always evaluated to true in this scenario, using the `transferNFT` function to transfer ERC1155 tokens mistakenly invokes the `IERC721(order.collection).safeTransferFrom()` function. Since the `IERC721(order.collection).safeTransferFrom()` is treated as an ERC1155 transfer with one item, only one item actually gets transferred when transferring multiple ERC1155 tokens.
LowCommented function bodyFixed
The current version of the `PPPZap.sol` contract has the function bodies of `_zapIn` and `_zapOut` commented out. Consequently, when a user invokes `zapIn` or `zapOut` through the `UniversalRouter`, only the token `transferFrom` call is executed, transferring the token from the user to the `PPPZap` contract. The expected subsequent call to `playSwapZap` does not get processed, and the user's funds remain stationary within the `PPPZap` contract.
LowConfusing commentsFixed
The auditor felt that "setTimeSet" and "setTicketInfo" functions are essential for the project; they must be called for the project to operate. The auditor wants to know why the command states, "This is for human error in service." ```sol /** * @dev External function to cancel the used ticket for the GSTAR2023 Admission. This is for Human Error in service. * @param ticketType a ticketType * @param startTime startTime of the GSTAR2023 Admission * @param endTime endTime of the GSTAR2023 Admission */ function setTimeSet(TypeGstar.TicketType ticketType, uint startTime, uint endTime) external onlyAdmin { require(startTime != 0 && startTime >= block.timestamp, "GSTAR: startTime is invalid"); require(endTime != 0 && endTime >= startTime, "GSTAR: endTime is invalid"); _timeSet[ticketType].startTime = startTime; _timeSet[ticketType].endTime = endTime; emit SetTimeSet(ticketType, startTime, endTime); } /** * @dev External function to set the ticket information for the GSTAR2023 Admission. This is for Human Error in service. * @param startId start ticketId * @param endId end ticketId */ function setTicketInfo(uint startId, uint endId) external onlyAdmin { for(uint i = startId; i <= endId; i++) { require(_exists(i), "GSTAR: nonexistent token"); _ticketInfo[i].ticketType = _ticketTypeOf[i]; _ticketInfo[i].isUsed = false; _ticketInfo[i].useDate = 0; if(_ticketInfo[i].ticketType == TypeGstar.TicketType.PASS_THU) { _ticketInfo[i].timeSet = _timeSet[TypeGstar.TicketType.PASS_THU]; } else if (_ticketInfo[i].ticketType == TypeGstar.TicketType.PASS_FRI) { _ticketInfo[i].timeSet = _timeSet[TypeGstar.TicketType.PASS_FRI]; } else if (_ticketInfo[i].ticketType == TypeGstar.TicketType.PASS_SAT) { _ticketInfo[i].timeSet = _timeSet[TypeGstar.TicketType.PASS_SAT]; } else { _ticketInfo[i].timeSet = _timeSet[TypeGstar.TicketType.PASS_SUN]; } emit SetTicketInfo(i, _ticketInfo[i]); } } ```
LowConfusing Function NamingFixed
The function naming of `onlyBatchFanToken` and `onlyDonationFanToken` conflicts with its functionality and is confusing. In the current implementation, `OnlyBatchFanToken` will toggle the state of `OnlyDonation` while `OnlyDonationFanToken` will toggle the state of `OnlyBatch`. ```solidity=209 function onlyBatchFanToken(address fanToken) external onlyOwner{ require(isOnlyDonation(fanToken), "FTR : Not Registered FanToken or Already OnlyBatch FanToken"); onlyDonation[fanToken] = false; } function onlyDonationFanToken(address fanToken) external onlyOwner{ require(onlyBatch[fanToken], "FTR : Not Registered FanToken or Already OnlyDonation FanToken"); onlyBatch[fanToken] = false; } ```
LowContract `PlayMarketTokenManager` imports inaccurFixed
The contract `PlayMarketTokenManager` imports the file `IERC721MetaData.sol`, while the correct file name is `IERC721Metadata.sol`.
LowDesign Issues Related to `loanStartTimestamp`Fixed
A potential vulnerability exists in the `bidLoan()` function in the codebase. When this function is called by a lender, it extends the `loanStartTimestamp` for a given loan by a certain time period, specified by the value of `extendDurationFromLastBidding`. Until the `loanStartTimestamp <= block.timestamp`, the loan cannot be claimed by the borrower. Similarly, once the loan has received a bid, it can no longer be delisted. The borrower, having sent an NFT as collateral to list the loan, faces the risk of having their NFT locked indefinitely. This is because a malicious lender could continuously bid on the loan, consequently preventing the borrower from retrieving their collateral. It's worth noting that while a lender can't continuously bid using the same address, the lender can alternate between two different addresses to enact this unfavorable behavior. Additionally, the initial `loanStartTimestamp` is assigned a value computed as `_blockTimestamp() + listTerms.auctionDuration`. When `listTerms.auctionDuration` is smaller or equal to `timeToBorrowerFeeDenominator`, a larger `listTerms.auctionDuration` results in a higher "borrower fee" for the borrower. Given that the `listLoan()` function requires `listTerms.auctionDuration` to be greater than 0, a borrower could set `listTerms.auctionDuration` to 1, bid on the loan in the same block, and then extend the `loanStartTimestamp` by the `extendDurationFromLastBidding` period. This would mean the borrower only has to pay a minimal fee.
LowDesign Logic of The `myrtleMigration()` FunctionFixed
When invoking the `myrtleMigration()` function, if the provided argument `newHighestPrice` exceeds the current `highestPriceGrade`, only the newly minted amount of `MYRTL` tokens and their corresponding pricing are documented within the `FanTokenMarket` contract. However, no additional `MYRTL` token assets are minted. This creates a difference between the recorded information and the actual circulating `MYRTL` assets. **FanTokenRouter.sol** ```solidity=93 //2. set data migration IMYRTLEMinter(fanTokenMinter).myrtleMigration(highestPrice); ``` **MYRTLEMinter.sol** ```solidity=12 function myrtleMigration(uint newHighestPrice) external { for(uint i = highestPriceGrade ; i <= newHighestPrice ; i++){ IFanTokenMarket(fanTokenMarket).setTokenPriceAmount(fanToken); } highestPriceGrade = newHighestPrice; } ``` The `setTokenPriceAmount()` function is also called within the `twapMint()` function. And the `FanTokenMarket` contract keeps track of the newly minted FanToken amount along with its corresponding prices. Simultaneously, an equivalent amount of FanToken assets is minted to the `FanTokenVault` contract (refer to the code snippet at lines 155-159). This process guarantees the supply of FanToken assets for users to purchase. **FanTokenMinter.sol** ```solidity=155 for(uint i = prevHighestPrice + 1 ; i <= twapPrice ; i++) { IFanTokenMarket(fanTokenMarket).setTokenPriceAmount(fanToken); } _mintUserFanToken(fanTokenVault, mintingAmount); highestPriceGrade = twapPrice; isMinted = true; ```
LowDiscussion: the use of whiltelistFixed
In the contract `PriceOracle`, the admin can add or remove addresses from the whitelist. However, it appears that the whitelist variable is not utilized in any other function within the contract. As such, it may be unnecessary to keep the whitelist variable and the associated functions within the contract, and they can be safely removed if they serve no other purpose.
LowDivide Before MultiplyFixed
Performing integer division before multiplication truncates the low bits, losing the precision of calculation. For example: 999/1000 * 9 = 0, while 999*9 / 1000 = 8
Show 10 more
LowDuplicates should not be added to the `feeders` aFixed
The `addFeeder()` function allows for adding a feeder that already exists, resulting in an array with redundant data and causing unnecessary gas consumption when looping through the array.
LowExit market if borrower has no debt and no assetsFixed
In the `repayBorrowFresh()` function, if the borrower has no outstanding borrowing balance and their account balance is zero in the market, the borrower exits the market. However, the function handles the case where `repayAmount == type(uint256).max`, but ignores the case that debts are fully paid off and `vars.accountBorrowsNew` is zero.
LowFunction `approve` Does Not Prevent Approval forFixed
It is expected that calls of the form `approve(spender, amount)` fail if the address in `spender` is the zero address.
LowFunction Calls User-Provided Addresses With No AcFixed
Calling a user provided address is dangerous, especially in a public function with no access control restriction. An attacker could deploy a malicious contract and use the vulnerable function to trigger a call to the malicious contract. The contract allows users to call the deposit function with any "assetAddress" address, and the contract performs a "safeTransferFrom" call to that address.
LowFunction Calls User-Provided Addresses With No AcFixed
Calling a user provided address is dangerous, especially in a public function with no access control restriction. An attacker could deploy a malicious contract and use the vulnerable function to trigger a call to the malicious contract, potentially stealing user funds or causing other serious damages.
LowFunction Can Be Modified With `pure` Or `view`Fixed
The `isFanTokenRouter()` function doesn't require modifying the contract state, nor do they need to receive Ether. However, this function is not labeled with `pure` or `view` modifiers.
LowFunctions With `_` as Name Prefix Are Not `privatFixed
Functions with names starting with `_` should be declared as `private`/`internal`.
LowGas OptimizationFixed
The linked code can be moved outside of the loop to save gas. ```solidity=295 bytes memory message = abi.encodePacked(id_, ca_, tokenAddr_, from_, to_, fromChain_, txHash_, amount_); ```
LowGet ineffective average price from `getTwapPrice(Fixed
In the function `getTwapPrice()`, if `blockTimeLog[twapIndex] != 0 and timeStamp - blockTimeLog[twapIndex] != 0`, the TWAP price is calculated as follows: ```solidity=291 // (currentprice - lastprice) / (current time stamp - last time stamp) twap = (cumlativeLast - priceLog[twapIndex]) / ( timeStamp - blockTimeLog[twapIndex]); ``` Due to the absence of validation on the time log, if the price log and time log are updated just before the `getTwapPrice()` function is called, the fetched price will closely approximate the current token price, rather than reflecting an average price.
LowHow to check the tokenA is more valuable than tokFixed
How to check the tokenA is more valuable than tokenB for the function unproportionalZapIn as it's comment: ```solidity=60 * tokenA should be valuable than tokenB (swap a into b) ```
Show 10 more
LowImplementation contract is not automatically initFixed
"Do not leave an implementation contract uninitialized. An uninitialized implementation contract can be taken over by an attacker, which may impact the proxy." See https://docs.openzeppelin.com/upgrades-plugins/1.x/writing-upgradeable#initializing_the_implementation_contract
LowImproper function namesFixed
Functions with inappropriate names do not reflect the implementation logic: - Function `blockBuyToken()` can be called by the breaker role to block or unblock the buying of a fan token. - Function `blockTwap()` can be called by the breaker role to block or unblock `twapMint()` for a fan token.
LowImproper variable name for order takerFixed
The mapping `orderSeller` stores accounts responsible for making payments for orders. However, its name implies that it is used to store sellers of orders.
LowInaccurate Check ConditionsFixed
When initializing the `FanTokenMinter` contract, it is essential to ensure that the provided `_swapPair` corresponds to the `WEMIX$ - fanToken` pair. The current flaw in the code logic arises from the absence of validation for the `_swapPair` to guarantee that it is indeed a pairing of `WEMIX$` with a fanToken. Consequently, the existing code allows the initialization of the `FanTokenMinter` contract even when an incorrect swap pair is used, such as pairing `WEMIX$` with a non-fanToken. ```solidity=104 if (IWeswapPair(swapPair).token0() == WEMIX$) { isFanTokenReserve0 = false; } else if (IWeswapPair(swapPair).token1() == WEMIX$) { isFanTokenReserve0 = true; } else { require(false, "FTM : Invalid FanToken<->WEMIX$ Pair address"); } ``` The check conditions at L104 and L106 only validate the presence of `WEMIX$` in `swapPair`. Consequently, one of the two checks will pass, even if the paired token for `WEMIX$` is a non-FanToken. Additionally, once the `FanTokenMinter` contract is initialized, the state variable `swapPair` cannot be modified.
LowInaccurate ComparisonFixed
The type of `spenderAllowance` is `uint96`, which should be compared with the maximum value of the `uint96`, not `uint256`, otherwise the `spenderAllowance != type(uint256).max)` condition will always evaluate true. Resulting codes in lines 67-74 will be executed even if `spenderAllowance` equals `type(uint96).max`.
LowInaccurate input for eventsFixed
The `offerer` is being used as the input for the `sender` parameter of the `OrderAccepted` event, which is not accurate if the caller of the function `offerSellOrder()` is not the same as the input `offerer`. The input for the `sender` of `OrderRegistered` event should be `msg.sender`.
LowIncompatibility With Deflationary TokensFixed
When transferring standard ERC20 deflationary tokens, the input amount may not be equal to the received amount due to the charged transaction fee. As a result, an inconsistency in the amount will occur and the transaction may fail due to the validation checks. For example, if a user sends 100 deflationary tokens (with a 10% transaction fee) to the target bridge contract, only 90 tokens actually arrive to the contract. Current implementation of `WemixBridgeV3` and `Dispatcher` is not compatible with deflationary tokens.
LowIncomplete Key Removal in `removeAddressOf()` FunFixed
The `removeAddressOf()` function in the codebase, intended to remove an address associated with a given key, does not remove the key itself from the `_addressKey` array, which maintains the list of all registered keys.
LowInconsistency Between Code and CommentFixed
The `burn()` and `burnBatch()` functions are specifically intended for NFT owners to burn their owned NFTs. However, the error messages at L32 and L39 indicate that an account with an `editor` role can also burn NFTs. The current code issue lies in the inconsistency between the logic of the functions and the corresponding error messages. ```solidity=31 function burn(uint _tokenID) external { require(msg.sender == ownerOf(_tokenID), "FN: only NFT owner or editor can burn"); _burn(_tokenID); } function burnBatch(uint[] calldata _tokenIDs) external { for (uint i = 0; i < _tokenIDs.length; i++) { uint tokenID = _tokenIDs[i]; require(msg.sender == ownerOf(tokenID), "FN: only NFT owner or editor can burn"); _burn(tokenID); } } ``` --- The message in the `isNotOneOfOwner()` function does not reflect the logic correctly. ```solidity=281 modifier isNotOneOfOwner(address _owner) { require(!_isOwner[_owner], "FanTokenVault: Owner can not access."); _; } ``` --- The error message for the token minter check in the GameTokenTwapRouter contract should be "GTTR : not registered game token minter". ```solidity=26 require(isRegisteredTokenMinter[gameTokenMinter], "GTTR : not registered game token"); ```
LowInconsistency Check ValidationFixed
The sum of `incomeRatio` and the ratio of creators is checked to be less than `_baseUnit` in the function `isValidParams()` of the contract `RevenueSpell`, but the function `_checkCreate()` does not.
Show 10 more
LowInconsistent auction start timeFixed
```solidity=238 function isAuctionStarted(uint orderID) public view returns (bool) { Type.Order memory _order = _orders[orderID]; return _order.startTime < block.timestamp; } ``` ```solidity=76 require(_order.startTime <= current, "AC: order not started yet"); ``` The auction start time checks are inconsistent if the `block.timestamp` is equal to `_order.startTime` in the `offer()` and `isAuctionStarted()` functions.
LowInconsistent Comment and CodeFixed
The `_settleDex()` function settles fees for `DEX` and `NFTMARKET`. The comments do not describe the function accurately.
LowInconsistent Comment and CodeFixed
In the comment of function `zapIn`, fromToken address need to be (address(0x00) if WEMIX. It will revert the invests from WEMIX. ```solidity=32 * @param _fromToken The ERC20 token used for investment (address(0x00) if WEMIX) ``` But there's no related logic in `zapIn` function implements this logic. If a user invoke the `zapIn` function to invest from WEMIX, _performZapIn still could be proceed.
LowInconsistent Comment and CodeFixed
The check "MIN_ARBITRGE_SWAP_RATIO =< arbitrageSwapRatioInverse < minBYLOPrice" in the comment not implement in the function `setArbitrageSwapRatio`.
LowInconsistent Comments and function nameFixed
```solidity=115 // @dev Returns `totalLength` if `addr` gets role of manager. function addManager(address addr) external onlyOwner { ``` The addManager() function is responsible for adding a new manager to the _managers list, however, the accompanying comment does not accurately describe this functionality.
LowInconsistent direction check between `WemixBridgeFixed
In the current implementation of `WemixBridgeV3`, when the `BridgeDir` is set to `Uni`, the `TokenObjHome` is required to be `Away`. ```solidity=328 if(_wemixBridgeStorage.getTokenObjBridgeDir(exitObj.id) == BridgeObjs.BridgeDir.UNI){ require(_wemixBridgeStorage.getTokenObjHome(exitObj.id) == BridgeObjs.Home.AWAY, "WemixBridge: invalid direction"); } ``` However, in the similar case in `WemixBridgeV2`, the `Tokens.Side` is required to be `HOME` ```solidity=231 if(_wemixBridgeStorage.getTokenExchangeDir(id) == Tokens.ExchangeDir.UNI){ require(_wemixBridgeStorage.getTokenSideType(id) == Tokens.Side.HOME, "WemixBridge: invalid exchange direction"); } ``` This inconsistency is confusing to the auditor and we expect the team to double check the linked code logic and make sure it align with the intended design.
Lowinconsistent error messageFixed
In the current implementation of the contract, there's an inconsistency in the error messages provided within the `lockToken` and `unlockToken` functions. In the lockToken function, the error message for the require statement checking the validity of the id is "WemixBridge: incorrect id", while in the unlockToken function, the corresponding error message is "WemixBridge: invalid id". ```solidity=264 require(_wemixBridgeStorage.getTokenObjId(id) == id, "WemixBridge: incorrect id"); ``` ```solidity=275 require(_wemixBridgeStorage.getTokenObjId(id) == id, "WemixBridge: invalid id"); ```
LowInconsistent Error Message Prefixing in ContractsFixed
Error messages in linked contracts are prefixed with "AC:", an abbreviation that does not align with the standard practice of using the actual contract name initials.
LowInconsistent with the logic of the `distributeSupFixed
In the `distributeSupplierIncentiveToken()` function, line 712 covers the case where users supplied tokens before the market's supply state index was set. Rewards the user with `INCENTIVE` accrued from the start of when supplier rewards were first set for the market. So the `getCurrentMyIncentiveToken()` function should use the same comparison conditions.
LowIncorrect calculation of ratioFixed
In the `SwapPause` function, the pause condition is not correct. Because `_reserve0` is greater than `_reserve1`, and `_reserve1.mul(10 ** 18) / _reserve0` is less than `ONE_WEMIX`(10 ** 18), and the `pauseSwapRatio` is greater than `ONE_WEMIX`, so the `_reserve1.mul(10 ** 18) / _reserve0 > pauseSwapRatio` is always false.
Show 10 more
LowIncorrect CommentFixed
The function takes 'ticketType' as an argument, but the comment states '... get the ticket time of the ticketId.' It should be 'ticketType' instead of 'ticketId. /** * @dev External function to get ticket time of the ticketId. * @param ticketType a ticketType * @return TypeGstar.TimeSet of the ticketId */ function getTimeSet(TypeGstar.TicketType ticketType) external view returns(TypeGstar.TimeSet memory) { return _timeSet[ticketType]; }
LowIncorrect ComparisonFixed
In the `ResumeSwap` function, the unpause condition is not correct. Because `_reserve0` is greater than `_reserve1`, and `_reserve1.mul(10 ** 18) / _reserve0` is less than `ONE_WEMIX`(10 ** 18), and the `pauseSwapRatio` is greater than `ONE_WEMIX`, so the `_reserve1.mul(10 ** 18) / _reserve0 > pauseSwapRatio` is always false. Given that the condition is being negated, it is always true.
LowIncorrect Transaction Reversion in `_maxApproveNFFixed
```solidity=205 function _maxApproveNFT(address operator, address nftContract) internal { IAddressManager addressManager = auction.addressManager(); INFTWhiteList.NFTType nftType = INFTWhiteList( addressManager.addressOf(NFT_WHITE_LIST_KEY) ).nftToType(nftContract); if ( nftType == INFTWhiteList.NFTType.ERC721 && !IERC721(nftContract).isApprovedForAll(address(this), operator) ) { IERC721(nftContract).setApprovalForAll(operator, true); } else if ( nftType == INFTWhiteList.NFTType.ERC1155 && !IERC1155(nftContract).isApprovedForAll(address(this), operator) ) { // cause it is NFT, transfer amount = 1 IERC1155(nftContract).setApprovalForAll(operator, true); } else { revert("NftFiHelper::_maxApproveNFT: Failed to approve NFT."); } } ``` The `_maxApproveNFT()` function is designed to grant approval to a specific operator for either ERC721 or ERC1155 tokens. However, it incorrectly reverts transactions for already-approved ERC721 or ERC1155 tokens, leading to list-loan transaction rejections.
LowIncorrect Transaction Revertion in `_swapAndApproFixed
The `_swapAndApproveForAction()` function is expected to handle `address(0)` as a valid input for the `toToken` parameter. However, if `address(0)` is indeed passed in as the `toToken`, the function will end up reverting due to the following function call: ```solidity=62 require( IERC20(toToken).balanceOf(msg.sender) >= userToTokenUsage, "NfiFiHelper::listLoanHelper: Insufficient balance for the request." ); ```
LowIncorrect User State Update When User Has MultiplFixed
The function `burn()` removes user details from the `_users` and `_poolUsers` mappings without checking if the user has multiple positions. Setting `_users[msg.sender] = false` and `_poolUsers[poolId][msg.sender] = false` indiscriminately deletes the user’s involvement in other positions they may hold, which also inaccurately reduces `totalUsers` and `totalPoolUsers`. ```solidity=665 function _burnInternal(uint256 tokenId) internal { Position storage position = _positions[tokenId]; require( position.liquidity == 0 && position.tokensOwed0 == 0 && position.tokensOwed1 == 0, "Not cleared" ); delete _positions[tokenId]; _burn(tokenId); uint80 poolId = position.poolId; if (_users[msg.sender]) { _users[msg.sender] = false; totalUsers -= 1; } if (_poolUsers[poolId][msg.sender]) { _poolUsers[poolId][msg.sender] = false; totalPoolUsers[poolId] -= 1; } } ``` Besides, when burning a position NFT, `_poolIdToPoolTotalSupply` should also be updated to reflect the total supply changes.
LowLack of a function to set `guardians` to `false`Fixed
The `setGuardians()` function can only set `guardians` to `true`, there is no function that can set it to `false`. If the admin sets an incorrect address for the `guardians`, then the address can set the price.
LowLack of exiting market logic in the `redeemUnderlFixed
In the `redeemUnderlyingMax()` function, if the sender has no outstanding borrowing balance, the sender redeems all cTokens and exits the market. However, in the `redeemUnderlying()` function, if the input parameter `redeemUnderlyingAmount` is equal to the balance of the sender's CToken, and the sender doesn't have any debt, the logic for exiting the market is missing.
LowLack of Input ValidationFixed
The `getPath()` function takes a user provided `sp` address as function argument, without validating whether the address is indeed a valid staking pool. The user can implement ay logic in the `rt()` and `st()` functions in line 110 and 111.
LowLack of Input Validation in `swapOut()`Fixed
The function `swapOut()` does not require `amountIn <= amountInMax` when `path[0] == WWEMIX`. This could potentially lead to unexpected transaction in the contract's execution.
LowLack of sanity check on `orderStrategyType`Fixed
The `open()` function of strategy contracts is missing a sanity check for input `order.orderStrategyType` to ensure the correct strategy is used to handle the order. - Verify the input `order.orderStrategyType` for `PlayMarketEnglishAuctionOrder.open()` function is `OrderStrategyType.AUCTION`. - Verify the input `order.orderStrategyType` for `PlayMarketFixedPriceOrder.open()` function is `OrderStrategyType.SELL`. - Verify the input `order.orderStrategyType` for `PlayMarketOpenPriceOrder.open()` function is `OrderStrategyType.BUY`.
Show 10 more
LowLack of Storage Gap in Upgradeable ContractFixed
There is no storage gap preserved in the `NFTReceiverUpgradeable` contract. Any logic contract that acts as a base contract that needs to be inherited by other upgradeable child should have a reasonable size of storage gap preserved for the new state variable introduced by the future upgrades.
LowLack of Storage Gap in Upgradeable ContractFixed
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". Otherwise it may be very difficult to write new implementation code. Without storage gap, the variable in child contract might be overwritten by the upgraded base contract if new variables are added to the base contract. Refer to https://docs.openzeppelin.com/upgrades-plugins/1.x/writing-upgradeable
LowLack Validation for Array LengthFixed
The `setLiquidationIncentive()` and `setIncentiveTokenSpeed()` functions lack of sanity check to ensure `cTokens.length` and `newLiquidationIncentiveMantissas.length` are the same.
LowLocked Native token(Coin) in contract `UniversalRFixed
There is no rescue function in the universal router contract. If the user provided more `msg.value` than the `transferAmount` or directly transfer coins to the `UnivesalRouter`, the leftover coins will be forever locked in the contract. ```solidity=60 else { // if fromToken is coin (success, output) = CA(PPPConfigs.pppRequest).call{value: transferAmount}(inputs); } ```
LowLogical issue of function `exchangeRateStoredInteFixed
In the aforementioned line, the formula for the calculation of `exchangeRate` is as follows after cToken is minted: $$ exchangeRate =\\ {totalCash + totalBorrows - totalReserves \over totalSupply } $$ ```solidity=647 function exchangeRateStoredInternal() internal view returns (uint256) { uint256 _totalSupply = totalSupply; if (_totalSupply == 0) { return initialExchangeRateMantissa; } else { uint256 totalCash = getCashPrior(); uint256 cashPlusBorrowsMinusReserves = totalCash + totalBorrows - totalReserves; uint256 exchangeRate = (cashPlusBorrowsMinusReserves * expScale) / _totalSupply; return exchangeRate; } } ``` In solidity, division calculations have truncation problems. The `totalSupply` will be 1 and `exchangeRate` will be much smaller than `initialExchangeRate` in case the last user redeems (`accountTokens[redeemer] - 1`) cToken. As a result, the `exchangeRate` would be extremely small. When the value of `exchangeRate` is much smaller than `initialExchangeRate`, the user can mint cTokens well above normal values, and then the value of `exchangeRate` will be normal with the interest generating. In other words, the users can use this arbitrage to take away the underlying tokens in this pool. For example, the user can mint the amount of 1e8 CToken with one underlying token in case `exchangeRate = 1/1e8`.
LowLoop OptimizationFixed
The below loop will continue until `criteria` is smaller than `start`. ```solidity=131 for(uint i = 0; i < size; i++) { end = startBlock.add(_schedules[i].stage); if(criteria >= start && criteria < end) { pos = i; start = startBlock.add(_schedules[i].stage); } } ``` `criteria < end` is already checked on line 128 and the value of `criteria` hasn't changed since then.
LowMagic Number in PlaySwapLiquidityMathLibraryFixed
In the function `computeProfitMaximizingTradeRenewal` and `computeProfitMaximizingStablecoinPoolTrade` used for swap amountIn calculation, the magic number used in the calculation of `leftSide` and `rightSide`. We need confirm the mathematics of the formulas. Why the number `53200`, `53067`, and `636804` are used for the left side and `799`, `798`( `636804`'s square root ) for the right side.
LowMisleading State Variable Initialization in UpgraFixed
In upgradeable contracts, any state variable that is initialized at the point of declaration is misleading, as these initializations will have no effect. This is because the constructor in an upgradeable contract is never invoked, which means that initializations typically performed by the constructor are not executed. Therefore, if state variables are initialized when they are declared in the contract, these initializations will be ignored.
LowMissing `creator` CheckFixed
The provided `creator` is intended to receive LP token assets, and its address should match the creator of the given `fanToken` that has been registered within the `FanTokenFactory` contract. However, there is currently no validation mechanism in place to ensure this match. ```solidity=118 function initAddLiquidity(address fanToken, address creator, uint liquidityAmount) external onlyEditor { require(!initialized[fanToken], "FTR : fanToken is already initialized"); require(IFanTokenFactory(fanTokenFactory).isRegisteredFanToken(fanToken), "FTR : not registered fan token"); //1. check create pair address swapPair = IPlaySwapRouter(playSwapRouter).getPair(fanToken, WEMIXDollar); if(swapPair == address(0)){ swapPair = IWeswapFactory(weswapFactory).createPair(fanToken, WEMIXDollar); } //2. exchange fan token(fan token & dollar is in this contract) IERC20(WEMIXDollar).transferFrom(msg.sender, address(this), liquidityAmount); IFanToken(fanToken).mint(address(this), liquidityAmount); //3. add liquidity(reserve ratio msut be one by one because of initial liquidity provision) IERC20(WEMIXDollar).approve(playSwapRouter, liquidityAmount); IERC20(fanToken).approve(playSwapRouter, liquidityAmount); (uint fanTokenLiquidity, uint wemixDollarLiquidity, ) = IPlaySwapRouter(playSwapRouter).addLiquidity(fanToken, WEMIXDollar, liquidityAmount, liquidityAmount, 0, 0, creator, block.timestamp + 10); ... ```
LowMissing `order.orderAmount` ValidationFixed
The `_mintFanNFT()` function is designed to mint a new FanNFT and register its sell order in the `PlayMarketBaseV3` contract. The comment at line 144 of the `registerSellOrder()` function states: **PlayMarketBaseV3.sol** ```solidity=144 `_order.orderAmount` 1 for 721 standard, valid amount for 1155 standard. ``` The current implementation of the `_mintFanNFT()` function may result in `orderAmount` being greater than one when the token is of the 721 standard. This is because both the `_mintFanNFT()` in `FanNFTMinter.sol` and `registerSellOrder()` in `PlayMarketBaseV3.sol` functions do not check if `requestOrder.orderAmount` equals one when listing a token of the 721 standard on market. **FanNFTMinter.sol** ```solidity=188 for (uint i = 0; i < _needToMintAmount; i++) { currentFanTokenID += 1; Type.Order memory requestOrder = _order[i]; require(permissibleOrderStrategyType[requestOrder.orderStrategyType], "FNM : not support orderStrategyType"); require(requestOrder.payment == address(_fanToken), "FNM: payment is not fanToken"); require(requestOrder.collection == address(_fanNFT), "FNM: collection is not fanNFT"); require(requestOrder.seller == _creator, "FNM: order seller must be creator"); require(requestOrder.tid == currentFanTokenID, "FNM: requested order's Tid must be same from the Tid being minted"); fanNFTMinted[_year][_month].push(currentFanTokenID); tokenIDs[i] = currentFanTokenID; fanNFTTokenMintedDate[currentFanTokenID] = MintedNFT({ year : _year, month : _month, creator : _creator }); _fanNFT.mint(address(this), _uri[i]); _fanNFT.approve(address(_playMarket), requestOrder.tid); _playMarket.registerSellOrder(_strategyIndex[i], requestOrder); } ``` **PlayMarketBaseV3.sol** ```solidity=149 function registerSellOrder(uint _strategyIndex, Type.Order memory _order) public virtual payable isPaymentToken(_order.payment) isFunctionNotBlocked(FUNCTIONS.REGISTER_SELL_ORDER) { (address strategy, , bool isValid, Type.OrderStrategyType strategyType) = strategyByIndex(_strategyIndex); _order.orderStrategyType = strategyType; require(isValid, "MB: Invalid strategy"); require(_order.collection != address(0) && ITokenManagerV2(tokenManager).isRegisteredCollectionByOrderType(_order), "MB: Invalid collection address"); require(_order.seller != address(0), "MB: Invalid order due to invalid seller address"); require(_order.orderAmount > 0, "MB: Invalid order amount"); require(_checkBalance(msg.sender, _order.collection, _order.tid, _order.orderAmount), "MB: Not enough nft balance to list order"); require(!(_order.orderStrategyType == Type.OrderStrategyType.BUY), "MB: This function is not for buy order"); uint orderID = ordersID.getOrderId(); if (_order.orderStrategyType == Type.OrderStrategyType.AUCTION) { require(_order.orderAmount == 1, "MB: auction order amount should be 1"); } ... ```
Show 10 more
LowMissing `return` in `if` blockFixed
The function `editPte()` can change or delete the address of `pte`. However, the `if` block does not exit, which causes the update logic to continue executing. Once the `_pte[name]` is updated to `address(0)`, execute the function `editPte()` again to update it to a non-zero address, the new `Pte` is not added to `_ptes`.
LowMissing `tokenId` validation in function `getCollFixed
The function `getCollectBlockNumber()` directly accesses the `_positions` mapping with the provided `tokenId` but lacks any form of validation to confirm that the tokenId corresponds to a valid, existing position. This oversight could lead to undesired behavior, as the function would return a default value of 0 for nonexistent positions, potentially misleading users or external systems. ```solidity=225 function getCollectBlockNumber( uint256 tokenId ) external view override returns (uint256 collectBlockNumber) { Position memory position = _positions[tokenId]; return position.collectBlockNumber; } ``` In contrast, the `positions()` function includes a check for `position.poolId != 0` to validate that the tokenId maps to an existing position. ```solidity=206 Position memory position = _positions[tokenId]; require(position.poolId != 0, "Invalid token ID"); ```
LowMissing `whenNotPaused` Modifier in `retryEnter()Fixed
The `retryEnter()` function currently does not have the `whenNotPaused` modifier implemented. This allows transactions to be executed even when the contract is paused.
LowMissing Break In LoopFixed
```solidity=45 for(uint i = 0; i < _melters.length; i++) { if(_melters[i] == melter) { _melters[i] = _melters[_melters.length - 1]; _melters.pop(); } } ``` ```solidity=45 for(uint i = 0; i < _ptes.length; i++) { if(_ptes[i].name == name) { _ptes[i] = _ptes[_ptes.length - 1]; _ptes.pop(); } } ``` If `_melters[i] == melter`/`_ptes[i].name == name` is true, the loop can finish after swapping and popping to save gas.
LowMissing Check that sender is `user`Fixed
In the contract `Station`, the sender can only refund themselves when the Dao fails in the function `refund()`. However, the function `_checkRefund()` is missing a check that ensures the sender is equal to the `user`. This allows the sender to refund other users.
LowMissing Checks On The Function Parameter `_tochaiFixed
In the public function `exit`, there is no sanity check on function parameter `_tochain`. ```solidity=200 function exit( bytes32 service, bytes32 id, address tokenAddr, address from, address to, bytes32 toChain, uint amount, uint feeAmount ) public payable whenNotPaused nonReentrant { require(id != bytes32(0), "WemixBridge: invalid token id"); require(_wemixBridgeStorage.getTokenId(id) == id, "WemixBridge: incorrect token id"); require(_wemixBridgeStorage.getTokenAddr(id) == tokenAddr, "WemixBridge: incorrect token address"); require(!_wemixBridgeStorage.getTokenLocked(id), "WemixBridge: token is locked"); require(amount > 0, "WemixBridge: amount must greater than zero"); ``` Parameter `_toChain` can be set as any chain name, including the current deployed chain `_chain`. Invalid chain names might cause the failure of bridge exit operation and the loss of user's asset.
LowMissing Emit EventsFixed
There should always be events emitted in the sensitive functions that are controlled by centralization roles.
LowMissing Emit EventsFixed
There should always be events emitted in the sensitive functions that are controlled by centralization roles.
LowMissing Emit EventsFixed
There should always be events emitted in the sensitive functions that are controlled by centralization roles.
LowMissing Emit EventsFixed
Functions that update state variables should emit relevant events as notifications.
Show 10 more
LowMissing Emit EventsFixed
There should always be events emitted in the sensitive functions that are controlled by centralization roles.
LowMissing Error MessagesFixed
The **require** can be used to check for conditions and throw an exception if the condition is not met. It is better to provide a string message containing details about the error that will be passed back to the caller.
LowMissing Event EmitFixed
There's no event emitted from the constructor of `DAOToken`, in which several privileged addresses are set. These variables are also `private`. In `FundRouter`, there's no event emit from the privileged `setDeadline()` function.
LowMissing Storage Gap in ContractsFixed
The `PoET` contract is upgradeable but does not contain any storage gap. If the contract is used as a base contract, the lack of storage gap could limit the ability to add new storage variables in future upgrades, or even risk storage slot collision.
LowMissing Token Pair Order Verification in _swap FuFixed
The `_swap` function in the `PlaySwapRouter.sol` contract does not perform an essential verification check that identifies the order of the token pair involved in the transaction. Due to the indirect interaction with the `sortToken` function through the different `swap*` functions, a discrepancy in the order of the tokens could occur.
LowMissing validation of `fromToken` , `toToken` andFixed
In the current implementation, there is no validation that make sure `fromToken` and `Path[0]`, `toToken` and `Path[1]` are the same token address or corresponding `orginal token/wrapped token` pair. If the `fromToken`, `toToken` and `Path` does not match, the call to the `swapIn` and `swapOut` will revert due to the insufficient token balance in the `PPPSwap` contract. ```solidity=335 } catch (bytes memory _errMessage){ if (_swapInInfo.fromToken.isContract()) { IERC20(_swapInInfo.fromToken).safeTransfer(_swapInInfo.user, _swapInInfo.amountIn); ```
LowMissing validation of `fromToken`, `zapInToken` aFixed
The existing implementation lacks a validation process that ensures `fromToken`, `zapInToken`, and `zapOutToken` are either the same token address (for home tokens) or a corresponding pair of 'original token/wrapped token' (for wrapped tokens). In situations where `fromToken` does not match `zapInToken`, if the function call to `playSwapZapIn.zapIn` or `playSwapZapOut.zapOut` fails, the caller may be unable to receive any refund due to the discrepancy between `fromToken` and `zapInToken` or `zapOutToken`. ```solidity=247 catch (bytes memory _errMessage){ if (_zapInInfo.fromToken.isContract()) { IERC20(_zapInInfo.fromToken).safeTransfer(_zapInInfo.user, _zapInInfo.amount); ```
LowMissing validation of `tokens` and `swapTokens`Fixed
In the current implementation of `addLiquidity`, there is no validation that make sure the `tokens` and `swapTokens` are corresponding token/wrapped token pair. If the user set `tokens` and `swapTokens` as different values, they may not be able to receive remaining tokens or refund when `addLiqudity` failed.
LowMissing Zero Address and `isContract()` CheckFixed
There is no 0 address or `isContract()` check on the important addresses being set.
LowMissing Zero Address ValidationFixed
The cited address input is missing a check that it is not `address(0)`.
Show 10 more
LowMissing Zero Address ValidationFixed
Addresses are not validated before assignment or external calls, potentially allowing the use of zero addresses and leading to unexpected behavior or vulnerabilities. For example, transferring tokens to a zero address can result in a permanent loss of those tokens.
LowMissing Zero Address ValidationFixed
Addresses are not validated before assignment or external calls, potentially allowing the use of zero addresses and leading to unexpected behavior or vulnerabilities. ```solidity=121 operatorManager = _newOperatorManager; ``` - `_newOperatorManager` is not zero-checked before being used.
LowMissing Zero Address ValidationFixed
Addresses are not validated before assignment or external calls, potentially allowing the use of zero addresses and leading to unexpected behavior or vulnerabilities. For example, mistakenly transferring ownership to a zero address can result in a permanent loss of ownership. ```solidity=35 admin = newAdmin; ``` - `newAdmin` is not zero-checked before being used. ```solidity=82 addressManager = IAddressManager(newAddressManager); ``` - `addressManager` is not zero-checked before being used. ```solidity=114 borrower: to, ``` - `to` is not zero-checked before being used. ```solidity=29 WWEMIX = _WWEMIX; ``` - `_WWEMIX` is not zero-checked before being used. --- ```solidity=34 factory = _factory; ``` - `_factory` is not zero-checked before being used. --- ```solidity=35 WWEMIX = _WWEMIX; ``` - `_WWEMIX` is not zero-checked before being used.
LowMissing Zero Address ValidationFixed
Addresses should be checked before assignment or external call to make sure they are not zero addresses. ```solidity=40 masterAdmin = masterAdmin_; ``` - `masterAdmin_` is not zero-checked before being used. --- ```solidity=49 masterAdmin = newMasterAdmin; ``` - `newMasterAdmin` is not zero-checked before being used. --- ```solidity=33 masterAdmin = newMasterAdmin; ``` - `newMasterAdmin` is not zero-checked before being used. ```solidity=19 masterAdmin = masterAdmin_; ``` - `masterAdmin_` is not zero-checked before being used. ```solidity=25 controllerView = controllerView_; ``` - `controllerView_` is not zero-checked before being used. ```solidity=31 INCENTIVE_TOKEN_ADDRESS = incentiveTokenAddr; ``` - `incentiveTokenAddr` is not zero-checked before being used. ```solidity=42 masterAdmin = newMasterAdmin; ``` - `newMasterAdmin` is not zero-checked before being used.
LowMissing Zero Address ValidationFixed
The cited address input is missing a check that it is not `address(0)` before setting： - factory
LowMisuse of the `giveBenefit` Function Can PotentiaFixed
All the benefit contracts (Gift, FreeDrink, FreeFood, etc.) contain the "giveBenefit" function. The function resets the "_isUsed" to false when called, without modifying the "benefitInfo.round.count" value. If the "useBenefit" function is called for a benefit, and then "giveBenefit" is called subsequently, it can cause the benefit status to become messed up. Consequently, "unuseBenefit" and "useBenefit" functions will no longer work as intended for that particular benefit.
LowMultiple `name`s are mapped to the same `pte` addFixed
The function `regist()` only checks whether the `name` is registered, but not whether the corresponding `pte` address exists, which can lead to multiple `name` mappings pointing to the same `pte` address. The function `editPte()` has the same issue.
LowNo Upper Limit in changeExchangeFeePermile functiFixed
There are no upper boundaries for "changeExchangeFeePermile" which is used to change the token or item. It is possible to set the total fee rate up to any arbitrary amount.
LowPossible Signature Reuse in `enter()` FunctionFixed
The function `enter()` in the current code requires the `msg.sender` to use a valid signature for the function to be called. This is controlled by the `onlyValidatorSig` modifier, which takes `address(this)`, `msg.sender`, and `seq` as parameters. However, given that the values of `address(this)` and `msg.sender` are constant, and the lack of restriction on `seq` (except that it cannot be 0), there exists a possibility for signature reuse. This vulnerability, if exploited, would allow a user to make repeated calls to the `enter()` function. ```solidity=99 onlyValidatorSig(abi.encodePacked(address(this), msg.sender, seq), validatorSig) ```
LowPotential Divide by ZeroFixed
In the function `_confirm()` of the contract `ReOpen`, the `reOpenInfo.purposeAmount` is divided by `reOpenInfo.addMintAmount` which may be zero. Because the function `isValidParams()` is missing to check that `inputs.addMintAmount` is not equal to 0. The function `_confirm()` of the contract `Station` has the same issue.
Show 10 more
LowPotential Gas ExhaustionFixed
The computation for each iteration is complex, it could lead to gas insufficiency.
LowPotential Incorrect Payment TokenFixed
The function `payWithSwap()` allows the caller to input any `token` address as a parameter to perform payments with the contract. The documentation states that only wrapped game tokens are intended to be used with the function `payWithSwap()`. The current implementation allows any token paired with the `dollar` address on PlaySwap to be used as payments. Users could potentially bypass this requirement and pay by swapping tokens that are not wrapped game tokens but paired with the `dollar` address.
LowPotential Mismatch in Token WithdrawalFixed
```solidity=191 borrowerFeeAccrued[loan.lendingERC20Contract] += loan.borrowerFeeAmount; ``` There's a potential flaw related to how fees are handled when a borrower cancels/delists a loan using the `delistLoan()` function. According to the code, the borrower pays a "borrowerFee" which is pre-collected in the form of `loan.borrowerFeeToken`. However, the accrued fee is tracked in the `borrowerFeeAccrued` mapping that links it to the `loan.lendingERC20Contract` instead. This mismatch could create an inconsistency if `loan.borrowerFeeToken` and `loan.lendingERC20Contract` are not the same. In such cases, the contract's admin might inadvertently withdraw incorrect tokens, leading to potential losses or incorrect balances.
LowPotential Out-of-Gas ExceptionFixed
When removing a FanToken, using a for loop that involves an arbitrary number of iterations or accesses the state variable `allFanToken` within its body may lead to the transaction running out of gas and reverting when accessing the `removeFanToken` function. **FanTokenFactory.sol** ```solidity=89 for(uint i = 0 ; i < length ; i++){ if(allFanToken[i] == fanToken) { allFanToken[i] = allFanToken[length - 1]; allFanToken.pop(); } } ``` Based on the code logic of the `registerFanToken()` and `createFanTokenMinter()` functions, there appears to be no restriction on the number of FanToken addresses within the `allFanToken` collection. Additionally, the provided for loop code scans through all items in `allFanToken` during the removal of a fanToken, as there is no `break` statement within the loop. If `allFanToken` contains a substantial number of FanToken addresses, executing the aforementioned for loop could result in running out of gas. ```solidity=37 function registerFanToken(address fanToken, address fanTokenMinter, address creator) external onlyEditor { require(fanToken.isContract() && fanTokenMinter.isContract() && creator != address(0), "FTF : address is invalid"); if(!isRegisteredFanToken[fanToken]){ allFanToken.push(fanToken); } ... ``` ```solidity=54 function createFanTokenMinter(address fanToken, address creator) external onlyEditor { require(fanToken.isContract() && creator != address(0), "FTF : address is invalid"); if(!isRegisteredFanToken[fanToken]){ allFanToken.push(fanToken); } ... ``` Moreover, the `allFanToken` must not include duplicate FanToken addresses, as only non-registered FanToken addresses are added to the `allFanToken` list. Therefore, it is essential to terminate the for loop at lines 89-94 when the statements at lines 91-92 are executed. --- If the ledger of a specific FanToken stores large numbers of records, the for loop within `getTotalTokenInfo()` function may run out of gas and revert the transaction accessing it. **FanTokenMarket.sol** ```solidity=165 for (uint price = ledger.startPrice; price <= ledger.endPrice; price += _interval) { prices[idx] = price; amounts[idx] = priceBook[token][price]; idx++; } ```
LowPotential Out-of-Gas ExceptionFixed
When a loop allows an arbitrary number of iterations or accesses state variables in its body, the function may run out of gas and revert the transaction. ```solidity=88 for(uint i = 0; i < _receiveInfos.length; i++) { ``` Function `AirDrop.airDropWemix` contains a loop and its loop condition depends on parameters: `receiveInfos`.
LowPotential Out-of-Gas ExceptionFixed
When a loop allows an arbitrary number of iterations or accesses state variables in its body, the function may run out of gas and revert the transaction. ```solidity=154 for (uint256 i = 0; i < rewardsInfo.length; ) { ``` Function `PoET.setRewards` contains a loop and its loop condition depends on parameters: `rewardsInfo`.
LowPotential Panic in Modulo OperationFixed
If `bInfo.round` equals 0, the `_initPeriodic()` function would panic, as modulo 0 causes panic as described in the doc: https://docs.soliditylang.org/en/v0.8.10/types.html#modulo
LowPotential Revert Due to Array Index Out of BoundFixed
The `tally()` function checks if an agenda has passed, and one requirement is that the current block time is greater than the agenda start time plus `aliveTime`. While there are 7 possible statuses for an agenda, there are only 2 subPolicy for each agenda. If the agenda status is not CONSENSUS or PROPOSAL, the variable `agenda.subPolicy[uint(agenda.status)-1]` would revert due to array index out of bound. ```solidity enum Status {EMERGENCY, CONSENSUS, PROPOSAL, DONE, REJECT, FAIL, WAIT} struct Agenda { uint startTime; // agenda consensus process start time uint permitedTime; // last permited time. created time => permit consensus step time => permit proposal step time Status status; // current status BasePolicy basePolicy; // basepolicy for agende SubPolicy[2] subPolicy; // subpolicy for agenda Info info; // agenda info VoteStatus[3] board; // vote status per steps, [EMERGENCY, CONSENSUS, PROPOSAL] } ```
LowPotential Underflow in the `getOraclePrice()` funFixed
In the `getOraclePrice()` function, if the decimal of the cToken is greater than 18, an underflow will occur.
LowPotential wrong calling methodFixed
Based on the modifier `onlyBurner` naming, the `onlyBurner` checks whether the `msg.sender` has privileges to burn tokens, but the method signature of the called _wDogma contract is `isMinter`.
Show 10 more
LowPotentially Dangerous AllowanceFixed
An unlimited allowance is generally considered ill-practice and increases security risks. ```solidity=357 if (IERC20(lp).allowance(address(this), psr) < amount) require(IERC20(lp).approve(psr, uint(-1))); ```
LowPotentially Dangerous AllowanceFixed
An unlimited allowance is generally considered ill-practice and increases security risks. ```solidity=329 require(IERC20(token).approve(wemixBridge, uint(-1))); ``` ```solidity=360 if (IERC20(lp).allowance(address(this), psr) < amount) require(IERC20(lp).approve(psr, uint(-1))); ``` ```solidity=395 require(IERC20(path[0]).approve(psr, uint(-1))); ```
LowPotentially Incorrect Decimal AssumptionFixed
The functions `nextPrice()` and `_isValidPrice()` assume that the token decimal is 18. However, it may have different decimals. For example, in Ethereum, USDC's decimals are 6.
LowPotentially Mint FanToken to Blacklisted AccountsFixed
According to the logic within the `_transfer()` function, we know that accounts on the blacklist are restricted from both sending and receiving `FanToken` assets. ```solidity=46 function _transfer( address from, address to, uint256 amount ) internal virtual override isNotBlackList(from) isNotBlackList(to) { super._transfer(from, to, amount); } ``` Nevertheless, blacklisted accounts can still receive newly minted `FanToken`. This occurs despite their inability to transfer assets, due to the lack of blacklist validation for the `to` argument in the `mint()` function. ```solidity=28 function mint(address to, uint256 value) external onlyMinter returns(bool){ super._mint(to, value); return true; } ```
LowRedundant `delete` StatementFixed
In EVM, the statement `delete var` indicates the assignment of the initial value for the variable `var`. However, employing the `delete var` statement is redundant if `var` already holds the default value for its type. After the execution of the statement at L299, `priceBook[token][price]` will be assigned the default value of uint, which is zero. However, the boolean expression at L298, preceding the statement at L299, validates that the value of `priceBook[token][price]` is already zero. So the statement at L299 is redundant. ```solidity=298 if (priceBook[token][price] == 0) { delete priceBook[token][price]; ledger.startPrice = price + _interval; ledger.length -= 1; } ```
LowRedundant CheckFixed
The `isOwner()` check in the function `isValidatorSig()` has already been performed in the function `isValidator()` and thus is redundant. ```solidity=28 function isValidator(address account) public view override returns (bool) { return (bearersMap[account] == true || isOwner()); } function isValidatorSig(bytes memory message, bytes memory signature) public view override returns (bool){ address recover = keccak256(message).recover(signature); return isValidator(recover) == true || isOwner(); } ```
LowRedundant CodeFixed
The checks in the linked lines are unnecessary because the required check has already been performed in the `repayBorrowFresh()` function.
LowRedundant Code ComponentsFixed
The linked statements do not affect the functionality of the codebase and appear to be either leftovers from test code or older functionality.
LowRedundant ERC721 import and inheritanceFixed
The contract imports and uses the "ERC721Holder", which enables the contract to receive NFT721 tokens. However, the contract doesn't provide a way for NFTs to be transferred out of the contract.
LowRedundant FanNFT Existence CheckFixed
The two state variables, `fanNFTMinted` and `fanNFTTokenMintedDate`, each capture mapping information between FanNFT and its corresponding creation year and month. Whenever a new FanNFT is minted, a corresponding entry is added to both variables (see code sections at L155-L160 and L198-L204). Consequently, if a FanNFT is recorded in `fanNFTTokenMintedDate`, it is guaranteed to be recorded in `fanNFTMinted` as well. ```solidity=155 fanNFTMinted[_year][_month].push(_tokenID); fanNFTTokenMintedDate[_tokenID] = MintedNFT({ year : _year, month : _month, creator : _creator }); ``` ```solidity=198 fanNFTMinted[_year][_month].push(currentFanTokenID); tokenIDs[i] = currentFanTokenID; fanNFTTokenMintedDate[currentFanTokenID] = MintedNFT({ year : _year, month : _month, creator : _creator }); ``` The boolean expression at line 144 returning true indicates that the specified FanNFT is already registered. Therefore, the statements at lines 148 to 151, which are also used to verify the existence of the given FanNFT, are redundant. ```solidity=144 require(fanNFTTokenMintedDate[_tokenID].year == 0, "FNM: already registered other date"); uint[] memory alreadyRegistered = fanNFTMinted[_year][_month]; require(alreadyRegistered.length + 1 <= monthMintLimit, "FNM: request amount limited"); for (uint i = 0; i < alreadyRegistered.length; i++) { require(alreadyRegistered[i] != _tokenID, "FNM: already registered NFT"); } ```
Show 10 more
LowRedundant FunctionFixed
The linked two functions have the same return value thus one of them is redundant. ```solidity= function getAllFanToken() external view returns (address[] memory fanTokens) { fanTokens = allFanTokens; } function getAllFanTokens() external view returns (address[] memory) { return allFanTokens; } ```
LowRedundant modifiersFixed
The modifiers `initializedMint()` and `onlyBreaker()` in the NightCrowTWAP contract are not used, rendering them redundant.
LowRedundant StatementsFixed
The function `swapExactTokensForTokens` stores the output of `IWeswapFactory(factory).getPair(path[0], path[1])` in the variable `pair`, then repeats the same call on the next line. ```solidity address pair = IWeswapFactory(factory).getPair(path[0], path[1]); TransferHelper.safeTransferFrom(path[0], msg.sender, IWeswapFactory(factory).getPair(path[0], path[1]), amounts[0]); ```
LowRewards Logic DiscussionFixed
We have the following observations on the `setRewards()` function: 1) Rewards can only be incremented, as shown in line 162. Once set, there is no way to remove the reward of an address unless the said address claim its reward. 2) The reward used in this function is `uint96`, while the `rewards` and `claimedAmounts` mappings store reward as `uint256`. 3) There is no check that the balance of the WEMIX token in the contract is sufficient for all the rewards being set. It is possible to set rewards, without users being able to claim their rewards due to insufficient WEMIX balance.
LowRole Management Mechanism InconsistencyFixed
In the `renounceRole()`, `grantRole()` and `revokeRole()` functions, there is no mechanism to check whether the `account` already has the role before the role is granted or revoked. This could lead to inconsistencies or even reversion.
LowSame event for different operationsFixed
The event `SetWemixDollarTreasury` is used in different set functions. Each of these functions changes a different state variable, which could potentially lead to confusion.
LowShould Not Allow to Liquidate If Price Oracle IsFixed
If the price oracle for the collateralized asset or lending asset is paused, the value of `shortfall` will be calculated based on a paused price oracle and still allow the liquidation to occur. This is not consistent with the design. ```solidity=404 require(!isOraclePaused[cTokenCollateral] || !isOraclePaused[cTokenBorrowed], "E97"); ```
LowState Variable ShadowingFixed
A state variable in a derived contract is shadowing a similarly named component in a parent contract. This means that when the derived contract accesses the state variable by its name, it will use the one defined in the derived contract, not the one in the parent contract. This can make the variable `_totalSupply` and the function `_beforeTokenTransfer()` redundant in `PlayNFT1155`. Variable `_totalSupply` in `PlayNFT1155` shadows the variable `_totalSupply` in `ERC1155Supply`. ```solidity=30 mapping(uint256 => uint256) private _totalSupply; ``` ```solidity=17 mapping(uint256 => uint256) private _totalSupply; ```
LowState Variable ShadowingFixed
A state variable in a derived contract is shadowing a similarly named component in a parent contract. This means that when the derived contract accesses the state variable by its name, it will use the one defined in the derived contract, not the one in the parent contract. This can lead to malfunction of contract. Variable `__threshold` in `BridgeV1AwayFactory` shadows the variable `__threshold` in `BridgeV1Validator`. ```solidity=20 uint private __threshold = 3; // Minimum number of validator signatures ``` ```solidity=17 uint private __threshold; // Minimum number of validator signatures ```
LowState Variable Should Be Declared ConstantFixed
State variables that never change should be declared as `constant` to save gas. ```solidity=18 bytes32 private FORCE_UNSTAKE = "forced-unstake-lockup"; // column of forced unstaking before cool-time ``` - `FORCE_UNSTAKE` should be declared `constant`.
Show 10 more
LowThe `exchange()` function does not allow users toFixed
The identified condition check only allows exchange when exchange amount is less than the user's balance. So an user can not exchange all his/her WRFT tokens to REFLECT tokens.
LowThe best practice for upgradeable contracts is noFixed
The current contract and the 'initialize' function only called the base `__ERC721_init` and `__Ownable_init` initialize function, but they are missing the calls to the extension's initialized functions, which are `__ERC721URIStorage_init()` and `__Ownable2Step_init()`. It is best practice to call parent initializations even if they are empty. If the contract is being upgraded to a new version and parent contracts have added some state variables that need to be initialized, it is safer for the upgrader to call these initialization functions. ```sol function initialize(address _accessController) initializer public { __ERC721_init("G-STAR 2023", "GSTAR"); __Ownable_init(); __ReentrancyGuard_init(); accessController = AccessController(_accessController); } ```
LowThe purpose of `transferToken()`Fixed
Users can use the `transferFrom` or `transfer` functions to transfer ERC20 tokens to another address, and they will only need to pay a gas fee if there are no transfer fees. Is there a reason for users to approve token to the `TokenBridgeTradeV2` contract and call the `transferToken()` function to pay the cost and transfer tokens to another address?
LowThe use of "onlyAdmin" for the "getNFTWallet" funFixed
The "getNFTWallet" function is a view function designed to retrieve the current "_NFTWallet." The use of "onlyAdmin" is useless because transaction information on the chain is public and the data is accessible to anyone. Additionally, the "onlyAdmin" restriction can be inconvenient for community members who should have transparent access to the "_NFTWallet" value. ```sol function getNFTWallet() external view onlyAdmin returns(address) { return _NFTWallet; } ```
LowToken transfer separated from the item transfer oFixed
For all of the `exchangeToxxx` functions, users need to transfer tokens into the contract, but they do not get the item within the same function. Our assumption is that the logic of user getting those items are in a separate function, which is not in the current audit scope. Separating these operations may have a broader attack surface like replay attack.
LowTyposFixed
There are several typos in the code base: - "Busniess" should be "Business" - "Policys" should be "Policies"
LowUnchecked ERC-20 `transfer()`/`transferFrom()` CaFixed
The return values of the `transfer()` and `transferFrom()` calls in the smart contract are not checked. Some ERC-20 tokens' transfer functions return no values, while others return a bool value, they should be handled with care. If a function returns `false` instead of reverting upon failure, an unchecked failed transfer could be mistakenly considered successful in the contract. ```solidity=66 IERC20(_token).transferFrom(treasury, _receiveInfos[i].to, _receiveInfos[i].amount); ```
LowUnchecked ERC-20 `transfer()`/`transferFrom()` CaFixed
The return value of the transfer()/transferFrom() call is not checked. ```solidity=193 pToken.transferFrom(msg.sender, address(this), amountInMax); ``` --- ```solidity=195 pToken.transfer(msg.sender, amountInMax.sub(expectAmount[0])); ```
LowUnchecked Value of ERC-20 `transfer()`/`transferFFixed
The linked `transfer()`/`transferFrom()` invocations do not check the return value of the function call, which should yield `true` in the case of a proper ERC-20 implementation. If transfer fails, the code should revert instead of return normally.
LowUnclear LogicFixed
The logic and intended design in the following code block is unclear. Line 272 checks that `bInfo.endTime` is greater than a certain time stamp. And then line 272 sets `bInfo.endTime` to equal to the above time stamp. 1) Even if `bInfo.endTime` is smaller than the calculated time stamp, line 273 would overwrite it, so the check is not necessary. 2) There is no potential underflow as the error message in line 272 indicates, because `currentTime - (currentTime % bInfo.round)` is guaranteed to be >= 0, and line 273 is not subtracting the right hand side of the equation off of the left hand side. ```solidity=272 require(bInfo.endTime >= currentTime - (currentTime % bInfo.round), "Periodic: underflow err"); bInfo.endTime = currentTime - (currentTime % bInfo.round); ```
Show 10 more
LowUnclear Purpose Of The `require` statementFixed
The following code checks the return value of the `writeChart` function call, however, the `writeChart` function in the `TokenExchangeV2.sol` file always returns true. The auditor would like to know the purpose of the require statement. ```sol = 359 require(TokenExchangeChartV2(CA(CHART_CONTRACT)).writeChart(_seed.token, _price, _payment, _amount), "X:chart"); ```
LowUnnecessary CheckFixed
In the `_vote()` function, the `status` variable is defined to equal to `_agendas[daoId][agendaId].status` in line 464-465, and as such the first condition in line 476 before && is always `true` and not necessary.
LowUnnecessary Code in The `onlyParameterSetter` ModFixed
Based on the implementation of modifier `onlyParameterSetter()`, `fanTokenRouter` contract has the ability of accessing the functions modified by modifier `onlyParameterSetter()`. However, in `FanTokenRouter` contract, we don't find any code logic for calling `FanTokenMinter` contract functions which are modified by modifier `onlyParameterSetter()`. **FanTokenMinter.sol** ```solidity=398 modifier onlyParameterSetter() { require(msg.sender == parameterSetter || msg.sender == fanTokenRouter, "FTM : Only Parameter setter can call"); _; } ```
LowUnnecessary Storage Read Access in For LoopFixed
The for loop contains repeated storage read access in the condition check. Given that the ending condition does not change in the for loop, the repeated storage read is unnecessary, and its associated high gas cost can be eliminated. ```solidity=459 for(uint i=0; i < totalStrategy.length; i++) { ``` Loop condition `i < totalStrategy.length` accesses the `length` field of a storage array. --- ```solidity=582 for(uint i=0; i<totalStrategy.length ; i++) { ``` Loop condition `i < totalStrategy.length` accesses the `length` field of a storage array. --- ```solidity=402 for(uint i=0; i < totalStrategy.length; i++) { ``` Loop condition `i < totalStrategy.length` accesses the `length` field of a storage array. --- ```solidity=568 for(uint i=0; i<totalStrategy.length ; i++) { ``` Loop condition `i < totalStrategy.length` accesses the `length` field of a storage array.
Lowunoptimized `for` loopFixed
The following for loop doesn't terminate the iteration when a matched condition is found. This can potentially waste gas. for(uint i = 0; i < set.length ; i++) { if(set[i].benefitType == Type.BenefitType.GALLERY) { gallery = set[i].benefit; } }
LowUnprotected InitializerFixed
One or more logic contracts do not protect their initializers. An attacker can call the initializer and assume ownership of the logic contract, whereby she can perform privileged operations that trick unsuspecting users into believing that she is the owner of the upgradeable contract. ```solidity=9 contract ExecuteManager is Ownable2StepUpgradeable, IExecuteManager { ``` - `ExecuteManager` is an upgradeable contract that does not protect its initializer. ```solidity=15 function initialize(address _roleManager) public initializer { ``` - `initialize` is an unprotected initializer function. --- ```solidity=10 contract RoleManager is IRoleManager, Ownable2StepUpgradeable, AccessControlUpgradeable { ``` - `RoleManager` is an upgradeable contract that does not protect its initializer. ```solidity=15 function initialize() public initializer() { ``` - `initialize` is an unprotected initializer function.
LowUnprotected InitializerFixed
One or more logic contracts do not protect their initializers. An attacker can call the initializer and assume ownership of the logic contract, whereby he can perform privileged operations that trick unsuspecting users into believing that he is the owner of the upgradeable contract. ```solidity=17 contract NonfungiblePositionHelper is ReentrancyGuardUpgradeable, ``` - `NonfungiblePositionHelper` is an upgradeable contract that does not protect its initializer. ```solidity=26 function initialize( ``` - `initialize` is an unprotected initializer function.
LowUnprotected InitializerFixed
One or more logic contracts do not protect their initializers. An attacker can call the initializer and assume ownership of the logic contract, whereby she can perform privileged operations that trick unsuspecting users into believing that she is the owner of the upgradeable contract. ```solidity=16 contract BenefitBox is IBenefitBox, Initializable, Ownable2StepUpgradeable { ``` - `BenefitBox` is an upgradeable contract that does not protect its initializer. ```solidity=36 function initialize(address _dynamicNFT, address _accessController) initializer public { ``` - `initialize` is an unprotected initializer function. --- ```solidity=11 contract AccessController is IAccessController, Ownable2StepUpgradeable, AccessControlUpgradeable { ``` - `AccessController` is an upgradeable contract that does not protect its initializer. ```solidity=22 function initialize() public initializer { ``` - `initialize` is an unprotected initializer function.
LowUnprotected InitializerFixed
One or more logic contracts do not protect their initializers. An attacker can call the initializer and assume ownership of the logic contract, whereby she can perform privileged operations that trick unsuspecting users into believing that she is the owner of the upgradeable contract. ```solidity=26 contract PlayMarketBase is ReentrancyGuard, AccessControlRoleUpgradeable{ ``` - `PlayMarketBase` is an upgradeable contract that does not protect its initializer. ```solidity=110 function init(address _tokenManager, address _wemixDollarAddress, address _priceOracle) external initializer { ``` - `init` is an unprotected initializer function. --- ```solidity=28 contract PlayMarketBaseV2 is ReentrancyGuard, AccessControlRoleUpgradeable{ ``` - `PlayMarketBaseV2` is an upgradeable contract that does not protect its initializer. ```solidity=119 function init(address _tokenManager, address _priceOracle) external initializer { ``` - `init` is an unprotected initializer function.
LowUnprotected InitializerFixed
The contract `AddressManager` does not protect their initializers. An attacker can call the initializer and assume ownership of the logic contract, whereby she can perform privileged operations that trick unsuspecting users into believing that she is the owner of the upgradeable contract. ```solidity=13 contract AddressManager is IAddressManager, Initializable { ``` - `AddressManager` is an upgradeable contract that does not protect its initializer. ```solidity=29 function initialize() public initializer { ``` - `initialize` is an unprotected initializer function.
Show 10 more
LowUnprotected InitializerFixed
The linked logic contracts do not protect their initializers. An attacker can call the initializer and assume ownership of the logic contract, whereby she can perform privileged operations that trick unsuspecting users into believing that she is the owner of the upgradeable contract.
LowUnsafe Integer CastFixed
Type casting refers to changing an variable of one data type into another. The code contains unsafe casts between integer types, which may result in unexpected truncation or sign flipping of the value. ```solidity=51 uint256 oraclePrice = uint256(IDataCollector(oracleInfo.priceOracle).latestAnswer()); ``` - The type conversion `uint256(IDataCollector(oracleInfo.priceOracle).latestAnswer())` from type int256 to type uint256 may flip the value's sign. --- ```solidity=63 uint8 fromTokenDecimals = uint8(_tokenToOracleInfo[fromToken].tokenDecimals); ``` - The type conversion `uint8(_tokenToOracleInfo[fromToken].tokenDecimals)` from type int8 to type uint8 may flip the value's sign. --- ```solidity=64 uint8 toTokenDecimals = uint8(_tokenToOracleInfo[toToken].tokenDecimals); ``` - The type conversion `uint8(_tokenToOracleInfo[toToken].tokenDecimals)` from type int8 to type uint8 may flip the value's sign. --- ```solidity=66 uint256 numerator = (fromAmount * fromPrice) * (10 ** uint8(toPriceDecimals)); ``` - The type conversion `uint8(toPriceDecimals)` from type int8 to type uint8 may flip the value's sign. --- ```solidity=67 uint256 denominator = toPrice * (10 ** uint8(fromPriceDecimals)); ``` - The type conversion `uint8(fromPriceDecimals)` from type int8 to type uint8 may flip the value's sign.
LowUnused `initialMint()` FunctionFixed
The comment at line 130 specifies that the initial mint amount should be `$1000`. Consequently, the `initialMintAmount` value needs to be set to `1000e18`. In the current implementation of the `initialMint()` function, the `initialMintAmount` value, passed in by its caller, is variable rather than constant. Nevertheless, we couldn't find the code calling the `initialMint()` function in `FanTokenRouter.sol`. **FanTokenMinter.sol** ```solidity=120 function initialMint( address from, uint initialMintAmount ) external onlyRouter whenNotPaused initializedMint { IERC20(WEMIX$).safeTransferFrom(from, fanTokenVault, initialMintAmount); //must initial mint amount is 1000$ _mintUserFanToken(fanTokenVault, initialMintAmount); initMint = true; emit InitMinted(from, fanTokenVault, initialMintAmount); } ```
LowUnused `onlyOwner` modifierFixed
The `onlyOwner` modifier in the VaultManagerDeployer` contract is declared but not used.
LowUnused ModifierFixed
The modifier `onlyMintBatcher` is declared but never used in the contract. ```solidity=339 modifier onlyMintBatcher() { require(ValidatorRole(fanTokenRouter).isValidator(msg.sender), "FTM : Only Mint Batcher can call"); _; } ```
LowUnused Return ValueFixed
The return value of an external call is not stored in a local or state variable. ```solidity=190 pToken.approve(address(_router), uint(-1)); ```
LowUnused State VariableFixed
Some state variables are not used in the codebase. This can lead to incomplete functionality or potential vulnerabilities if these variables are expected to be utilized. ```sol=36 uint256 public nextPositionId; // Incremental uint256 public nextCloseOrderId; // Incremental ```
LowUnused State VariableFixed
One or more state variables are never used in the codebase. Variable `_deadline` in `SpellBase` is never used in `SpellBase`. ```solidity=18 uint[] private _deadline; // idx = 0 => consensus, idx = 1 => proposal ``` ```solidity=11 abstract contract SpellBase is ISpell, Context, EditorRole { ``` --- Variable `_isValidToken` in `DAOToken` is never used in `DAOToken`. ```solidity=21 bool private _isValidToken = true; ``` ```solidity=11 contract DAOToken is ERC20, IDAOToken, ReentrancyGuard, IStationStruct, RouterRole { ```
LowUnused State VariableFixed
Variable `epochEndTimestamp` in `DataCollector` is never used in `DataCollector`. ```solidity=38 uint256 public epochEndTimestamp; ```
LowUnused state variablesFixed
The variable `_closedToken` is not used in `PlayMarketTokenManager`. This can lead to incomplete functionality or potential vulnerabilities if these variables are expected to be utilized.
Show 10 more
LowUpdate `quorum` is redundant in `removeOwner()`Fixed
In the function `removeOwner`, it is redundant to check `quorum > owners.length` as the modifier `validRequirement` would enforce the `quorum` is less or equal to `owners.length`.
LowUpgradeable Contract Extends Non-Upgradeable ContFixed
Upgradeable contracts should inherit upgradeable contracts. The upgradeable contracts `PlayMarketBase` and `PlayMarketBaseV2` inherit the non-upgradable contract `ReentrancyGuard`. All upgradeable contracts in an inheritance chain must follow the same rules for writing upgradeable contracts. See https://docs.openzeppelin.com/upgrades-plugins/1.x/writing-upgradeable#use-upgradeable-libraries.
LowUpgradeable Contract Extends Non-Upgradeable ContFixed
Upgradeable contract must extend upgradeable contracts and call initializer functions of parent-contracts in its own initializer function. Otherwise it might malfunction due to improper initialization or improper upgrading. The following upgradeable contracts extend non-upgradeable contracts: - Admin - LoanAuctionBase
LowUsage of `transfer()` for Sending EtherFixed
It is not recommended to use Solidity's `transfer()` functions for transferring Ether, since some contracts may not be able to receive the funds. Those functions forward only a fixed amount of gas (2300 specifically) and the receiving contracts may run out of gas before finishing the transfer. Also, EVM instructions' gas costs may increase in the future. Thus, some contracts that can receive now may stop working in the future due to the gas limitation. ```solidity=553 recipient.transfer(amount); ``` - `LoanAuctionBase._transferFTOut()` uses `transfer()`.
LowUsage of `transfer`/`send` for sending EtherFixed
It is not recommended to use Solidity's `transfer()` and `send()` functions for transferring Ether, since some contracts may not be able to receive the funds. Those functions forward only a fixed amount of gas (2300 specifically) and the receiving contracts may run out of gas before finishing the transfer. Also, EVM instructions' gas costs may increase in the future. Thus, some contracts that can receive now may stop working in the future due to the gas limitation. ```solidity=344 payable(user).transfer(amount); ``` - `ReOpen._cancel` uses `transfer()`. --- ```solidity=521 payable(user).transfer(amount); ``` - `Station._cancel` uses `transfer()`.
LowUsage of `transfer`/`send` for Sending Native TokFixed
It is not recommended to use Solidity's `transfer()` and `send()` functions for transferring native tokens, since some contracts may not be able to receive the funds. Those functions forward only a fixed amount of gas (2300 specifically) and the receiving contracts may run out of gas before finishing the transfer. Also, EVM instructions' gas costs may increase in the future. Thus, some contracts that can receive now may stop working in the future due to the gas limitation. ```solidity=28 msg.sender.transfer(wad); ``` - `WWEMIX_.withdraw` uses `transfer()`.
LowUsing storage variables after deletionFixed
The storage variable **position** is still used after it is deleted. Leading to the `poolId` used in L676 being zero but not the real pool id for the given position. ```solidity=676 uint80 poolId = position.poolId; ```
LowValidate price in the `nextPrice()` functionFixed
A valid next auction price is calculated by the `nextPrice()` function based on the given price and validated by the `_isValidPrice()` function. Users call the public function `nextPrice()` to obtain the next price. However, the price returned by the `nextPrice()` function is not validated. As a result, the transaction could potentially be reverted if users use the `nextPrice()` function calculated price to bid.
LowVariables That Could Be Declared as ImmutableFixed
The linked variables assigned in the constructor can be declared as `immutable`. Immutable state variables can be assigned during contract creation but will remain constant throughout the lifetime of a deployed contract. A big advantage of immutable variables is that reading them is significantly cheaper than reading from regular state variables since they will not be stored in storage.
LowVariables That Could Be Declared as ImmutableFixed
The linked variables assigned in the constructor can be declared as `immutable`. Immutable state variables can be assigned during contract creation but will remain constant throughout the lifetime of a deployed contract. A big advantage of immutable variables is that reading them is significantly cheaper than reading from regular state variables since they will not be stored in storage.
Show 2 more
LowVariables That Could Be Declared as ImmutableFixed
The linked variables assigned in the constructor can be declared as `immutable`. Immutable state variables can be assigned during contract creation but will remain constant throughout the lifetime of a deployed contract. A big advantage of immutable variables is that reading them is significantly cheaper than reading from regular state variables since they will not be stored in storage.
LowVariables That Could Be Declared as ImmutableFixed
The linked variables assigned in the constructor can be declared as `immutable`. Immutable state variables can be assigned during contract creation but will remain constant throughout the lifetime of a deployed contract. A big advantage of immutable variables is that reading them is significantly cheaper than reading from regular state variables since they will not be stored in storage.
Contracts
No deployed address is recorded for these audits.
The reports name a repository or files. Open a report to see its scope.
Show this on your site
Is this your project? Show this record on your own website. The label states what the record holds, links to this page and updates by itself. It is free and needs no sign-in.
Content
Automatic: the three strongest facts of this project’s own record (its last audit and two more), highlighted. They update with the record. Picking a fact turns Automatic off.
Style
Format
<head><script src="https://widget.trustblock.run/project-widget.min.js" defer></script></head><body><divdata-tb-project-widgetdata-tb-project-slug="wemix"></div></body>
Paste the code where the label should appear. It uses no API credit and updates by itself.
Something wrong on this page? A missing report, a wrong date, a finding that was fixed since. Tell us and we check it against its source.
Tell us