Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Consensus upgrade to disallow linking to non-existing permission #6333

Closed
arhag opened this issue Nov 16, 2018 · 2 comments
Closed

Consensus upgrade to disallow linking to non-existing permission #6333

arhag opened this issue Nov 16, 2018 · 2 comments
Labels
CONSENSUS Introduces a change that may modify consensus protocol rules on an existing blockchain.

Comments

@arhag
Copy link
Contributor

arhag commented Nov 16, 2018

Background

The eosio::linkauth native action was meant to disallow linking an action to a non-existing permission. Due to a bug in apply_eosio_linkauth, it is possible for a user to link an action to a non-existing permission as long as a permission with the same name exists within some account.

There are no critical security issues due to this bug. A user has no incentive to link an action to a non-existing permission. If they do so anyway, for example by accident, the result would be that the linked action could not be authorized by that account until it was unlinked. To unlink, the non-existing permission would first need to be created.

Nevertheless, it is desirable to fix this bug to avoid the above inconvenience due to an accident. A subjective mitigation against this could first be deployed (this is tracked in #6290) before the objective fix is activated as part of a consensus upgrade feature.

Consensus upgrade feature

The goal of this consensus upgrade feature is to correct the checks for the existence of the permission linked to by the eosio::linkauth action.

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 ONLY_LINK_TO_EXISTING_PERMISSION will be use to stand-in for whatever the feature identifier will actually end up being.

In apply_eosio_linkauth, the permission pointer should be reset from an initial value of nullptr to:

  • db.find<permission_object, by_name>(requirement.requirement) (prior to ONLY_LINK_TO_EXISTING_PERMISSION activation);
  • or db.find<permission_object, by_owner>( boost::make_tuple( requirement.account, requirement.requirement ) ) (after ONLY_LINK_TO_EXISTING_PERMISSION activation).

Note: even with ONLY_LINK_TO_EXISTING_PERMISSION activated, it is not safe to assume that the invariant that all permission links map to existing permissions holds because the feature activation will not correct for any existing broken links. The EOSIO code has not yet relied on that invariant in any way that would lead to a security issue, and it is important for the code to continue to not rely on that invariant even after ONLY_LINK_TO_EXISTING_PERMISSION activation.

@arhag
Copy link
Contributor Author

arhag commented Dec 7, 2018

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
Copy link
Contributor Author

arhag commented Mar 28, 2019

Resolved by #6831.

@arhag arhag closed this as completed Mar 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CONSENSUS Introduces a change that may modify consensus protocol rules on an existing blockchain.
Projects
None yet
Development

No branches or pull requests

1 participant