OpenZeppelin / OpenZeppelin/openzeppelin-contracts

AccessControl uses wrong value for address parameter in authorization check

Open
#4,747 3 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Solidity
Stars
27.2k
Forks
12.4k
Avg merge
2d 19h
Merged PRs (30d)
33

Description

If an ERC20 token contract that inherits ERC2771Context has this function:

    function mint(address to, uint256 amount) public onlyRole(MINTER_ROLE) {
        _mint(to, amount);
    }

which is called like this:

  await token.mint(signer.address, 6)

then this error message appears:

AccessControlUnauthorizedAccount("0x0000000000000000000000000000000000000006", "0x9f2df0fed2c77648de5860a4cc508cd0818c85b8b8a1ab4ceeef8d981c8956a6")'

💻 Environment
"@openzeppelin/contracts": "^5.0.0",
"hardhat": "^2.19.1",

📝 Details

AccessControl mistakenly takes the amount value as the address for checking authorization.

🔢 Code to reproduce bug

Token contract code:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.23;

import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import "@openzeppelin/contracts/access/AccessControl.sol";
import "@openzeppelin/contracts/metatx/ERC2771Context.sol";

contract TokenAccessControlWithERC2771 is
    ERC20,
    AccessControl,
    ERC2771Context
{
    bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE");

    constructor(
        address adminAddress
    ) ERC20("TEST", "TEST") ERC2771Context(0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266) {
        _grantRole(DEFAULT_ADMIN_ROLE, adminAddress);
        _grantRole(MINTER_ROLE, adminAddress);
    }

    function mint(address to, uint256 amount) public onlyRole(MINTER_ROLE) {
        _mint(to, amount);
    }

    function _msgSender()
        internal
        view
        override(Context, ERC2771Context)
        returns (address)
    {
        return super._msgSender();
    }

    function _msgData()
        internal
        view
        override(Context, ERC2771Context)
        returns (bytes calldata)
    {
        return super._msgData();
    }

}

Deployment and test script for hardhat:

import { ethers } from "hardhat"

async function main() {
	const [signer] = await ethers.getSigners()

	const token = await ethers.deployContract("TokenAccessControlWithERC2771", [signer.address])
	await token.waitForDeployment()
	await token.mint(signer.address, 6)
}

main().catch(error => {
  console.error(error)
  process.exitCode = 1
})

Dependencies:

    "@nomicfoundation/ethereumjs-trie": "^6.0.3",
    "@nomicfoundation/ethereumjs-util": "^9.0.3",
    "@nomicfoundation/hardhat-chai-matchers": "^2.0.2",
    "@nomicfoundation/hardhat-ethers": "^3.0.5",
    "@nomicfoundation/hardhat-toolbox": "^4.0.0",
    "@nomicfoundation/hardhat-verify": "^2.0.1",
    "@openzeppelin/contracts": "^5.0.0",
    "@typechain/ethers-v6": "^0.5.1",
    "@typechain/hardhat": "^9.1.0",
    "axios": "^1.6.2",
    "chai": "^4.3.10",
    "ethers": "^6.8.1",
    "hardhat": "^2.19.1",
    "hardhat-gas-reporter": "^1.0.9",
    "lodash": "^4.17.21",
    "solidity-coverage": "^0.8.5",
    "ts-node": "^10.9.1",
    "typechain": "^8.3.2",
    "typescript": "^5.2.2"

Command line: yarn hardhat run --network hardhat scripts\deploy.ts

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reproducing the failure with the TokenAccessControlWithERC2771 contract and Hardhat script shown in the issue. Read the AccessControl and ERC2771Context interaction, then verify the authorization check uses the intended caller value and that the mint call succeeds with the expected error behavior covered by a regression test.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, solidity
Domain
blockchain, security
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.