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

Role checks in `burn` and `burnBatch`

hackz

Summary

The burn and burnBatch functions in OWPIdentity.sol restrict burning permissions to accounts with the MINTER_ROLE. This configuration prevents token holders from burning their tokens unless they have the MINTER_ROLE, which may lead to user dissatisfaction or loss of intended functionality.

Finding Description

The burn and burnBatch functions override the burnable functionality from OpenZeppelin’s ERC1155Burnable extension. However, instead of allowing any owner or approved account to burn tokens, the current implementation restricts this action to accounts with the MINTER_ROLE. This restrictive access control limits users' ability to manage their own tokens, which deviates from the expected behavior in token burn functionality. This configuration could frustrate users or potentially result in tokens being permanently locked.

This issue does not directly break security guarantees but restricts flexibility and usability. If the intent was to allow only specific roles to burn, this design is appropriate. However, if the intent was to give token holders burn access, this implementation falls short.

Vulnerability Details

The burn and burnBatch functions in OWPIdentity.sol override OpenZeppelin's ERC1155Burnable functions, which typically allow the token owner or an approved account to burn tokens. Due to the onlyRole(MINTER_ROLE) modifier, only accounts with the MINTER_ROLE can execute these functions. This implementation breaks the usual expected behavior of burnable tokens, leading to potential usability issues.

Impact

Impact assessment: Medium
The impact is categorized as medium because this issue does not compromise security but affects usability and could potentially lead to dissatisfaction among users who expect to have control over their tokens. Locked tokens may also affect the perceived value and utility of the tokens in circulation.

Proof of Concept

The following functions only permit accounts with MINTER_ROLE to burn tokens:

function burn(address account, uint256 id, uint256 amount)
public override
onlyRole(MINTER_ROLE)
{
_burn(account, id, amount);
}
​
function burnBatch(address to, uint256[] memory ids, uint256[] memory amounts)
public override
onlyRole(MINTER_ROLE)
{
_burnBatch(to, ids, amounts);
}

Attempting to burn tokens without the MINTER_ROLE will revert with an access control error.

Recommendations

To allow token holders to burn their tokens without needing the MINTER_ROLE, remove the onlyRole(MINTER_ROLE) modifier from burn and burnBatch. Here is an updated code snippet:

function burn(address account, uint256 id, uint256 amount)
public override
{
require(
account == _msgSender() || isApprovedForAll(account, _msgSender()),
"ERC1155: caller is not owner nor approved"
);
_burn(account, id, amount);
}
​
function burnBatch(address to, uint256[] memory ids, uint256[] memory amounts)
public override
{
require(
to == _msgSender() || isApprovedForAll(to, _msgSender()),
"ERC1155: caller is not owner nor approved"
);
_burnBatch(to, ids, amounts);
}

This fix ensures that token holders or approved accounts can burn their tokens, aligning with typical ERC1155 burnable token behavior.

File Location: OWPIdentity.sol

Updates

Lead Judging Commences

0xbrivan2 Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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