Project
One World
NFTDeFi
15,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

Function MembershipERC1155::mint is susceptible to reentrance manipulation when it calls onERC1155Received()

sala1

Summary

The mint function inherits and uses ERC1155Upgradeable::_mint() which has a protection against zero address calls, however, it requires that if the caller is a smart contract, it must implement {IERC1155Receiver-onERC1155Received}. In that external call and inside the onERC1155Received(), reentrance attack can be introduced. Below are mint() code as well as ERC1155Upgradeable::_mint()

Vulnerability Details

reentrance attack can be used when mint() calls to verify that "to" address is smart contract and inside the onERC1155Received(). This happened in the documented attack against "HypeBears NFT contract".

https://blocksecteam.medium.com/when-safemint-becomes-unsafe-lessons-from-the-hypebears-security-incident-2965209bda2a

Can also be problem of future exploitation of same external call(onERC1155Received()).

Note: The access control on the mint() can't help against this since, this is mint() legitimately calling external contract to verify address as stated above.

/// @notice Mint a new token
/// @param to The address to mint tokens to
/// @param tokenId The token ID to mint
/// @param amount The amount of tokens to mint
function mint(address to, uint256 tokenId, uint256 amount) external override onlyRole(OWP_FACTORY_ROLE) {
totalSupply += amount * 2 ** (6 - tokenId); // Update total supply with weight
_mint(to, tokenId, amount, "");
}
​
​
https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable/blob/master/contracts/token/ERC1155/ERC1155Upgradeable.sol
​
/**
* @dev Creates a `value` amount of tokens of type `id`, and assigns them to `to`.
*
* Emits a {TransferSingle} event.
*
* Requirements:
*
* - `to` cannot be the zero address.
* - If `to` refers to a smart contract, it must implement {IERC1155Receiver-onERC1155Received} and return the
* acceptance magic value.
*/
function _mint(address to, uint256 id, uint256 value, bytes memory data) internal {
if (to == address(0)) {
revert ERC1155InvalidReceiver(address(0));
}
(uint256[] memory ids, uint256[] memory values) = _asSingletonArrays(id, value);
_updateWithAcceptanceCheck(address(0), to, ids, values, data);
}
​
​

Impact

Manipulation of mint logic and future issues that can be introduced by reentrance attack() as the case linked above.
For example an attacker can setup his/her onERC1155Received() function and include logic to manipulate / call back the contract as below.

function onERC1155Received(address, address, uint256, uint256, bytes memory) public virtual returns (bytes4) {
ox233fddi799dfdfdfdf.0x6444455(raw data); //address.mintAddress(raw data)
return this.onERC1155Received.selector;
}

Tools Used

Manual review

Recommendations

Add reentrance guard modifier on mint or add additional if condition.

Inherit from ReentrancyGuard, then use nonReentrant modifier on mint() function.

Updates

Lead Judging Commences

0xbrivan2 Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
0xbrivan2 Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.