Skip to content

Sharing a map that was never created puts it in the grantee's listing #444

Description

@marc0olo

Reproduced against ic-vetkeys@0.6.0, Motoko; line references are main at 36ff80e.

Three decisions that are each defensible compose into one that isn't: an owner
can share a map name that has never existed, and it appears in the grantee's
listing as a map with no values, and nothing distinguishing it from a real one.

The tightest evidence is a single function holding two different notions of
whether a map exists:

// EncryptedMaps.mo:333
func getAccessibleMapIdsIter(caller : Caller) : Iter.Iter<MapId> {
    let accessibleMapIds = Iter.fromArray(getAccessibleSharedMapNames(caller));   // no condition
    let ownedMapIds = Iter.fromArray(getOwnedNonEmptyMapNames(caller)).map(        // must be non-empty
        func(mapName) = (caller, mapName)
    );
    return accessibleMapIds.concat(ownedMapIds);
};

Your own empty map is not yours; someone else's empty map is yours to see.

The other two parts:

  • ensureUserCanSetUserRights short-circuits to ownerRights() whenever the
    caller is keyId.0, so a grant over any (caller, anyBlob) is authorised
    (KeyManager.mo:371-374).
  • EncryptedMaps.setUserRights is a bare delegation to the key manager
    (EncryptedMaps.mo:317-324),
    and the key manager holds only accessControl and sharedKeys — it has no
    way to know whether a map exists.

None of the three is wrong on its own, which is presumably why it has survived.

Reproduction

Alice creates nothing, then grants Bob #Read on "ghost":

alice creates nothing. owned: 0 | names: 0
alice shares an uncreated map  : ACCEPTED
bob's listing                  : 1 map — "ghost", owner alice

get_all_accessible_encrypted_maps returns it, so a client cannot tell it from
a map the owner meant to share.

No data is exposed and no authorisation is bypassed — the owner may
legitimately grant rights over their own map id. The defect is that an empty,
uncreated map appears in the grantee's listing as though it were real.

Why we do not think the obvious fix is available to you

An existence check in EncryptedMaps.setUserRights — the one layer that can see
both the ACL and the values — would read as "refuse if the map does not exist".
But EncryptedMapsState is keyManager, mapKeyVals and mapKeys, with no
creation step, so a map exists exactly when it has values. The check would
therefore forbid sharing an empty vault, which is legitimate and which #439 is
already about.

So this looks like a consequence of there being no map existence independent of
emptiness, rather than a missing guard.

Short of that, the two halves of getAccessibleMapIdsIter should agree — and
the direction matters, because only one of them is safe. Drop the
non-emptiness condition on owned maps
, rather than adding one to shared maps.
Adding it would stop an empty vault being shared, which is the case #439 exists
to fix; removing it is what #439 wants anyway. The asymmetry today means your
own empty map is hidden from you while someone else's is shown to you, so the
inconsistency is already resolved in favour of listing.

What it costs an adopter today

We hit this in a password manager on ic-vetkeys 0.6.0. Our fix was to stop
inheriting the access-control write endpoints and gate set_user_rights on our
own vault registry — a concept the library has no equivalent of. That is only
possible for an adopter who owns that endpoint, which is what #443
is asking for; with the composite mixin there is no seam to add the rule at.

Worth saying plainly: the guard we added is not one the library could adopt,
because it depends on our registry. The library-side fix is the existence notion.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions