16 files changed, 95 insertions, 19 deletions
diff --git a/cli/ops.rs b/cli/ops.rs
index 166610bac..7a68dee55 100644
--- a/cli/ops.rs
+++ b/cli/ops.rs
@@ -497,7 +497,12 @@ fn op_fetch_source_file(
   let specifier = inner.specifier().unwrap();
   let referrer = inner.referrer().unwrap();
 
-  let resolved_specifier = state.resolve(specifier, referrer, false)?;
+  // TODO(ry) Maybe a security hole. Only the compiler worker should have access
+  // to this. Need a test to demonstrate the hole.
+  let is_dyn_import = false;
+
+  let resolved_specifier =
+    state.resolve(specifier, referrer, false, is_dyn_import)?;
 
   let fut = state
     .file_fetcher
@@ -750,7 +755,7 @@ fn op_fetch(
   let req = msg_util::deserialize_request(header, body)?;
 
   let url_ = url::Url::parse(url).map_err(ErrBox::from)?;
-  state.check_net_url(url_)?;
+  state.check_net_url(&url_)?;
 
   let client = http_util::get_client();
 
diff --git a/cli/permissions.rs b/cli/permissions.rs
index 9774d92c0..814c3ff94 100644
--- a/cli/permissions.rs
+++ b/cli/permissions.rs
@@ -274,7 +274,7 @@ impl DenoPermissions {
     }
   }
 
-  pub fn check_net_url(&self, url: url::Url) -> Result<(), ErrBox> {
+  pub fn check_net_url(&self, url: &url::Url) -> Result<(), ErrBox> {
     let msg = &format!("network access to \"{}\"", url);
     match self.allow_net.get_state() {
       PermissionAccessorState::Allow => {
@@ -627,7 +627,7 @@ mod tests {
 
     for (url_str, is_ok) in url_tests.iter() {
       let u = url::Url::parse(url_str).unwrap();
-      assert_eq!(*is_ok, perms.check_net_url(u).is_ok());
+      assert_eq!(*is_ok, perms.check_net_url(&u).is_ok());
     }
 
     for (domain, is_ok) in domain_tests.iter() {
diff --git a/cli/state.rs b/cli/state.rs
index eb912161d..0b0f3b1ae 100644
--- a/cli/state.rs
+++ b/cli/state.rs
@@ -4,6 +4,7 @@ use crate::compilers::JsCompiler;
 use crate::compilers::JsonCompiler;
 use crate::compilers::TsCompiler;
 use crate::deno_dir;
+use crate::deno_error::permission_denied;
 use crate::file_fetcher::SourceFileFetcher;
 use crate::flags;
 use crate::global_timer::GlobalTimer;
@@ -119,6 +120,7 @@ impl Loader for ThreadSafeState {
     specifier: &str,
     referrer: &str,
     is_main: bool,
+    is_dyn_import: bool,
   ) -> Result<ModuleSpecifier, ErrBox> {
     if !is_main {
       if let Some(import_map) = &self.import_map {
@@ -128,8 +130,14 @@ impl Loader for ThreadSafeState {
         }
       }
     }
+    let module_specifier =
+      ModuleSpecifier::resolve_import(specifier, referrer)?;
 
-    ModuleSpecifier::resolve_import(specifier, referrer).map_err(ErrBox::from)
+    if is_dyn_import {
+      self.check_dyn_import(&module_specifier)?;
+    }
+
+    Ok(module_specifier)
   }
 
   /// Given an absolute url, load its source code.
@@ -294,7 +302,7 @@ impl ThreadSafeState {
   }
 
   #[inline]
-  pub fn check_net_url(&self, url: url::Url) -> Result<(), ErrBox> {
+  pub fn check_net_url(&self, url: &url::Url) -> Result<(), ErrBox> {
     self.permissions.check_net_url(url)
   }
 
@@ -303,6 +311,30 @@ impl ThreadSafeState {
     self.permissions.check_run()
   }
 
+  pub fn check_dyn_import(
+    self: &Self,
+    module_specifier: &ModuleSpecifier,
+  ) -> Result<(), ErrBox> {
+    let u = module_specifier.as_url();
+    match u.scheme() {
+      "http" | "https" => {
+        self.check_net_url(u)?;
+        Ok(())
+      }
+      "file" => {
+        let filename = u
+          .to_file_path()
+          .unwrap()
+          .into_os_string()
+          .into_string()
+          .unwrap();
+        self.check_read(&filename)?;
+        Ok(())
+      }
+      _ => Err(permission_denied()),
+    }
+  }
+
   #[cfg(test)]
   pub fn mock(argv: Vec<String>) -> ThreadSafeState {
     ThreadSafeState::new(
diff --git a/core/modules.rs b/core/modules.rs
index 072de4bc9..5956a7317 100644
--- a/core/modules.rs
+++ b/core/modules.rs
@@ -40,6 +40,7 @@ pub trait Loader: Send + Sync {
     specifier: &str,
     referrer: &str,
     is_main: bool,
+    is_dyn_import: bool,
   ) -> Result<ModuleSpecifier, ErrBox>;
 
   /// Given ModuleSpecifier, load its source code.
@@ -125,12 +126,15 @@ impl<L: Loader> RecursiveLoad<L> {
 
   fn add_root(&mut self) -> Result<(), ErrBox> {
     let module_specifier = match self.state {
-      State::ResolveMain(ref specifier) => {
-        self.loader.resolve(specifier, ".", true)?
-      }
-      State::ResolveImport(ref specifier, ref referrer) => {
-        self.loader.resolve(specifier, referrer, false)?
-      }
+      State::ResolveMain(ref specifier) => self.loader.resolve(
+        specifier,
+        ".",
+        true,
+        self.dyn_import_id().is_some(),
+      )?,
+      State::ResolveImport(ref specifier, ref referrer) => self
+        .loader
+        .resolve(specifier, referrer, false, self.dyn_import_id().is_some())?,
       _ => unreachable!(),
     };
 
@@ -160,7 +164,12 @@ impl<L: Loader> RecursiveLoad<L> {
     referrer: &str,
     parent_id: deno_mod,
   ) -> Result<(), ErrBox> {
-    let module_specifier = self.loader.resolve(specifier, referrer, false)?;
+    let module_specifier = self.loader.resolve(
+      specifier,
+      referrer,
+      false,
+      self.dyn_import_id().is_some(),
+    )?;
     let module_name = module_specifier.as_str();
 
     let mut modules = self.modules.lock().unwrap();
@@ -277,7 +286,12 @@ impl<L: Loader> ImportStream for RecursiveLoad<L> {
         |specifier: &str, referrer_id: deno_mod| -> deno_mod {
           let modules = self.modules.lock().unwrap();
           let referrer = modules.get_name(referrer_id).unwrap();
-          match self.loader.resolve(specifier, &referrer, is_main) {
+          match self.loader.resolve(
+            specifier,
+            &referrer,
+            is_main,
+            self.dyn_import_id().is_some(),
+          ) {
             Ok(specifier) => modules.get_id(specifier.as_str()).unwrap_or(0),
             // We should have already resolved and Ready this module, so
             // resolve() will not fail this time.
@@ -675,6 +689,7 @@ mod tests {
       specifier: &str,
       referrer: &str,
       _is_root: bool,
+      _is_dyn_import: bool,
     ) -> Result<ModuleSpecifier, ErrBox> {
       let referrer = if referrer == "." {
         "file:///"
diff --git a/tests/013_dynamic_import.test b/tests/013_dynamic_import.test
index 8fe463b20..d65139dfa 100644
--- a/tests/013_dynamic_import.test
+++ b/tests/013_dynamic_import.test
@@ -1,2 +1,2 @@
-args: tests/013_dynamic_import.ts --reload
+args: tests/013_dynamic_import.ts --reload --allow-read
 output: tests/013_dynamic_import.ts.out
diff --git a/tests/014_duplicate_import.test b/tests/014_duplicate_import.test
index 57d5b6e8b..c4811a32c 100644
--- a/tests/014_duplicate_import.test
+++ b/tests/014_duplicate_import.test
@@ -1,2 +1,2 @@
-args: tests/014_duplicate_import.ts --reload
+args: tests/014_duplicate_import.ts --reload --allow-read
 output: tests/014_duplicate_import.ts.out
diff --git a/tests/015_duplicate_parallel_import.test b/tests/015_duplicate_parallel_import.test
index ec8c79b21..837827795 100644
--- a/tests/015_duplicate_parallel_import.test
+++ b/tests/015_duplicate_parallel_import.test
@@ -1,2 +1,2 @@
-args: tests/015_duplicate_parallel_import.js --reload
+args: tests/015_duplicate_parallel_import.js --reload --allow-read
 output: tests/015_duplicate_parallel_import.js.out
diff --git a/tests/042_dyn_import_evalcontext.test b/tests/042_dyn_import_evalcontext.test
index cfe0df3f2..ab1efc084 100644
--- a/tests/042_dyn_import_evalcontext.test
+++ b/tests/042_dyn_import_evalcontext.test
@@ -1,2 +1,2 @@
-args: run --reload tests/042_dyn_import_evalcontext.ts
+args: run --allow-read --reload tests/042_dyn_import_evalcontext.ts
 output: tests/042_dyn_import_evalcontext.ts.out
diff --git a/tests/error_014_catch_dynamic_import_error.test b/tests/error_014_catch_dynamic_import_error.test
index 8345ad1c8..13ece4843 100644
--- a/tests/error_014_catch_dynamic_import_error.test
+++ b/tests/error_014_catch_dynamic_import_error.test
@@ -1,2 +1,2 @@
-args: tests/error_014_catch_dynamic_import_error.js --reload
+args: tests/error_014_catch_dynamic_import_error.js --reload --allow-read
 output: tests/error_014_catch_dynamic_import_error.js.out
diff --git a/tests/error_015_dynamic_import_permissions.js b/tests/error_015_dynamic_import_permissions.js
new file mode 100644
index 000000000..3460ca787
--- /dev/null
+++ b/tests/error_015_dynamic_import_permissions.js
@@ -0,0 +1,3 @@
+(async () => {
+  await import("http://localhost:4545/tests/subdir/mod4.js");
+})();
diff --git a/tests/error_015_dynamic_import_permissions.out b/tests/error_015_dynamic_import_permissions.out
new file mode 100644
index 000000000..90ccd0d1a
--- /dev/null
+++ b/tests/error_015_dynamic_import_permissions.out
@@ -0,0 +1 @@
+error: Uncaught TypeError: permission denied
diff --git a/tests/error_015_dynamic_import_permissions.test b/tests/error_015_dynamic_import_permissions.test
new file mode 100644
index 000000000..62f31e6b3
--- /dev/null
+++ b/tests/error_015_dynamic_import_permissions.test
@@ -0,0 +1,4 @@
+args: --reload --no-prompt tests/error_015_dynamic_import_permissions.js
+output: tests/error_015_dynamic_import_permissions.out
+check_stderr: true
+exit_code: 1
diff --git a/tests/error_016_dynamic_import_permissions2.js b/tests/error_016_dynamic_import_permissions2.js
new file mode 100644
index 000000000..71c70815c
--- /dev/null
+++ b/tests/error_016_dynamic_import_permissions2.js
@@ -0,0 +1,5 @@
+// If this is executed with --allow-net but not --allow-read the following
+// import should cause a permission denied error.
+(async () => {
+  await import("http://localhost:4545/tests/subdir/evil_remote_import.js");
+})();
diff --git a/tests/error_016_dynamic_import_permissions2.out b/tests/error_016_dynamic_import_permissions2.out
new file mode 100644
index 000000000..f52186481
--- /dev/null
+++ b/tests/error_016_dynamic_import_permissions2.out
@@ -0,0 +1,2 @@
+[WILDCARD]
+error: Uncaught TypeError: permission denied
diff --git a/tests/error_016_dynamic_import_permissions2.test b/tests/error_016_dynamic_import_permissions2.test
new file mode 100644
index 000000000..0fc514f0f
--- /dev/null
+++ b/tests/error_016_dynamic_import_permissions2.test
@@ -0,0 +1,5 @@
+# We have an allow-net flag but not allow-read, it should still result in error.
+args: --no-prompt --reload --allow-net tests/error_016_dynamic_import_permissions2.js
+output: tests/error_016_dynamic_import_permissions2.out
+check_stderr: true
+exit_code: 1
diff --git a/tests/subdir/evil_remote_import.js b/tests/subdir/evil_remote_import.js
new file mode 100644
index 000000000..4ff7d1b97
--- /dev/null
+++ b/tests/subdir/evil_remote_import.js
@@ -0,0 +1,4 @@
+// We expect to get a permission denied error if we dynamically
+// import this module without --allow-read.
+export * from "file:///c:/etc/passwd";
+console.log("Hello from evil_remote_import.js");