Ownable2Step::accept_ownership should assert nominated_owner != EMPTY_WORD to block permanent account lockout via zero-address nomination

[giwaov]

Background

TheOwnable2Step account component was introduced in #2572 (v0.14.0). It exposes tansfer_ownership, accept_ownership, and renounce_ownership.

The gap

transfer_ownership(EMPTY_WORD) is documented as a way to cancel a pending nomination. However, if accept_ownership does not first assert that the stored nominated_owner is a non-zero word, a race or off-by-one mistake could allow ownership to be silently transferred to EMPTY_WORD:

• The account becomes permanently inaccessible because no signer can produce a valid signature for the zero key.
• Depending on the signature-verification path, the zero word might vacuously satisfy an empty auth map entry, which is a separate and potentially more severe concern.

Proposed fix

Add a guard at the start of accept_ownership that traps when no nomination is pending:

Assert there is an active nomination (nominated_owner != EMPTY_WORD)

exec.get_nominated_owner
push.0.0.0.0
exec.word::test_eq
assertz # error: no pending nomination

And add a test:

1. Call transfer_ownership(EMPTY_WORD) to cancel any existing nomination.
2. Immediately call accept_ownership -- it must trap.
3. Verify the owner is unchanged.

Impact

Without this guard, any account using Ownable2Step can be permanently locked out if ownership is transferred to the zero address, whether by accident or by a malicious counterparty who times a replay of a stale signed transaction.

[bobbinth]

I'm not sure there is an issue - or at least the issue is not what's described. I believe accept_ownership works as intended (i.e., we do check that note sender is the nominated owner).

But, I think transfer_ownership will cancel any pending transfers if the nominated account is supplied as (0, 0). At the very least, this is not documented - but also, not sure if that's a problem. cc @onurinanc.

[onurinanc]

I don’t think there is a security issue here as accept_ownership already checks that the note sender is the nominated owner. Additionally, transfer_ownership currently clears any pending nomination when the new owner is the current owner, and it also effectively clears the nomination when (0, 0) is supplied, which is fine. Though, the zero-address case may be worth documenting explicitly.

[bobbinth]

@onurinanc - I would maybe document that (0, 0) also clears the nomination - but other than that, I don't think there is much to do - so, I'm closing this issue.