From 6bb72a80863ac3913d32ea21aae32dd327ce6b71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Wed, 24 Aug 2022 18:07:49 +0200 Subject: [PATCH] feat(unstable): add more permission checks for ext/node/ (#15581) --- ext/node/lib.rs | 86 +++++++++++++++++++++++++++++------------- runtime/build.rs | 11 +++++- runtime/permissions.rs | 6 +++ runtime/web_worker.rs | 2 +- runtime/worker.rs | 2 +- 5 files changed, 77 insertions(+), 30 deletions(-) diff --git a/ext/node/lib.rs b/ext/node/lib.rs index f72f72cc96..f96259797f 100644 --- a/ext/node/lib.rs +++ b/ext/node/lib.rs @@ -19,6 +19,10 @@ pub use resolution::package_imports_resolve; pub use resolution::package_resolve; pub use resolution::DEFAULT_CONDITIONS; +pub trait NodePermissions { + fn check_read(&mut self, path: &Path) -> Result<(), AnyError>; +} + pub trait DenoDirNpmResolver { fn resolve_package_folder_from_package( &self, @@ -44,7 +48,7 @@ pub const MODULE_ES_SHIM: &str = include_str!("./module_es_shim.js"); struct Unstable(pub bool); -pub fn init( +pub fn init( unstable: bool, maybe_npm_resolver: Option>, ) -> Extension { @@ -56,25 +60,25 @@ pub fn init( )) .ops(vec![ op_require_init_paths::decl(), - op_require_node_module_paths::decl(), + op_require_node_module_paths::decl::

(), op_require_proxy_path::decl(), op_require_is_deno_dir_package::decl(), op_require_resolve_deno_dir::decl(), op_require_is_request_relative::decl(), op_require_resolve_lookup_paths::decl(), - op_require_try_self_parent_path::decl(), + op_require_try_self_parent_path::decl::

(), op_require_try_self::decl(), - op_require_real_path::decl(), + op_require_real_path::decl::

(), op_require_path_is_absolute::decl(), op_require_path_dirname::decl(), - op_require_stat::decl(), + op_require_stat::decl::

(), op_require_path_resolve::decl(), op_require_path_basename::decl(), - op_require_read_file::decl(), + op_require_read_file::decl::

(), op_require_as_file_path::decl(), op_require_resolve_exports::decl(), op_require_read_package_scope::decl(), - op_require_package_imports_resolve::decl(), + op_require_package_imports_resolve::decl::

(), ]) .state(move |state| { state.put(Unstable(unstable)); @@ -95,15 +99,22 @@ fn check_unstable(state: &OpState) { } } -fn ensure_read_permission( +fn ensure_read_permission

( state: &mut OpState, file_path: &Path, -) -> Result<(), AnyError> { +) -> Result<(), AnyError> +where + P: NodePermissions + 'static, +{ let resolver = { let resolver = state.borrow::>(); resolver.clone() }; - resolver.ensure_read_permission(file_path) + if resolver.ensure_read_permission(file_path).is_ok() { + return Ok(()); + } + + state.borrow_mut::

().check_read(file_path) } #[op] @@ -159,10 +170,13 @@ pub fn op_require_init_paths(state: &mut OpState) -> Vec { } #[op] -pub fn op_require_node_module_paths( +pub fn op_require_node_module_paths

( state: &mut OpState, from: String, -) -> Result, AnyError> { +) -> Result, AnyError> +where + P: NodePermissions + 'static, +{ check_unstable(state); // Guarantee that "from" is absolute. let from = deno_core::resolve_path(&from) @@ -170,7 +184,7 @@ pub fn op_require_node_module_paths( .to_file_path() .unwrap(); - ensure_read_permission(state, &from)?; + ensure_read_permission::

(state, &from)?; if cfg!(windows) { // return root node_modules when path is 'D:\\'. @@ -326,10 +340,16 @@ fn op_require_path_is_absolute(state: &mut OpState, p: String) -> bool { } #[op] -fn op_require_stat(state: &mut OpState, path: String) -> Result { +fn op_require_stat

( + state: &mut OpState, + path: String, +) -> Result +where + P: NodePermissions + 'static, +{ check_unstable(state); let path = PathBuf::from(path); - ensure_read_permission(state, &path)?; + ensure_read_permission::

(state, &path)?; if let Ok(metadata) = std::fs::metadata(&path) { if metadata.is_file() { return Ok(0); @@ -342,13 +362,16 @@ fn op_require_stat(state: &mut OpState, path: String) -> Result { } #[op] -fn op_require_real_path( +fn op_require_real_path

( state: &mut OpState, request: String, -) -> Result { +) -> Result +where + P: NodePermissions + 'static, +{ check_unstable(state); let path = PathBuf::from(request); - ensure_read_permission(state, &path)?; + ensure_read_permission::

(state, &path)?; let mut canonicalized_path = path.canonicalize()?; if cfg!(windows) { canonicalized_path = PathBuf::from( @@ -393,12 +416,15 @@ fn op_require_path_basename(state: &mut OpState, request: String) -> String { } #[op] -fn op_require_try_self_parent_path( +fn op_require_try_self_parent_path

( state: &mut OpState, has_parent: bool, maybe_parent_filename: Option, maybe_parent_id: Option, -) -> Result, AnyError> { +) -> Result, AnyError> +where + P: NodePermissions + 'static, +{ check_unstable(state); if !has_parent { return Ok(None); @@ -411,7 +437,7 @@ fn op_require_try_self_parent_path( if let Some(parent_id) = maybe_parent_id { if parent_id == "" || parent_id == "internal/preload" { if let Ok(cwd) = std::env::current_dir() { - ensure_read_permission(state, &cwd)?; + ensure_read_permission::

(state, &cwd)?; return Ok(Some(cwd.to_string_lossy().to_string())); } } @@ -476,13 +502,16 @@ fn op_require_try_self( } #[op] -fn op_require_read_file( +fn op_require_read_file

( state: &mut OpState, file_path: String, -) -> Result { +) -> Result +where + P: NodePermissions + 'static, +{ check_unstable(state); let file_path = PathBuf::from(file_path); - ensure_read_permission(state, &file_path)?; + ensure_read_permission::

(state, &file_path)?; Ok(std::fs::read_to_string(file_path)?) } @@ -551,14 +580,17 @@ fn op_require_read_package_scope( } #[op] -fn op_require_package_imports_resolve( +fn op_require_package_imports_resolve

( state: &mut OpState, parent_filename: String, request: String, -) -> Result, AnyError> { +) -> Result, AnyError> +where + P: NodePermissions + 'static, +{ check_unstable(state); let parent_path = PathBuf::from(&parent_filename); - ensure_read_permission(state, &parent_path)?; + ensure_read_permission::

(state, &parent_path)?; let resolver = state.borrow::>().clone(); let pkg = PackageJson::load(&*resolver, parent_path.join("package.json"))?; diff --git a/runtime/build.rs b/runtime/build.rs index 5e81326569..a9ba09825f 100644 --- a/runtime/build.rs +++ b/runtime/build.rs @@ -125,6 +125,15 @@ mod not_docs { } } + impl deno_node::NodePermissions for Permissions { + fn check_read( + &mut self, + _p: &Path, + ) -> Result<(), deno_core::error::AnyError> { + unreachable!("snapshotting!") + } + } + impl deno_net::NetPermissions for Permissions { fn check_net>( &mut self, @@ -167,7 +176,7 @@ mod not_docs { deno_broadcast_channel::InMemoryBroadcastChannel::default(), false, // No --unstable. ), - deno_node::init(false, None), // No --unstable. + deno_node::init::(false, None), // No --unstable. deno_ffi::init::(false), deno_net::init::( None, false, // No --unstable. diff --git a/runtime/permissions.rs b/runtime/permissions.rs index 0a55608336..5893f44d46 100644 --- a/runtime/permissions.rs +++ b/runtime/permissions.rs @@ -1322,6 +1322,12 @@ impl deno_flash::FlashPermissions for Permissions { } } +impl deno_node::NodePermissions for Permissions { + fn check_read(&mut self, path: &Path) -> Result<(), AnyError> { + self.read.check(path) + } +} + impl deno_net::NetPermissions for Permissions { fn check_net>( &mut self, diff --git a/runtime/web_worker.rs b/runtime/web_worker.rs index 7ef280252f..d4a96633d0 100644 --- a/runtime/web_worker.rs +++ b/runtime/web_worker.rs @@ -423,7 +423,7 @@ impl WebWorker { unstable, options.unsafely_ignore_certificate_errors.clone(), ), - deno_node::init(unstable, options.npm_resolver), + deno_node::init::(unstable, options.npm_resolver), ops::os::init_for_worker(), ops::permissions::init(), ops::process::init(), diff --git a/runtime/worker.rs b/runtime/worker.rs index ad2bcdc17c..e0f54e09d9 100644 --- a/runtime/worker.rs +++ b/runtime/worker.rs @@ -165,7 +165,7 @@ impl MainWorker { unstable, options.unsafely_ignore_certificate_errors.clone(), ), - deno_node::init(unstable, options.npm_resolver), + deno_node::init::(unstable, options.npm_resolver), ops::os::init(exit_code.clone()), ops::permissions::init(), ops::process::init(),