OpenZeppelin / OpenZeppelin/openzeppelin-contracts

Backup Accounts Mechanism For Unique-Roles Transfer Can Cheapen A Lot The Defense Against Leaked Keys

Open
#4,874 0 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

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

Description

Backup Accounts Mechanism For Unique-Roles Transfer Can Cheapen A Lot The Defense Against Leaked Keys

🧐 Motivation
So I was auditing and I thought of this scenario:

What if the private key(s) for the DEFAULT_ADMIN_ROLE address on a system using AccessControlDefaultAdminRules gets leaked and an attacker tries to change it to hijack the system?

The attacker would need to initiate the transfer process and wait a delay to accept it hoping the legit owner or owners won't cancel the operation in the meanwhile. So in practice the only thing the legit owner needs to do is to keep front-running the attacker canceling its change DEFAULT_ADMIN_ROLE proposals until all users are aware of the battle going on and move their funds to a save place.

Actually the attacker could elaborate a more complex attack, for example in systems where there are special settings only modifyable by the admin, the attacker can complicate the defense strategy by trying to modify those settings at the same time it tries to get the ownership to its own wallet, thus making the defense strategy more complex and expensive to nullify until all users are safe.

So I thought, isn't there a cheaper simpler way of protecting against this? 🤔 And that is when I came with
the idea of a backup-address.

📝 Details

A simple implementation and more details in this draft github repo I've created.

Basically, a backup-address is an address which a user holding a unique-role (a role that must only be held by 1 account) can pass its role to without having to pass trhough the 2tx process or/and the time delay. This backup-address must be owned by the same user which holds the role.

This creates a mechanism in which in case of compromised key(s) there is a legit backup to another account controlled by the team that can be used to transfer the role immediately in 1tx. This cheapens and facilitates the defense protocol plus there would not exist the need for clients to migrate as you can retake the control over the system.

Here is a general idea of how it would work with AccessControlDefaultAdminRules:

  function beginDefaultAdminTransfer(address newAdmin) public override onlyRole(DEFAULT_ADMIN_ROLE) {
        if (checkIsBackup(defaultAdmin(), newAdmin)) {
            _grantRole(DEFAULT_ADMIN_ROLE, newAdmin);
        } else {
            super.beginDefaultAdminTransfer(newAdmin);
        }
    }

See the full draft implementation in AccessControlDefaultAdminRulesWithBackup.sol

Surely this alone wouldn't prevent the attacker from creating this defense racing problem again because he could always just use some changeBackup() function before the attack and thus creating again a the race condition to set your desired admin. This is why I've also added a time delay to change the backup-address in the AccountBackup.sol contract.

There is also a 2tx process for changing the backup-address to increase security and prevent accidentally setting undesired addresses as backup addresses. In 1st tx you propose a backup address and in the 2nd tx, sent by very same backup-address, the backup-address accepts the proposal.

📘 Note 2 ℹ️: All this makes it also compatible with Ownable2Step as the setting of a new backup-address requires 2 steps assuring security. It is just that its transfer to the backup-address can be done in 1 which would require the legit admin to just safely use 1tx instead of 2 while the attacker would require 2tx. Still the attacker can front-run but it would become harder to do so.

We could describe my idea as a general measure that can be taken to easen and cheapen defense against compromised keys in systems where there is a unique-role being transfered in a 2tx manner and/or with a time delay.

Lets get crazier: But what if the backup also gets compromised? 🤔

Well, we can cheaply implement backups for the backups if we desire it so with mappings. Now auditors have a new question to ask to protocols that implement this kind of unique-roles handling: how many back-up accounts do you have?

The example draft of AccountBackup.sol handles this recursive backing up.

So, even if the back-up account gets leaked, you can just have prepared as many back-ups for the back-ups as you want.

📘 Note 3 ℹ️: I would be super-ultra-happy to work along with the OZ team in this feature if it gets approved! 😄

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

Review the proposal around AccessControlDefaultAdminRules and the referenced Ownable2Step behavior, then inspect the draft AccountBackup.sol and AccessControlDefaultAdminRulesWithBackup.sol implementations. Before coding, the project would need to accept a concrete scope and security/API design; done would require that design to be implemented and validated with repository tests.

Written by the indexing model from the issue text.

Assessment

Tech stack
solidity
Domain
authorization, blockchain, security
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
20/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.