Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consensus protocol upgrade to fix excessive restrictions of eosio::linkauth #6672

Closed
arhag opened this issue Jan 27, 2019 · 2 comments

Comments

Projects
None yet
1 participant
@arhag
Copy link
Contributor

commented Jan 27, 2019

Background

To preserve the security properties of five special native actions, they cannot be linked to a custom minimum permission. This code is enforced in authorization_manager::check_linkauth_authorization by blacklisting the special actions by name. Unfortunately, there is a bug in that code which causes it to only consider the name of the action and not also require that the code account is eosio. This means the restriction applies to an action of any contract as long as it is named one of the following: updateauth, deleteauth, linkauth, unlinkauth, canceldelay. This bug was originally reported in #6654.

Until this bug is fixed with a consensus protocol upgrade, contracts should avoid naming their actions any of above five names. Doing so would not result in a security issue, however, it would prevent users from setting a custom minimum permission for those actions.

Consensus upgrade feature

The goal of this consensus upgrade feature is to correct the checks that prevent an eosio::linkauth action from being used on the five special native actions (and only those five actions).

A new consensus protocol upgrade feature will be added to trigger the changes described in this consensus upgrade proposal. The actual digest for the feature understood at the blockchain level is to be determined. For the purposes of this proposal the codename FIX_LINKAUTH_RESTRICTION will be use to stand-in for whatever the feature identifier will actually end up being.

In authorization_manager::check_linkauth_authorization, the five assertion checks

EOS_ASSERT( link.type != updateauth::get_name(),  action_validate_exception,
            "Cannot link eosio::updateauth to a minimum permission" );
EOS_ASSERT( link.type != deleteauth::get_name(),  action_validate_exception,
            "Cannot link eosio::deleteauth to a minimum permission" );
EOS_ASSERT( link.type != linkauth::get_name(),    action_validate_exception,
            "Cannot link eosio::linkauth to a minimum permission" );
EOS_ASSERT( link.type != unlinkauth::get_name(),  action_validate_exception,
            "Cannot link eosio::unlinkauth to a minimum permission" );
EOS_ASSERT( link.type != canceldelay::get_name(), action_validate_exception,
            "Cannot link eosio::canceldelay to a minimum permission" );

should be guarded by an if statement that only allows those assertions to be checked if FIX_LINKAUTH_RESTRICTION has not yet been activated OR link.code == config::system_account_name.

@arhag arhag added the HARDFORK label Jan 27, 2019

@arhag

This comment has been minimized.

Copy link
Contributor Author

commented Jan 27, 2019

This issue depends on #6429. It will also be setup by default to be a protocol feature requiring pre-activation, thus it also depends on #6431.

@arhag

This comment has been minimized.

Copy link
Contributor Author

commented Mar 29, 2019

Resolved by #7025.

@arhag arhag closed this Mar 29, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.