Pieces Protocol
First Flight #32
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Missing access control in `ERC20ToGenerateNftFraccion::mint`, leading to unauthorized minting of ERC20 tokens

marcoroccon

Description:
Once the NFT has been divided in multiple fractions, represented by a given amount of ERC20 (which should be fixed), an attacker could call the ERC20ToGenerateNftFraccion::mint function to generate an arbitrary amount of that specific ERC20 token.

Impact:
The unauthorized minting of ERC20 tokens causes an increase in the amount of "pieces" of a particular NFT available in the market, therefore leading to a break in the protocol core logic. Even though the state variable in TokenDivider are not affected by this unauthorized minting, the user should not be able to mint ERC20 tokens outside the scope of TokenDivider.

Tools Used:
Manual review

Proof of Concept:
Add the following test in TokenDividerTest.t.sol. This test demonstrates how the user can actually mint more ERC20 tokens than the amount established when calling the TokenDivider::divideNft function. The test is an expansion of the already existing TokenDividerTest::testDivideNft test.

function testUnauthorizedMintingOfERC20Tokens() public {
vm.startPrank(USER);
erc721Mock.approve(address(tokenDivider), TOKEN_ID);
​
tokenDivider.divideNft(address(erc721Mock), TOKEN_ID, AMOUNT);
vm.stopPrank();
ERC20Mock erc20Mock = ERC20Mock(tokenDivider.getErc20InfoFromNft(address(erc721Mock)).erc20Address);
​
console.log("ERC20 Token name is: ", erc20Mock.name());
console.log("ERC20 Token symbol is: ", erc20Mock.symbol());
assertEq(tokenDivider.getErc20TotalMintedAmount(address(erc20Mock)), AMOUNT);
assertEq(erc721Mock.ownerOf(TOKEN_ID), address(tokenDivider));
assertEq(tokenDivider.getBalanceOf(USER, address(erc20Mock)), AMOUNT);
​
uint256 erc20BalanceBeforeUnauthorizedMinting = erc20Mock.balanceOf(USER);
uint256 unauthorizedERC20Amount = 1e18;
​
vm.prank(USER);
erc20Mock.mint(USER, unauthorizedERC20Amount);
​
uint256 erc20BalanceAfterUnauthorizedMinting = erc20Mock.balanceOf(USER);
​
assert(erc20BalanceBeforeUnauthorizedMinting != erc20BalanceAfterUnauthorizedMinting);
assertEq(erc20BalanceAfterUnauthorizedMinting, erc20BalanceBeforeUnauthorizedMinting + unauthorizedERC20Amount);
}

Recommended Mitigation:
It is recommended to limit the minting of new ERC20 tokens to TokenDivider contract, which should be intended as the only authorized minter for those tokens. This can be achieved by creating a modifier that checks if the msg.sender that calls the ERC20ToGenerateNftFraccion::mint function is the TokenDivider contract. Otherwise it will revert with a custom error.

Below is the code implementation to be added in the ERC20ToGenerateNftFraccion::mint, including the _tokenDivider parameter to be passed in the constructor.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.18;
​
import {ERC20Burnable} from "@openzeppelin/contracts/token/ERC20/extensions/ERC20Burnable.sol";
import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
+ import {TokenDivider} from "../TokenDivider.sol";
​
​
contract ERC20ToGenerateNftFraccion is ERC20, ERC20Burnable {
+ error ERC20ToGenerateNftFraccion__UnauthorizedMinter();
​
+ TokenDivider tokenDivider;
​
+ modifier onlyTokenDivider {
+ if(msg.sender != address(tokenDivider)) {
+ revert ERC20ToGenerateNftFraccion__UnauthorizedMinter();
+ }
+ _;
+ }
​
- constructor(string memory _name, string memory _symbol) ERC20(_name, _symbol) {
-
- }
​
+ constructor(string memory _name, string memory _symbol, address _tokenDivider) ERC20(_name, _symbol) {
+ tokenDivider = TokenDivider(_tokenDivider);
+ }
​
- function mint(address _to, uint256 _amount) external onlyTokenDivider {
- _mint(_to, _amount);
- }
​
+ function mint(address _to, uint256 _amount) external onlyTokenDivider {
+ _mint(_to, _amount);
+ }
}

The correct implementation can be checked by running the testUnauthorizedMintingOfERC20Tokens test again. This time the test should fail with the ERC20ToGenerateNftFraccion__UnauthorizedMinter custom error.

Updates

Lead Judging Commences

fishy Lead Judge 5 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Lack of token access control chekcs

Any person can mint the ERC20 token generated in representation of the NFT

Support

FAQs

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