2

If you check this link: openzeppelin github access control you will notice they use 2 function for granting a role:

function grantRole(bytes32 role, address account) public virtual override onlyRole(getRoleAdmin(role)) {
        _grantRole(role, account);
    }

This function is external. All what it does it calling the internal function _grantRole. Note they add the modifier onlyRole which means if you dont have the right permissions (role), then the call will fail.

However, this is the internal function:

function _grantRole(bytes32 role, address account) internal virtual {
        if (!hasRole(role, account)) {
            _roles[role].members[account] = true;
            emit RoleGranted(role, account, _msgSender());
        }
    }

note that the internal function does not have the modifier onlyRole which means it does not do permission checking. As an internal function, I can write a contract and inherit this contract from openzeppelin and since my contract is derived contract, I have access to the internal functions, and I can easily write a function that call the internal function "_grantRole". Why that wont work? I dont understand the security behind this since only the external functions check for the role, but the internal ones dont.

Any contract can inherit this and call the internal functions. Would someone please explain!

Improve this question

1 Answer 1

Reset to default
2

This is by design.

OpenZeppelin contracts are meant to be inherited and easy to use for different scenarios. They often offer this kind of default external or public function, which is secure, but they also offer the possibility to write your own logic around the internal function.

So you can create for example create your own function grantRole2 which does something else, but inside it you call also _grantRole. This way you can change/extend the default external functionality.

If you are inheriting OpenZeppelin contracts you have free access to ruin the contracts' security as you wish. The default exposed functions are secure, but they allow you to also go around the default functionality.

It's your responsibility to make sure the external/public functions you write are secure.

Improve this answer
2
  • Thank you so much for the explanation. I understand that this way allows us to reuse the internal functions which is great. However, by inheriting this contract and calling the internal function (me without having the right permission) and add myself as admin is a huge security gap. So the only way to secure my contract is override all the internal functions and add the modifier onlyRole to them? Thanks again!
    – Crypto Man
    Mar 12 at 13:40
  • Internal functions are not callable from outside. Keeping the default functionality doesn't expose any security threats. Only if you implement your own public functions which bypass the OZ's security (by calling the internal functions directly) you may open new security holes
    – Lauri Peltonen
    Mar 12 at 14:59

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.