[Bugfix] Fix --clearenv environment variable leak (#725, #1)#2
Open
pratikpawar009 wants to merge 1 commit into
Open
[Bugfix] Fix --clearenv environment variable leak (#725, #1)#2pratikpawar009 wants to merge 1 commit into
pratikpawar009 wants to merge 1 commit into
Conversation
This commit implements proper environment variable isolation for the --clearenv flag, fixing a critical security vulnerability where parent process environment variables were leaking to child processes despite the isolation intent. PROBLEM: -------- Previously, the --clearenv flag failed to completely isolate environment variables. Parent process variables were visible to child processes, defeating the security guarantee expected by users relying on --clearenv for secret isolation. SOLUTION: --------- Implemented environment isolation using execve() with custom environment vector: - When --clearenv is specified, parent's PATH is saved before clearing environment - Accumulated --setenv options are stored in linked list during option parsing - At execution time, an environment vector is constructed from accumulated entries - execve() is used instead of execvp() to pass custom environment to child - Child process receives ONLY explicitly set variables via --setenv TECHNICAL DETAILS: ------------------ - New SetEnv struct for accumulating --setenv KEY=VALUE options - Validation functions for KEY=VALUE format checking - PATH resolution in parent process before environment isolation - Conditional execution path: execve() if --clearenv, execvp() otherwise - 100% backward compatible: default behavior completely unchanged TESTING: -------- - 11 comprehensive tests covering isolation, variable propagation, edge cases - Tests verify: isolation works, explicit vars visible, special vars cleared, backward compatibility maintained, invalid formats rejected - All tests in tests/test-clearenv.sh DOCUMENTATION: --------------- - Complete specification in docs/001-clearenv-env-leak/spec.md - Implementation plan in docs/001-clearenv-env-leak/plan.md - Data model and design decisions in docs/001-clearenv-env-leak/data-model.md - CLI contract in docs/001-clearenv-env-leak/contracts/cli-contract.md - User guide with examples in docs/001-clearenv-env-leak/quickstart.md - Research findings and technical decisions in docs/001-clearenv-env-leak/research.md BACKWARD COMPATIBILITY: ----------------------- ✓ 100% compatible with existing usage ✓ Default behavior when --clearenv not used is unchanged ✓ All existing flags and combinations work as before ✓ No modifications needed to existing code or tests Fixes containers#725 Fixes #1
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
2 participants
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.
Overview
Fix critical security vulnerability where
--clearenvflag fails to isolate environment variables.Problem
Previously, when using
--clearenvto isolate containers, parent process environment variables were still visible to child processes, defeating the security intent.Solution
Implemented proper environment isolation using
execve()with custom environment vector:--setenvChanges
Implementation
bubblewrap.c: ~150 lines added for proper environment isolation
tests/test-clearenv.sh: New comprehensive test suite with 11 tests
Documentation
Complete specification and design documents in
docs/001-clearenv-env-leak/:Testing
Backward Compatibility
✓ 100% compatible - default behavior when --clearenv not used is unchanged
✓ All existing tests pass without modification
✓ All existing flags and combinations work as before
Closes containers#725
Closes #1