Skip to content

Identity default flag: clear-then-set pair has no transaction and no unique constraint #10330

Description

@MiMoHo

Summary

The "exactly one default identity" invariant is maintained by a clear-then-set pair of statements with no transaction, no row lock and no database constraint backing it. Two concurrent set_default() calls can leave a user with two identities flagged standard = 1.

Evidence

Measured on current master, PHP 8.5.10. First, the database accepts the state:

[CDFG-04] identities flagged standard for one user: 2
[CDFG-04] database rejected the second default: NO
[CDFG-04] unique index covering (user_id, standard): NO

There is no unique index in the shipped schema that would prevent it, so the invariant rests entirely on application code. And that code has no serialisation:

[CDFG-04] rcube_user mentions startTransaction   : NO
[CDFG-04] rcube_user mentions endTransaction     : NO
[CDFG-04] rcube_user mentions FOR UPDATE         : NO

The interleaving that produces it is the ordinary one: request A clears the existing default, request B clears (already cleared) and sets its own, request A sets its own — both end up with standard = 1.

Impact

Low to medium, data integrity only, no security impact. Consequences are user-visible rather than dangerous: which identity is picked as the default becomes dependent on row order, so compose may pre-select an unexpected sender. The mirror case — a moment where no identity is flagged default — is also reachable in the window between the clear and the set.

The trigger is mundane: a double-click on "set as default", or the same account being edited from two tabs.

Suggested fix

Two options, ideally both:

  1. Wrap the clear-and-set pair in a transaction, using the existing rcube_db transaction helpers.
  2. Add a partial unique index so the database enforces the invariant as well — on the databases that support it, e.g. CREATE UNIQUE INDEX ... ON identities(user_id) WHERE standard = 1 AND del <> 1.

Point 2 also protects against any future code path that forgets the transaction.

Happy to prepare a PR if that's useful.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions