ApeIron - Token Contract

CertiK11 Oct 2022Files named in the report

CertiK reviewed Apeiron (ApeIron - Token Contract) on 11 Oct 2022: 34 findings, 3 critical; 20 marked fixed, 11 acknowledged, 3 open.

2 open critical findings

Open the report Open Apeiron’s security record

Report hosted on skynet.certik.com.

Trustblock does not review code.

Findings34

Statuses are the security firm’s own, from its report.

Severity	Open	Acknowledged	Fixed
Critical	2	0	1
High	0	11	3
Medium	0	0	1
Low	1	0	15

CriticalA Winner Can Lock All TreasuresFixed

If a winner does not have enough keys required, then the function `stakingAndKeys.burnUserKeys()` will be reverted because of underflow. Hence admins will be unable to assign treasures to all winners.

HighCentralization Risks in ApeironGodiverseCollectioFixed

In the contract `ApeironGodiverseCollection` the role `_owner` has authority over the function `setURI` as shown in the diagram below. Any compromise to the `_owner` account may allow the hacker to take advantage of this authority and set the new URI. ![](https://accelerator-tasks-prod.acc.corp.certik.com/73a064cf24fc4fe3a864a40de6cd99f6/diagrams/centralization_ApeironGodiverseCollection-ApeironGodiverseCollection-_owner.svg)

HighCentralization Risks in MockERC1155.solFixed

In the contract `MockERC1155` the role `_owner` has authority over the function `setURI` as shown in the diagram below. Any compromise to the `_owner` account may allow the hacker to take advantage of this authority and set the new URI. ![](https://accelerator-tasks-prod.acc.corp.certik.com/73a064cf24fc4fe3a864a40de6cd99f6/diagrams/centralization_MockERC1155-MockERC1155-_owner.svg)

HighUsers Can Join An Expedition Right Before It EndsFixed

Users can join an expedition right before it ends, which means users can join an expedition by staking assets only for a couple of minutes. Attack Flow: 1. call the function stakeNFTBeforeExpeditionEnd() and joinExpedition() right before the expedition ends. 2. call the function unstakeNFTAfterExpeditionEnd() to unstake NFT.

MediumMissing Zero Amount CheckFixed

1. Missing zero amount check on `Asset.amount`, so the function getStakedAssets() will return assets that have been unstaked. 2. 3. Missing zero amount check on `_amounts`, so attackers can add any assets into the `stakedAssets` mapping. 4. Missing zero amount check on `_amounts[i]` so an unstaked asset is still in the `stakedAssets` mapping.

LowCombining For LoopsFixed

The following two for loops ```solidity for (uint i = 0; i < _tokenIds.length; i++) { bool found = false; for (uint j = 0; j < stakedAssets[_stakeholder].length; j++) { if ( stakedAssets[_stakeholder][j].addr == _assetAddress && stakedAssets[_stakeholder][j].tokenId == _tokenIds[i] && stakedAssets[_stakeholder][j].amount >= _amounts[i] ) { found = true; } } require(found, "Not staked"); } //deduce the asset from the stakedAssets for (uint i = 0; i < _tokenIds.length; i++) { for (uint j = 0; j < stakedAssets[_stakeholder].length; j++) { if ( stakedAssets[_stakeholder][j].addr == _assetAddress && stakedAssets[_stakeholder][j].tokenId == _tokenIds[i] ) { stakedAssets[_stakeholder][j].amount -= _amounts[i]; break; } } } ``` can be combined into the follwong one ```solidity uint counter = 0; for (uint i = 0; i < _tokenIds.length; i++) { for (uint j = 0; j < stakedAssets[_stakeholder].length; j++) { if ( stakedAssets[_stakeholder][j].addr == _assetAddress && stakedAssets[_stakeholder][j].tokenId == _tokenIds[i] ) { counter += 1; stakedAssets[_stakeholder][j].amount -= _amounts[i]; break; } } } require(counter == _tokenIds.length, "Not staked"); ```

LowFunction Should Be Declared ExternalFixed

The functions which are never called internally within the contract should have external visibility for gas optimization. ```solidity=20 function pause() public onlyOwner { ``` --- ```solidity=24 function unpause() public onlyOwner { ``` --- ```solidity=28 function mint(address to, uint256 amount) public onlyAdmin { ``` --- ```solidity=30 function mint(address user, uint256 amount) ``` --- ```solidity=37 function pause() public onlyOwner { ``` --- ```solidity=41 function unpause() public onlyOwner { ``` --- ```solidity=36 function pause() public onlyOwner { ``` --- ```solidity=40 function unpause() public onlyOwner { ``` --- ```solidity=32 function initialize() public initializer { ``` --- ```solidity=30 function mint(address user, uint256 amount) ``` --- ```solidity=37 function pause() public onlyOwner { ``` --- ```solidity=41 function unpause() public onlyOwner { ``` --- ```solidity=36 function pause() public onlyOwner { ``` --- ```solidity=40 function unpause() public onlyOwner { ``` --- ```solidity=18 function setURI(string memory newuri) public onlyOwner { ``` --- ```solidity=90 function initialize() public initializer { ``` --- ```solidity=312 function getTreasureById(uint256 _treasureId) ``` --- ```solidity=323 function getUserTreasureIds(address _user) ``` --- ```solidity=96 function initialize() public initializer { ``` --- ```solidity=143 function initialize() public initializer { ``` --- ```solidity=12 function setURI(string memory newuri) public onlyOwner { ``` --- ```solidity=16 function mint( ``` --- ```solidity=25 function mintBatch( ``` --- ```solidity=10 function mint(address to, uint256 amount) public onlyAdmin { ``` --- ```solidity=16 function safeMint(address to) public onlyAdmin { ``` --- ```solidity=7 function testToWritePrizePerTreasureId(Treasure memory _treasure) public { ``` --- ```solidity=7 function testToWriteAssetTypes(uint256 assetType) public { ``` --- ```solidity=7 function testToWriteUserExpeditionIds(uint256[] memory ids) public { ``` --- ```solidity=29 function isAdmin(address admin) public view returns (bool) { ``` --- ```solidity=29 function isAdmin(address admin) public view returns (bool) { ```

LowImproper Usage of `public` and `external` TypeFixed

`public` functions that are never called by the contract could be declared as `external`. `external` functions are more efficient than `public` functions.

LowLack of Check For Start TimeFixed

There is no check that the beginning or end of an expedition is set in the past.

LowLack of Specified Rate Range RestrictionFixed

The `owner` of the contract has permission to modify the fees without limitation and the exchange rate can change. Therefore, in the extreme case, that fee could be a very large amount of value, which might cause unexpected loss to the project and users.

Show 10 more

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 Zero Address ValidationFixed

Addresses should be checked before assignment or external call to make sure they are not zero addresses. File: ExpeditionStakingAndKeys.sol (Line 431, Function `setupExchangeConfig`) ```sol exchangeToken = IERC20(_exchangeTokenAddress); ``` - `_exchangeTokenAddress` 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. File: NebulaExpedition.sol (Line 162, Function `setStakingAndKeys`) ```sol stakingAndKeysAddress = _address; ``` - `_address` is not zero-checked before being used. --- File: ExpeditionPrizes.sol (Line 148, Function `setExpedition`) ```sol expeditionAddress = _address; ``` - `_address` is not zero-checked before being used.

LowPotential Reentrancy Attack (Events)Fixed

*This finding has a minor impact because the reentrancy only causes out-of-order events.* 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=136 aprsToken.transfer(msg.sender, releasable); ``` ### Events emitted after the call(s) ```solidity=137 emit VestingReleased(msg.sender, releasable); ```

LowRedundant Import StatementFixed

The last 4 files, ERC20, ERC20Burnable, Pausable, and ownable, have been imported a few lines above.

LowTypo in CommentsFixed

`NebulaExpedition` is written as `NubulaExpedition`.

LowUnchecked ERC-20 `transfer()`/`transferFrom()` CaFixed

The return value of the transfer()/transferFrom() call is not checked. ```solidity=136 aprsToken.transfer(msg.sender, releasable); ```

LowUnnecessary ChecksFixed

The check ```solidity require(erc20.allowance(_stakeholder, address(this)) >=_amounts[i] && erc20.balanceOf(_stakeholder) >= _amounts[I], "Not enough allowance or balance for tokens"); ``` is redundant because it happens in ERC20.sol. ``` solidity function _transfer( address from, address to, uint256 amount ) internal virtual { require(from != address(0), "ERC20: transfer from the zero address"); require(to != address(0), "ERC20: transfer to the zero address"); _beforeTokenTransfer(from, to, amount); uint256 fromBalance = _balances[from]; require(fromBalance >= amount, "ERC20: transfer amount exceeds balance"); // (HERE!!!) unchecked { _balances[from] = fromBalance - amount; } _balances[to] += amount; emit Transfer(from, to, amount); _afterTokenTransfer(from, to, amount); } ``` The checks ```solidity require(erc721.isApprovedForAll(_stakeholder, address(this)),"Not approved"); require(erc721.ownerOf(_tokenIds[i]) == _stakeholder); ``` are redundant because they happen in ERC721.sol. ```solidity function safeTransferFrom( address from, address to, uint256 tokenId, bytes memory data ) public virtual override { require(_isApprovedOrOwner(_msgSender(), tokenId), "ERC721: caller is not token owner nor approved"); // (HERE!!!) _safeTransfer(from, to, tokenId, data); } function _isApprovedOrOwner(address spender, uint256 tokenId) internal view virtual returns (bool) { address owner = ERC721.ownerOf(tokenId); return (spender == owner || isApprovedForAll(owner, spender) || getApproved(tokenId) == spender); // (HERE!!!) } function _transfer( address from, address to, uint256 tokenId ) internal virtual { require(ERC721.ownerOf(tokenId) == from, "ERC721: transfer from incorrect owner"); // (HERE!!!) require(to != address(0), "ERC721: transfer to the zero address"); _beforeTokenTransfer(from, to, tokenId); // Clear approvals from the previous owner _approve(address(0), tokenId); _balances[from] -= 1; _balances[to] += 1; _owners[tokenId] = to; emit Transfer(from, to, tokenId); _afterTokenTransfer(from, to, tokenId); } ``` The checks ```solidity require(erc1155.isApprovedForAll(_stakeholder, address(this)), "Not approved"); require(erc1155.balanceOf(_stakeholder, _tokenIds[i]) >= _amounts[i], "Not enough balance"); ``` are redundant because they happen in ERC1155.sol. ```solidity function safeBatchTransferFrom( address from, address to, uint256[] memory ids, uint256[] memory amounts, bytes memory data ) public virtual override { require( from == _msgSender() || isApprovedForAll(from, _msgSender()), // (HERE!!!) "ERC1155: caller is not token owner nor approved" ); _safeBatchTransferFrom(from, to, ids, amounts, data); } function _safeBatchTransferFrom( address from, address to, uint256[] memory ids, uint256[] memory amounts, bytes memory data ) internal virtual { require(ids.length == amounts.length, "ERC1155: ids and amounts length mismatch"); require(to != address(0), "ERC1155: transfer to the zero address"); address operator = _msgSender(); _beforeTokenTransfer(operator, from, to, ids, amounts, data); for (uint256 i = 0; i < ids.length; ++i) { uint256 id = ids[i]; uint256 amount = amounts[i]; uint256 fromBalance = _balances[id][from]; require(fromBalance >= amount, "ERC1155: insufficient balance for transfer"); // (HERE!!!) unchecked { _balances[id][from] = fromBalance - amount; } _balances[id][to] += amount; } emit TransferBatch(operator, from, to, ids, amounts); _afterTokenTransfer(operator, from, to, ids, amounts, data); _doSafeBatchTransferAcceptanceCheck(operator, from, to, ids, amounts, data); } ```

LowUse Enum Data StructureFixed

Enums allow you to define a type by enumerating its possible values.

LowUser-Defined GettersFixed

The linked functions are equivalent to the compiler-generated getter functions for the respective variables.