fix(permissions): disallow any `LD_` or `DYLD_` prefixed env var without full --allow-run permissions (#25271)

Follow up to https://github.com/denoland/deno/pull/25221

I looked into what the list was and it was quite extensive, so I think
as suggested in
https://github.com/denoland/deno/issues/11964#issuecomment-2314585135 we
should disallow this for any `LD_` prefixed env var.

4 files changed, 66 insertions, 10 deletions

diff --git a/runtime/ops/process.rs b/runtime/ops/process.rs
index 564092454..25edfe662 100644
--- a/runtime/ops/process.rs
+++ b/runtime/ops/process.rs
@@ -229,21 +229,59 @@ fn create_command(
   mut args: SpawnArgs,
   api_name: &str,
 ) -> Result<CreateCommand, AnyError> {
+  fn get_requires_allow_all_env_var(args: &SpawnArgs) -> Option<Cow<str>> {
+    fn requires_allow_all(key: &str) -> bool {
+      let key = key.trim();
+      // we could be more targted here, but there are quite a lot of
+      // LD_* and DYLD_* env variables
+      key.starts_with("LD_") || key.starts_with("DYLD_")
+    }
+
+    /// Checks if the user set this env var to an empty
+    /// string in order to clear it.
+    fn args_has_empty_env_value(args: &SpawnArgs, key_name: &str) -> bool {
+      args
+        .env
+        .iter()
+        .find(|(k, _)| k == key_name)
+        .map(|(_, v)| v.trim().is_empty())
+        .unwrap_or(false)
+    }
+
+    if let Some((key, _)) = args
+      .env
+      .iter()
+      .find(|(k, v)| requires_allow_all(k) && !v.trim().is_empty())
+    {
+      return Some(key.into());
+    }
+
+    if !args.clear_env {
+      if let Some((key, _)) = std::env::vars().find(|(k, v)| {
+        requires_allow_all(k)
+          && !v.trim().is_empty()
+          && !args_has_empty_env_value(args, k)
+      }) {
+        return Some(key.into());
+      }
+    }
+
+    None
+  }
+
   {
     let permissions = state.borrow_mut::<PermissionsContainer>();
     permissions.check_run(&args.cmd, api_name)?;
-    // error the same on all platforms
-    if permissions.check_run_all(api_name).is_err()
-      && (args.env.iter().any(|(k, _)| k.trim() == "LD_PRELOAD")
-        || !args.clear_env
-          && std::env::vars().any(|(k, _)| k.trim() == "LD_PRELOAD"))
-    {
-      // we don't allow users to launch subprocesses with the LD_PRELOAD
-      // env var set because this allows executing any code
-      return Err(deno_core::error::custom_error(
+    if permissions.check_run_all(api_name).is_err() {
+      // error the same on all platforms
+      if let Some(name) = get_requires_allow_all_env_var(&args) {
+        // we don't allow users to launch subprocesses with any LD_ or DYLD_*
+        // env vars set because this allows executing code (ex. LD_PRELOAD)
+        return Err(deno_core::error::custom_error(
           "PermissionDenied",
-          "Requires --allow-all permissions to spawn subprocess with LD_PRELOAD environment variable."
+          format!("Requires --allow-all permissions to spawn subprocess with {} environment variable.", name)
         ));
+      }
     }
   }
 
diff --git a/tests/integration/run_tests.rs b/tests/integration/run_tests.rs
index 64e9d1c22..ade5c4560 100644
--- a/tests/integration/run_tests.rs
+++ b/tests/integration/run_tests.rs
@@ -501,6 +501,10 @@ itest!(_088_dynamic_import_already_evaluating {
 // TODO(bartlomieju): remove --unstable once Deno.Command is stabilized
 itest!(_089_run_allow_list {
   args: "run --unstable --allow-run=curl run/089_run_allow_list.ts",
+  envs: vec![
+    ("LD_LIBRARY_PATH".to_string(), "".to_string()),
+    ("DYLD_FALLBACK_LIBRARY_PATH".to_string(), "".to_string())
+  ],
   output: "run/089_run_allow_list.ts.out",
 });
 
@@ -3708,6 +3712,10 @@ itest!(test_and_bench_are_noops_in_run {
 #[cfg(not(target_os = "windows"))]
 itest!(spawn_kill_permissions {
   args: "run --quiet --allow-run=cat spawn_kill_permissions.ts",
+  envs: vec![
+    ("LD_LIBRARY_PATH".to_string(), "".to_string()),
+    ("DYLD_FALLBACK_LIBRARY_PATH".to_string(), "".to_string())
+  ],
   output_str: Some(""),
 });
 
diff --git a/tests/specs/run/ld_preload/__test__.jsonc b/tests/specs/run/ld_preload/__test__.jsonc
index f3a9b26bc..767e423d0 100644
--- a/tests/specs/run/ld_preload/__test__.jsonc
+++ b/tests/specs/run/ld_preload/__test__.jsonc
@@ -1,4 +1,9 @@
 {
+  "envs": {
+    "LD_LIBRARY_PATH": "",
+    "LD_PRELOAD": "",
+    "DYLD_FALLBACK_LIBRARY_PATH": ""
+  },
   "tests": {
     "env_arg": {
       "args": "run --allow-run=echo env_arg.ts",
diff --git a/tests/unit/os_test.ts b/tests/unit/os_test.ts
index 42b598511..9503f75d1 100644
--- a/tests/unit/os_test.ts
+++ b/tests/unit/os_test.ts
@@ -239,6 +239,11 @@ Deno.test(
   async function hostnameWithoutOtherNetworkUsages() {
     const { stdout } = await new Deno.Command(Deno.execPath(), {
       args: ["eval", "-p", "Deno.hostname()"],
+      env: {
+        LD_PRELOAD: "",
+        LD_LIBRARY_PATH: "",
+        DYLD_FALLBACK_LIBRARY_PATH: "",
+      },
     }).output();
     const hostname = new TextDecoder().decode(stdout).trim();
     assert(hostname.length > 0);