Skip to content

Derive AppContainer capability count from the capability list - #4

Merged
partouf merged 2 commits into
mainfrom
fix-capability-count
Jul 28, 2026
Merged

Derive AppContainer capability count from the capability list#4
partouf merged 2 commits into
mainfrom
fix-capability-count

Conversation

@partouf

@partouf partouf commented Jul 28, 2026

Copy link
Copy Markdown
Member

InitializeCapabilities allocated sec_cap.Capabilities = new SID_AND_ATTRIBUTES[2] but hardcoded sec_cap.CapabilityCount = 1, while the commented-out network capability lines wrote to indices 0 and 1. That meant:

  • uncommenting the index-0 line would silently overwrite remoteFileAccess
  • uncommenting the index-1 line would be ignored, because the count stayed at 1

Capabilities now append to a std::vector<SID_AND_ATTRIBUTES> member and the count is derived from its size, so entries can be added or removed without keeping indices and a count in sync. Uncommenting any of the three network capability lines now just works.

CreateCapabilitySID(sids, idx, ...) / CreateCapabilitySIDFromName(sids, idx, ...) became AddCapabilitySID(sids, ...) / AddCapabilitySIDFromName(sids, ...). The vector is a member so it outlives the pointer handed to CreateAppContainerProfile and getSecurityCapabilitiesPtr, and data()/size() are read only after all appends, so there is no reallocation invalidation.

Side effect: the new SID_AND_ATTRIBUTES[2] that was never deleted is gone — the vector owns that storage now. The malloc'd Sid buffers inside each entry still leak, unchanged from before.

Not build-tested — this is Win32-only code and it was written on Linux. Worth a Windows compile before merging.

🤖 Generated with Claude Code

partouf and others added 2 commits July 28, 2026 06:19
sec_cap.Capabilities was a fixed new[2] array with CapabilityCount
hardcoded to 1, while the commented-out network capability lines wrote to
indices 0 and 1. Uncommenting the index-0 line would have silently
overwritten remoteFileAccess, and the index-1 line would have been ignored
by the hardcoded count.

Capabilities now append to a std::vector member and the count is taken from
its size, so entries can be added or removed without keeping indices and a
count in sync. The vector also owns the array that was previously leaked.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
NUL is \Device\Null, a device object, so its DACL cannot be reached through the
SE_FILE_OBJECT named-object provider; it has to be read and written through an
open handle with GetKernelObjectSecurity/SetKernelObjectSecurity instead.

The kernel recreates \Device\Null with a default security descriptor on every
boot, and that default does not grant the AppContainer SIDs, so a sandboxed
child that opens NUL (directly, or by redirecting its stdio there) fails with
ERROR_ACCESS_DENIED. Grant it to ALL APPLICATION PACKAGES and ALL RESTRICTED
APPLICATION PACKAGES rather than to the per-run cesandbox<pid> SID: the device
is machine wide, so a per-run grant would race with concurrently running
sandboxes and would leak an ACE for every crashed run.

This is a standalone maintenance mode with no target executable, meant to run
elevated once per boot, so also guard the progid assignment - it previously
read argv[argc] when no executable followed the flags.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@partouf
partouf merged commit 559ae2c into main Jul 28, 2026
1 check passed
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