1
0
Fork 0
mirror of https://github.com/denoland/deno.git synced 2024-11-21 15:04:11 -05:00

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.
This commit is contained in:
David Sherret 2024-08-28 21:11:37 -04:00 committed by GitHub
parent 2afbc1aa39
commit c6793f52b9
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 66 additions and 10 deletions

View file

@ -229,23 +229,61 @@ 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)?;
if permissions.check_run_all(api_name).is_err() {
// 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
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)
));
}
}
}
let mut command = std::process::Command::new(args.cmd);

View file

@ -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(""),
});

View file

@ -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",

View file

@ -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);