diff options
| author | Daniil Baturin <daniil@baturin.org> | 2025-11-11 13:46:47 +0000 |
|---|---|---|
| committer | Daniil Baturin <daniil@baturin.org> | 2025-11-11 13:46:47 +0000 |
| commit | 4138a081bdfddd09bac348ec74c3ed8b162ba037 (patch) | |
| tree | 332ff62d5d5495a7b4c96226808158c846cc002f | |
| parent | 5285c78bc99c8f97cb85b113638259b80b269ae2 (diff) | |
| download | vyos-1x-4138a081bdfddd09bac348ec74c3ed8b162ba037.tar.gz vyos-1x-4138a081bdfddd09bac348ec74c3ed8b162ba037.zip | |
vyos-op-run: T8005: sanitize environment variables
to prevent malicious variable injection
| -rw-r--r-- | debian/changelog | 6 | ||||
| -rw-r--r-- | src/vyos_op_run.ml | 34 |
2 files changed, 39 insertions, 1 deletions
diff --git a/debian/changelog b/debian/changelog index 0db92790a..08a5f8ce2 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,9 @@ +vyos-utils (0.0.5) unstable; urgency=medium + + * Sanitize the environment in vyos-op-run to prevent malicious variable injection + + -- Daniil Baturin <daniil@vyos.io> Tue, 11 Nov 2025 13:46:05 +0000 + vyos-utils (0.0.4) unstable; urgency=medium * Add an operational command runner. diff --git a/src/vyos_op_run.ml b/src/vyos_op_run.ml index 0767ba298..c7e38a753 100644 --- a/src/vyos_op_run.ml +++ b/src/vyos_op_run.ml @@ -319,7 +319,39 @@ let run_external_command opts env command_tmpl = let cmd = render_command opts env command_tmpl in if opts.dry_run then Printf.printf "%s\n%!" cmd else let () = Logs.debug @@ fun m -> m "Command to be executed %s" cmd in - let res = Unix.system cmd in + (* Get the user database entry to populate the basic environment from: + we cannot trust an unprivileged user to supply $SHELL + or allow them to impersonate someone else by setting custom $LOGNAME, etc. + *) + let user_pw_entry = Unix.getpwuid @@ Unix.getuid () in + let make_var name value = Printf.sprintf "%s=%s" name value in + let env = [| + (* A knowingly safe executable lookup path. + Since we do not use /usr/local, we do not need to include that. + Executables in VyOS-specific directories are referred to by absolute paths + in the operational command JSON cache, + so we don't need to include those, either. + *) + make_var "PATH" "/usr/sbin:/usr/bin:/sbin:/bin"; + (* Standard UNIX variables *) + make_var "HOME" user_pw_entry.pw_dir; + make_var "USER" user_pw_entry.pw_name; + make_var "LOGNAME" user_pw_entry.pw_name; + make_var "SHELL" user_pw_entry.pw_shell; + (* VyOS-specific variables *) + make_var "vyos_data_dir" "/usr/share/vyos"; + make_var "vyos_validators_dir" "/usr/libexec/vyos/validators"; + make_var "vyos_completion_dir" "/usr/libexec/vyos/completion"; + make_var "vyos_libexec_dir" "/usr/libexec/vyos"; + make_var "vyos_op_scripts_dir" "/usr/libexec/vyos/op_mode"; + |] + in + let shell = "/bin/sh" in + (* We use execve with an absolute path to Bourne shell rather than execvpe + so that a user trying to do PATH=/bad/place vyos-op-run + cannot achieve anything with that trick. + *) + let res = Unix.execve shell [|shell; "-c"; cmd|] env in match res with | Unix.WEXITED 0 -> () | _ -> |