fix(sandbox): verify effective UID/GID after privilege drop#132
Merged
johntmyers merged 1 commit intomainfrom Mar 5, 2026
Merged
fix(sandbox): verify effective UID/GID after privilege drop#132johntmyers merged 1 commit intomainfrom
johntmyers merged 1 commit intomainfrom
Conversation
Closes #32 Add post-condition checks in drop_privileges() to verify that setgid() and setuid() actually changed the effective IDs. Also verify that setuid(0) fails after dropping privileges, confirming root cannot be re-acquired. This is a defense-in-depth hardening measure per CWE-250 and CERT POS37-C. All added syscalls (geteuid, getegid, setuid) are async-signal-safe, so they are safe in the pre_exec context.
drew
pushed a commit
that referenced
this pull request
Mar 16, 2026
Closes #32 Add post-condition checks in drop_privileges() to verify that setgid() and setuid() actually changed the effective IDs. Also verify that setuid(0) fails after dropping privileges, confirming root cannot be re-acquired. This is a defense-in-depth hardening measure per CWE-250 and CERT POS37-C. All added syscalls (geteuid, getegid, setuid) are async-signal-safe, so they are safe in the pre_exec context. Co-authored-by: John Myers <johntmyers@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #32
Summary
Add defense-in-depth post-condition checks in
drop_privileges()to verify thatsetgid()/setuid()actually changed the effective IDs, and that root cannot be re-acquired after dropping privileges. This hardens the sandbox privilege-drop path per CWE-250 and CERT POS37-C.Changes Made
crates/navigator-sandbox/src/process.rs: Addedgetegid()verification aftersetgid(),geteuid()verification aftersetuid(), and asetuid(0)re-acquisition guard. Added 5 unit tests for thedrop_privileges()function.architecture/security-policy.md: Updated enforcement sequence to document the new verification steps (steps 3, 5, 6).architecture/sandbox.md: Updateddrop_privileges()section to document the post-condition checks and their safety properties.Deviations from Plan
None — implemented as planned.
Tests Added
process::tests— covers no-op paths (no user/group, empty strings), current-user success path (exercises full verification), and error paths for nonexistent user/group.Documentation Updated
architecture/security-policy.md— enforcement sequence now includes verification stepsarchitecture/sandbox.md—drop_privileges()description now documents post-condition checksVerification
tmp/network_checks.pylicense header,navigator-clicompleter test)