Skip to content

fix(protocol): guard acl list reply encode and clamp admin reply counts - #162

Merged
jamofer merged 2 commits into
mainfrom
fix/agt4-admin-reply-encode-guards
Jul 27, 2026
Merged

fix(protocol): guard acl list reply encode and clamp admin reply counts#162
jamofer merged 2 commits into
mainfrom
fix/agt4-admin-reply-encode-guards

Conversation

@jamofer

@jamofer jamofer commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

What

Add the missing termination guard to the ACL list reply encoder, and clamp the entry count the admin ports return before the copy loops.

Why

Audit findings AGT-4 and HUB-minor(clamp). Both are latent contract holes, not live exploits — current callers and the SQLite adapter behave.

Site Hole
AdminAclListReplyMessage_Encode only reply encoder doing strlen+memcpy without checking termination first; an unterminated agent_name runs strlen past the field and writes past the 212-byte slot, overflowing the hub's 4 KiB encode buffer
handleAdminAclList / handleAdminPins the uint8_t a port returns is used directly as the copy-loop bound; a port reporting more than it filled reads past the local array and writes past the reply struct (stack corruption)

How

The guard mirrors the five sibling reply encoders (peers, pins, agents, clients, interfaces), checking the three fields this entry carries and returning 0. The clamp is a guard clause at both call sites: the port was handed a buffer of exactly ADMIN_*_ENTRIES_MAX, so anything beyond that is stack the port never wrote. Boy-scout while in the file: the ACL entry was the only one inlining its field copies, so it now has writeAclEntry/readAclEntry like its siblings.

Testing

  • make test green (407 tests)
  • Removed each fix locally to confirm the tests are red without it: the encoder guard fails 3 protocol tests, and the missing clamp segfaults test_broker (signal 139) — the overflow is real, not theoretical

Notes

No wire-format change. Broker tests drive a test-local port that fills max_entries and reports 255, so the shared mocks stay honest.

@jamofer
jamofer merged commit 2dac1a6 into main Jul 27, 2026
6 checks passed
@jamofer
jamofer deleted the fix/agt4-admin-reply-encode-guards branch July 27, 2026 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant