Skip to content

Commit

Permalink
refactor: check permissions in SourceFileFetcher (#5011)
Browse files Browse the repository at this point in the history
This PR hot-fixes permission escapes in dynamic imports, workers
and runtime compiler APIs.

"permissions" parameter was added to public APIs of SourceFileFetcher
and appropriate permission checks are performed during loading of
local and remote files.
  • Loading branch information
bartlomieju authored May 11, 2020
1 parent 0d148c6 commit 32aeec9
Show file tree
Hide file tree
Showing 11 changed files with 396 additions and 74 deletions.
278 changes: 238 additions & 40 deletions cli/file_fetcher.rs

Large diffs are not rendered by default.

9 changes: 5 additions & 4 deletions cli/global_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@ impl GlobalState {
compiler_starts: AtomicUsize::new(0),
compile_lock: AsyncMutex::new(()),
};

Ok(GlobalState(Arc::new(inner)))
}

Expand All @@ -95,14 +94,15 @@ impl GlobalState {
module_specifier: ModuleSpecifier,
maybe_referrer: Option<ModuleSpecifier>,
target_lib: TargetLib,
permissions: Permissions,
) -> Result<CompiledModule, ErrBox> {
let state1 = self.clone();
let state2 = self.clone();
let module_specifier = module_specifier.clone();

let out = self
.file_fetcher
.fetch_source_file(&module_specifier, maybe_referrer)
.fetch_source_file(&module_specifier, maybe_referrer, permissions.clone())
.await?;

// TODO(ry) Try to lift compile_lock as high up in the call stack for
Expand All @@ -115,14 +115,14 @@ impl GlobalState {
| msg::MediaType::JSX => {
state1
.ts_compiler
.compile(state1.clone(), &out, target_lib)
.compile(state1.clone(), &out, target_lib, permissions.clone())
.await
}
msg::MediaType::JavaScript => {
if state1.ts_compiler.compile_js {
state2
.ts_compiler
.compile(state1.clone(), &out, target_lib)
.compile(state1.clone(), &out, target_lib, permissions.clone())
.await
} else {
if let Some(types_url) = out.types_url.clone() {
Expand All @@ -132,6 +132,7 @@ impl GlobalState {
.fetch_source_file(
&types_specifier,
Some(module_specifier.clone()),
permissions.clone(),
)
.await
.ok();
Expand Down
14 changes: 11 additions & 3 deletions cli/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ use crate::global_state::GlobalState;
use crate::msg::MediaType;
use crate::op_error::OpError;
use crate::ops::io::get_stdio;
use crate::permissions::Permissions;
use crate::state::DebugType;
use crate::state::State;
use crate::tsc::TargetLib;
Expand Down Expand Up @@ -187,7 +188,7 @@ async fn print_file_info(

let out = global_state
.file_fetcher
.fetch_source_file(&module_specifier, None)
.fetch_source_file(&module_specifier, None, Permissions::allow_all())
.await?;

println!(
Expand All @@ -205,7 +206,12 @@ async fn print_file_info(
let module_specifier_ = module_specifier.clone();
global_state
.clone()
.fetch_compiled_module(module_specifier_, None, TargetLib::Main)
.fetch_compiled_module(
module_specifier_,
None,
TargetLib::Main,
Permissions::allow_all(),
)
.await?;

if out.media_type == msg::MediaType::TypeScript
Expand Down Expand Up @@ -407,7 +413,9 @@ async fn doc_command(
let fetcher = self.clone();

async move {
let source_file = fetcher.fetch_source_file(&specifier, None).await?;
let source_file = fetcher
.fetch_source_file(&specifier, None, Permissions::allow_all())
.await?;
String::from_utf8(source_file.source_code)
.map_err(|_| OpError::other("failed to parse".to_string()))
}
Expand Down
15 changes: 12 additions & 3 deletions cli/ops/compiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,11 @@ fn op_fetch_source_files(
None
};

let global_state = state.borrow().global_state.clone();
let s = state.borrow();
let global_state = s.global_state.clone();
let permissions = s.permissions.clone();
let perms_ = permissions.clone();
drop(s);
let file_fetcher = global_state.file_fetcher.clone();
let specifiers = args.specifiers.clone();
let future = async move {
Expand All @@ -78,6 +82,7 @@ fn op_fetch_source_files(
.map(|specifier| {
let file_fetcher_ = file_fetcher.clone();
let ref_specifier_ = ref_specifier.clone();
let perms_ = perms_.clone();
async move {
let resolved_specifier = ModuleSpecifier::resolve_url(&specifier)
.expect("Invalid specifier");
Expand All @@ -100,7 +105,7 @@ fn op_fetch_source_files(
}
}
file_fetcher_
.fetch_source_file(&resolved_specifier, ref_specifier_)
.fetch_source_file(&resolved_specifier, ref_specifier_, perms_)
.await
}
.boxed_local()
Expand All @@ -118,7 +123,11 @@ fn op_fetch_source_files(
let types_specifier = ModuleSpecifier::from(types_url);
global_state
.file_fetcher
.fetch_source_file(&types_specifier, ref_specifier.clone())
.fetch_source_file(
&types_specifier,
ref_specifier.clone(),
permissions.clone(),
)
.await
.map_err(OpError::from)?
}
Expand Down
12 changes: 9 additions & 3 deletions cli/ops/runtime_compiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,13 @@ fn op_compile(
) -> Result<JsonOp, OpError> {
state.check_unstable("Deno.compile");
let args: CompileArgs = serde_json::from_value(args)?;
let global_state = state.borrow().global_state.clone();
let s = state.borrow();
let global_state = s.global_state.clone();
let permissions = s.permissions.clone();
let fut = async move {
runtime_compile(
global_state,
permissions,
&args.root_name,
&args.sources,
args.bundle,
Expand All @@ -58,9 +61,12 @@ fn op_transpile(
) -> Result<JsonOp, OpError> {
state.check_unstable("Deno.transpile");
let args: TranspileArgs = serde_json::from_value(args)?;
let global_state = state.borrow().global_state.clone();
let s = state.borrow();
let global_state = s.global_state.clone();
let permissions = s.permissions.clone();
let fut = async move {
runtime_transpile(global_state, &args.sources, &args.options).await
runtime_transpile(global_state, permissions, &args.sources, &args.options)
.await
}
.boxed_local();
Ok(JsonOp::Async(fut))
Expand Down
13 changes: 13 additions & 0 deletions cli/permissions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,19 @@ impl Permissions {
}
}

pub fn allow_all() -> Self {
Self {
allow_read: PermissionState::from(true),
allow_write: PermissionState::from(true),
allow_net: PermissionState::from(true),
allow_env: PermissionState::from(true),
allow_run: PermissionState::from(true),
allow_plugin: PermissionState::from(true),
allow_hrtime: PermissionState::from(true),
..Default::default()
}
}

pub fn check_run(&self) -> Result<(), OpError> {
self
.allow_run
Expand Down
16 changes: 15 additions & 1 deletion cli/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ pub struct StateInner {
pub seeded_rng: Option<StdRng>,
pub target_lib: TargetLib,
pub debug_type: DebugType,
pub is_main: bool,
}

impl State {
Expand Down Expand Up @@ -314,9 +315,20 @@ impl ModuleLoader for State {
let module_url_specified = module_specifier.to_string();
let global_state = state.global_state.clone();
let target_lib = state.target_lib.clone();
let permissions = if state.is_main {
Permissions::allow_all()
} else {
state.permissions.clone()
};

let fut = async move {
let compiled_module = global_state
.fetch_compiled_module(module_specifier, maybe_referrer, target_lib)
.fetch_compiled_module(
module_specifier,
maybe_referrer,
target_lib,
permissions,
)
.await?;
Ok(deno_core::ModuleSource {
// Real module name, might be different from initial specifier
Expand Down Expand Up @@ -395,6 +407,7 @@ impl State {
seeded_rng,
target_lib: TargetLib::Main,
debug_type,
is_main: true,
}));

Ok(Self(state))
Expand Down Expand Up @@ -430,6 +443,7 @@ impl State {
seeded_rng,
target_lib: TargetLib::Worker,
debug_type: DebugType::Dependent,
is_main: false,
}));

Ok(Self(state))
Expand Down
2 changes: 2 additions & 0 deletions cli/tests/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1019,6 +1019,7 @@ fn workers() {
.arg("test")
.arg("--reload")
.arg("--allow-net")
.arg("--allow-read")
.arg("--unstable")
.arg("workers_test.ts")
.spawn()
Expand All @@ -1036,6 +1037,7 @@ fn compiler_api() {
.arg("test")
.arg("--unstable")
.arg("--reload")
.arg("--allow-read")
.arg("compiler_api_test.ts")
.spawn()
.unwrap()
Expand Down
Loading

0 comments on commit 32aeec9

Please sign in to comment.