[Bugfix] Fix --clearenv environment variable leak (#725, #1)

bubblewrap.c

@@ -61,6 +61,9 @@ static int proc_fd = -1;
 static const char *opt_exec_label = NULL;
 static const char *opt_file_label = NULL;
 static bool opt_as_pid_1;
+static bool opt_clearenv = false;
+static gint opt_setenv_count = 0;
+static char *opt_clearenv_saved_path = NULL;  /* Save parent's PATH before clearing */
 
 static const char *opt_argv0 = NULL;
 static const char *opt_chdir_path = NULL;
@@ -256,6 +259,27 @@ seccomp_program_new (int *fd)
   return self;
 }
 
+/* SetEnv structure for accumulating --setenv KEY=VALUE options */
+typedef struct _SetEnv SetEnv;
+
+struct _SetEnv
+{
+  char *key;
+  char *value;
+  SetEnv *next;
+};
+
+DEFINE_LINKED_LIST (SetEnv, setenv)
+
+static SetEnv *
+setenv_new (const char *key, const char *value)
+{
+  SetEnv *self = _setenv_append_new ();
+  self->key = xstrdup (key);
+  self->value = xstrdup (value);
+  return self;
+}
+
 static void
 seccomp_programs_apply (void)
 {
@@ -276,6 +300,130 @@ seccomp_programs_apply (void)
     }
 }
 
+/* Validate --setenv format: KEY=VALUE */
+static gboolean
+validate_setenv_format (const char *entry)
+{
+  const char *equals;
+
+  if (entry == NULL || entry[0] == '\0')
+    return FALSE;
+
+  equals = strchr (entry, '=');
+
+  if (equals == NULL)
+    return FALSE;
+
+  if (equals == entry)
+    return FALSE;  /* Empty key (=VALUE) */
+
+  /* Check key characters: alphanumeric, underscore, no leading digit */
+  for (const char *c = entry; c < equals; c++)
+    {
+      if (!g_ascii_isalnum (*c) && *c != '_')
+        return FALSE;
+
+      if (c == entry && g_ascii_isdigit (*c))
+        return FALSE;  /* Key starts with digit */
+    }
+
+  return TRUE;
+}
+
+/* Parse key from KEY=VALUE format */
+static char *
+parse_key (const char *entry)
+{
+  const char *equals = strchr (entry, '=');
+
+  if (equals == NULL)
+    return NULL;
+
+  return g_strndup (entry, equals - entry);
+}
+
+/* Parse value from KEY=VALUE format */
+static char *
+parse_value (const char *entry)
+{
+  const char *equals = strchr (entry, '=');
+
+  if (equals == NULL)
+    return NULL;
+
+  /* Return empty string if nothing after '=' */
+  return xstrdup (equals + 1);
+}
+
+/* Build environment vector for execve() from accumulated SetEnv entries */
+static char **
+build_environment_vector (SetEnv *list, gint count)
+{
+  char **envp;
+  gint i = 0;
+  SetEnv *entry;
+
+  envp = xcalloc (count + 1, sizeof (char *));
+
+  for (entry = list; entry != NULL; entry = entry->next)
+    {
+      size_t len = strlen (entry->key) + 1 + strlen (entry->value) + 1;
+      char *env_string = xcalloc (len, sizeof (char));
+
+      snprintf (env_string, len, "%s=%s", entry->key, entry->value);
+      envp[i++] = env_string;
+    }
+
+  envp[count] = NULL;
+  return envp;
+}
+
+/* Resolve program path: if relative, search in parent's PATH
+ * If absolute or found, return path; if not found, return NULL */
+static char *
+resolve_program_path (const char *program)
+{
+  const char *path_env;
+  char *resolved;
+
+  /* Check if program has absolute path */
+  if (program[0] == '/')
+    return (char *) program;
+
+  /* Check if program contains a slash (relative path like ./bin/app) */
+  if (strchr (program, '/') != NULL)
+    return (char *) program;
+
+  /* Program is just a name, search in saved PATH (if --clearenv was used) */
+  if (opt_clearenv)
+    path_env = opt_clearenv_saved_path;
+  else
+    path_env = g_getenv ("PATH");
+
+  if (path_env == NULL || path_env[0] == '\0')
+    return NULL;  /* No PATH, can't search */
+
+  /* Simple PATH search: iterate through colon-separated directories */
+  {
+    g_auto (GStrv) path_dirs = g_strsplit (path_env, ":", -1);
+    gint i = 0;
+
+    while (path_dirs[i] != NULL)
+      {
+        g_autofree char *candidate = NULL;
+
+        candidate = g_build_filename (path_dirs[i], program, NULL);
+
+        if (g_file_test (candidate, G_FILE_TEST_IS_EXECUTABLE))
+          return xstrdup (candidate);
+
+        i++;
+      }
+  }
+
+  return NULL;
+}
+
 static void
 usage (int ecode, FILE *out)
 {
@@ -2203,17 +2351,52 @@ parse_args_recurse (int          *argcp,
         }
       else if (strcmp (arg, "--clearenv") == 0)
         {
+          opt_clearenv = true;
+          /* Save parent's PATH before clearing environment */
+          opt_clearenv_saved_path = xstrdup (g_getenv ("PATH") ? g_getenv ("PATH") : "");
           xclearenv ();
         }
       else if (strcmp (arg, "--setenv") == 0)
         {
-          if (argc < 3)
-            die ("--setenv takes two arguments");
+          if (opt_clearenv)
+            {
+              /* When --clearenv is used, expect --setenv KEY=VALUE format */
+              const char *entry;
+              char *key = NULL;
+              char *value = NULL;
 
-          xsetenv (argv[1], argv[2], 1);
+              if (argc < 2)
+                die ("--setenv requires KEY=VALUE argument");
 
-          argv += 2;
-          argc -= 2;
+              entry = argv[1];
+
+              /* Validate and parse KEY=VALUE format */
+              if (!validate_setenv_format (entry))
+                die ("invalid --setenv format: %s (expected KEY=VALUE)", entry);
+
+              key = parse_key (entry);
+              value = parse_value (entry);
+
+              setenv_new (key, value);
+              opt_setenv_count++;
+
+              g_free (key);
+              g_free (value);
+
+              argv += 1;
+              argc -= 1;
+            }
+          else
+            {
+              /* Normal operation without --clearenv: old format --setenv VAR VALUE */
+              if (argc < 3)
+                die ("--setenv takes two arguments");
+
+              xsetenv (argv[1], argv[2], 1);
+
+              argv += 2;
+              argc -= 2;
+            }
         }
       else if (strcmp (arg, "--unsetenv") == 0)
         {
@@ -3221,18 +3404,58 @@ main (int    argc,
   if (opt_argv0 != NULL)
     argv[0] = (char *) opt_argv0;
 
-  if (execvp (exec_path, argv) == -1)
+  if (opt_clearenv)
+    {
+      /* Environment isolation mode: use execve() with custom environment */
+      char *resolved_path = NULL;
+      char **envp = NULL;
+
+      /* Build the environment vector from accumulated --setenv entries */
+      envp = build_environment_vector (seteenvs, opt_setenv_count);
+
+      /* Resolve program path in parent environment BEFORE isolating */
+      resolved_path = resolve_program_path (exec_path);
+
+      if (resolved_path == NULL)
+        {
+          if (setup_finished_pipe[1] != -1)
+            {
+              int saved_errno = ENOENT;
+              char data = 0;
+              res = write_to_fd (setup_finished_pipe[1], &data, 1);
+              errno = saved_errno;
+            }
+          die ("program not found: %s", exec_path);
+        }
+
+      if (execve (resolved_path, argv, envp) == -1)
+        {
+          if (setup_finished_pipe[1] != -1)
+            {
+              int saved_errno = errno;
+              char data = 0;
+              res = write_to_fd (setup_finished_pipe[1], &data, 1);
+              errno = saved_errno;
+            }
+          die_with_error ("execve %s", resolved_path);
+        }
+    }
+  else
     {
-      if (setup_finished_pipe[1] != -1)
+      /* Normal operation: use execvp() (unchanged behavior) */
+      if (execvp (exec_path, argv) == -1)
         {
-          int saved_errno = errno;
-          char data = 0;
-          res = write_to_fd (setup_finished_pipe[1], &data, 1);
-          errno = saved_errno;
-          /* Ignore res, if e.g. the parent died and closed setup_finished_pipe[0]
-             we don't want to error out here */
+          if (setup_finished_pipe[1] != -1)
+            {
+              int saved_errno = errno;
+              char data = 0;
+              res = write_to_fd (setup_finished_pipe[1], &data, 1);
+              errno = saved_errno;
+              /* Ignore res, if e.g. the parent died and closed setup_finished_pipe[0]
+                 we don't want to error out here */
+            }
+          die_with_error ("execvp %s", exec_path);
         }
-      die_with_error ("execvp %s", exec_path);
     }
 
   return 0;

docs/001-clearenv-env-leak/checklists/requirements.md

@@ -0,0 +1,53 @@
+# Specification Quality Checklist: Fix --clearenv Environment Leak
+
+**Purpose**: Validate specification completeness and quality before proceeding to planning  
+**Created**: 8 May 2026  
+**Feature**: [spec.md](../spec.md)
+
+---
+
+## Content Quality
+
+- [x] No implementation details (languages, frameworks, APIs)
+- [x] Focused on user value and business needs
+- [x] Written for non-technical stakeholders
+- [x] All mandatory sections completed
+
+## Requirement Completeness
+
+- [x] No [NEEDS CLARIFICATION] markers remain
+- [x] Requirements are testable and unambiguous
+- [x] Success criteria are measurable
+- [x] Success criteria are technology-agnostic (no implementation details)
+- [x] All acceptance scenarios are defined
+- [x] Edge cases are identified
+- [x] Scope is clearly bounded
+- [x] Dependencies and assumptions identified
+
+## Feature Readiness
+
+- [x] All functional requirements have clear acceptance criteria
+- [x] User scenarios cover primary flows
+- [x] Feature meets measurable outcomes defined in Success Criteria
+- [x] No implementation details leak into specification
+
+---
+
+## Validation Summary
+
+**Status**: ✅ READY FOR PLANNING
+
+All quality criteria satisfied. Specification is complete, unambiguous, and ready for the planning phase.
+
+### Key Strengths
+
+1. **Clear Problem Statement**: Security gap is well-defined with concrete examples
+2. **User-Centric Scenarios**: Three realistic scenarios covering different use cases
+3. **Measurable Success Criteria**: All SC items include specific, verifiable outcomes
+4. **Scope Clarity**: Out of scope section prevents scope creep
+5. **Backward Compatibility**: Explicitly stated requirement for unchanged default behavior
+6. **No Ambiguity**: All requirements can be tested without implementation knowledge
+
+### Ready for Next Phase
+
+This specification is ready to proceed to `/plan` for technical planning and design decisions.