Skip to content

Repeater: CLI reply to 'setperm <key> 0' is lost when the removed ACL entry sits before the requester (stale ClientInfo* after array shift) #3513

Description

@marcelverdult

Summary

On a repeater, setperm <prefix> 0 (revoke) often gets no CLI reply although the entry is removed. Cause: MyMesh::onPeerDataRecv() keeps a ClientInfo* into the ACL array across handleCommand(), and the revoke path compacts that array in place, so the reply is addressed/routed with another client's data.

Where

examples/simple_repeater/MyMesh.cpp (tag repeater-v1.17.1; main @ e941259 is identical):

  • L670 — requester resolved by index before the command runs: ClientInfo* client = acl.getClientByIdx(i);
  • L739 — handleCommand(...) runs setperm → ClientACL::applyPermissions()
  • L751-L757 — the same client pointer is used afterwards: createDatagram(..., client->id, ...), client->out_path_len, client->out_path

src/helpers/ClientACL.cpp L123-L132 — guest/revoke branch shifts the fixed array down in place:

num_clients--;
int i = c - clients;
while (i < num_clients) {
  clients[i] = clients[i + 1];
  i++;
}

If the removed row's index is ≤ the requester's own index, client now points at the next client's data, so the OK goes to the wrong destination hash/path and the admin never sees it. If the removed row sits after the requester, nothing moves under the pointer and the reply arrives. Edge case: when the requester occupies the last populated slot, its old slot is not overwritten (stale copy of itself), so the reply still arrives.

Same code in v1.15.0, v1.16.0, v1.17.0 and v1.17.1 (only line numbers differ in MyMesh.cpp).

Reproduce (observed on a stock repeater, v1.17.0-727fc05)

  1. Log in as admin A. Make sure a client B sits before A in the ACL and another client C sits after A (e.g. add B, remove and re-add A with a password login, then add C with setperm <C> 1).
  2. setperm <B-prefix> 0 → no reply. Repeat the same command → Err - invalid params (B is already gone, so the first command did work).
  3. setperm <C-prefix> 0 → OK as expected.
  4. setperm <A-prefix> 0 (self-revoke) with another client after A → no reply either.

Expected

The CLI reply always goes to the admin who sent the command.

Suggested fix

Smallest change: before handleCommand(), copy what the reply needs (the requester's id, out_path, out_path_len) into locals and use those afterwards instead of client->.... Alternatives: look the requester up again by pubkey after the command, or defer the array compaction (mark permissions = 0 and compact later).

Clients can currently only treat a missing reply to setperm … 0 as "unconfirmed" and re-read the list with REQ_TYPE_GET_ACCESS_LIST.

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

    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