Path traversal / Improper subpath resolution in exports/imports handling

PoC (illustrative, environment-dependent): Goal: Demonstrate how an attacker could have exploited an unguarded exports subpath to load code from outside the package root before this fix. Setup (simulate in a local environment, e.g., a test workspace with a vulnerable turbopack resolution): 1) Create a local package with a subpath export that points to a file outside the package root: - directory structure: /workspace /node_modules /vuln-pkg package.json index.js /evil index.js - vuln-pkg/package.json { "name": "vuln-pkg", "version": "1.0.0", "exports": { "./sub": "./../evil/index.js" // a path that traverses outside the package, potentially loaded by resolver } } - evil/index.js module.exports = function(){ return 'ATTACKED'; }; - vuln-pkg/index.js module.exports = require('./sub'); 2) In a Next.js/Turbopack-based app, attempt to import the subpath: - import vuln from 'vuln-pkg/sub'; 3) Before the fix (vulnerability present): - Turbopack could resolve the exports subpath to the external file (evil/index.js) due to the lack of strict prohibition on path traversal in subpath resolution, loading and executing the attacker-controlled code. You would see the attacker’s code run (e.g., ATTACKED printed or side effects executed). 4) After the fix (this commit): - The resolver differentiates Export vs Import paths and ensures exports subpath resolution uses a relative path that begins with "./". An exports entry like "./sub": "./../evil/index.js" would be treated with a constrained request and not allowed to resolve to an external path in the same insecure manner, preventing loading of the attacker-controlled file. An attacker-controlled external path through exports would be blocked or normalized to a safe within-package path. Notes: - The exact runtime behavior depends on the bundler’s resolution and normalization of subpath exports. The PoC demonstrates the vulnerability concept (path traversal via exports) and shows how the fix prevents loading external paths via subpath exports by ensuring relative, properly scoped resolution for exports and by clearly separating import vs export handling. - In a real security test, you would set up a controlled environment with a vulnerable package exporting a subpath that points outside the package root and verify that, prior to this fix, the external file could be loaded, whereas after the fix, such external resolution is disallowed or properly sandboxed.

diff --git a/turbopack/crates/turbopack-core/src/resolve/mod.rs b/turbopack/crates/turbopack-core/src/resolve/mod.rs index 1ccb41ec1004..42745cc4b9ab 100644 --- a/turbopack/crates/turbopack-core/src/resolve/mod.rs +++ b/turbopack/crates/turbopack-core/src/resolve/mod.rs @@ -47,7 +47,7 @@ use crate::{ parse::{Request, stringify_data_uri}, pattern::{Pattern, PatternMatch, read_matches}, plugin::{AfterResolvePlugin, AfterResolvePluginCondition, BeforeResolvePlugin}, - remap::{ExportsField, ImportsField, ReplacedSubpathValueResult}, + remap::{ExportImport, ExportsField, ImportsField, ReplacedSubpathValueResult}, }, source::Source, }; @@ -2940,6 +2940,7 @@ async fn resolve_into_package( conditions, unspecified_conditions, query, + ExportImport::Export, ) .await?, ); @@ -3215,6 +3216,7 @@ async fn handle_exports_imports_field( conditions: &BTreeMap<RcStr, ConditionValue>, unspecified_conditions: &ConditionValue, query: RcStr, + ty: ExportImport, ) -> Result<Vc<ResolveResult>> { let mut results = Vec::new(); let mut conditions_state = FxHashMap::default(); @@ -3248,57 +3250,59 @@ async fn handle_exports_imports_field( map_key, } in results { - if let Some(result_path) = result_path.with_normalized_path() { - let request = *Request::parse(Pattern::Concatenation(vec![ - Pattern::Constant(rcstr!("./")), - result_path.clone(), - ])) - .to_resolved() - .await?; + let request = match ty { + ExportImport::Export => { + // Only relative paths are allowed in exports fields + Pattern::Concatenation(vec![Pattern::Constant(rcstr!("./")), result_path.clone()]) + } + ExportImport::Import => result_path.clone(), + }; + let request = *Request::parse(request).to_resolved().await?; - let resolve_result = Box::pin(resolve_internal_inline( - package_path.clone(), - request, - options, - )) - .await?; + let resolve_result = Box::pin(resolve_internal_inline( + package_path.clone(), + request, + options, + )) + .await?; - let resolve_result = if let Some(req) = req.as_constant_string() { - resolve_result.with_request(req.clone()) - } else { - match map_key { - AliasKey::Exact => resolve_result.with_request(map_prefix.clone().into()), - AliasKey::Wildcard { .. } => { - // - `req` is the user's request (key of the export map) - // - `result_path` is the final request (value of the export map), so - // effectively `'{foo}*{bar}'` - - // Because of the assertion in AliasMapLookupIterator, `req` is of the - // form: - // - "prefix...<dynamic>" or - // - "prefix...<dynamic>...suffix" - - let mut old_request_key = result_path; - // Remove the Pattern::Constant(rcstr!("./")), from above again + let resolve_result = if let Some(req) = req.as_constant_string() { + resolve_result.with_request(req.clone()) + } else { + match map_key { + AliasKey::Exact => resolve_result.with_request(map_prefix.clone().into()), + AliasKey::Wildcard { .. } => { + // - `req` is the user's request (key of the export map) + // - `result_path` is the final request (value of the export map), so + // effectively `'{foo}*{bar}'` + + // Because of the assertion in AliasMapLookupIterator, `req` is of the + // form: + // - "prefix...<dynamic>" or + // - "prefix...<dynamic>...suffix" + + let mut old_request_key = result_path; + if matches!(ty, ExportImport::Export) { + // Remove the Pattern::Constant(rcstr!("./")) from above again old_request_key.push_front(rcstr!("./").into()); - let new_request_key = req.clone(); - - resolve_result.with_replaced_request_key_pattern( - Pattern::new(old_request_key), - Pattern::new(new_request_key), - ) } + let new_request_key = req.clone(); + + resolve_result.with_replaced_request_key_pattern( + Pattern::new(old_request_key), + Pattern::new(new_request_key), + ) } - }; + } + }; - let resolve_result = if !conditions.is_empty() { - let resolve_result = resolve_result.await?.with_conditions(&conditions); - resolve_result.cell() - } else { - resolve_result - }; - resolved_results.push(resolve_result); - } + let resolve_result = if !conditions.is_empty() { + let resolve_result = resolve_result.await?.with_conditions(&conditions); + resolve_result.cell() + } else { + resolve_result + }; + resolved_results.push(resolve_result); } // other options do not apply anymore when an exports field exist @@ -3356,6 +3360,7 @@ async fn resolve_package_internal_with_imports_field( conditions, unspecified_conditions, RcStr::default(), + ExportImport::Import, ) .await } diff --git a/turbopack/crates/turbopack-core/src/resolve/remap.rs b/turbopack/crates/turbopack-core/src/resolve/remap.rs index 7d28032dd3fb..317f37199af6 100644 --- a/turbopack/crates/turbopack-core/src/resolve/remap.rs +++ b/turbopack/crates/turbopack-core/src/resolve/remap.rs @@ -16,7 +16,7 @@ use crate::resolve::{ /// A small helper type to differentiate parsing exports and imports fields. #[derive(Copy, Clone)] -enum ExportImport { +pub(crate) enum ExportImport { Export, Import, } diff --git a/turbopack/crates/turbopack-tests/tests/snapshot/imports/subpath-imports-nested/input/foo.js b/turbopack/crates/turbopack-tests/tests/execution/turbopack/resolving/subpath-imports-nested/input/foo.js similarity index 100% rename from turbopack/crates/turbopack-tests/tests/snapshot/imports/subpath-imports-nested/input/foo.js rename to turbopack/crates/turbopack-tests/tests/execution/turbopack/resolving/subpath-imports-nested/input/foo.js diff --git a/turbopack/crates/turbopack-tests/tests/execution/turbopack/resolving/subpath-imports-nested/input/index.js b/turbopack/crates/turbopack-tests/tests/execution/turbopack/resolving/subpath-imports-nested/input/index.js new file mode 100644 index 000000000000..6aae94afa607 --- /dev/null +++ b/turbopack/crates/turbopack-tests/tests/execution/turbopack/resolving/subpath-imports-nested/input/index.js @@ -0,0 +1,5 @@ +import foo from './nested' + +it('should resolve nested subpath imports', () => { + expect(foo).toBe('foo') +}) diff --git a/turbopack/crates/turbopack-tests/tests/snapshot/imports/subpath-imports-nested/input/nested/index.js b/turbopack/crates/turbopack-tests/tests/execution/turbopack/resolving/subpath-imports-nested/input/nested/index.js similarity index 100% rename from turbopack/crates/turbopack-tests/tests/snapshot/imports/subpath-imports-nested/input/nested/index.js rename to turbopack/crates/turbopack-tests/tests/execution/turbopack/resolving/subpath-imports-nested/input/nested/index.js diff --git a/turbopack/crates/turbopack-tests/tests/snapshot/imports/subpath-imports-nested/input/package.json b/turbopack/crates/turbopack-tests/tests/execution/turbopack/resolving/subpath-imports-nested/input/package.json similarity index 100% rename from turbopack/crates/turbopack-tests/tests/snapshot/imports/subpath-imports-nested/input/package.json rename to turbopack/crates/turbopack-tests/tests/execution/turbopack/resolving/subpath-imports-nested/input/package.json diff --git a/turbopack/crates/turbopack-tests/tests/snapshot/imports/subpath-imports/input/foo.js b/turbopack/crates/turbopack-tests/tests/execution/turbopack/resolving/subpath-imports/input/foo.js similarity index 100% rename from turbopack/crates/turbopack-tests/tests/snapshot/imports/subpath-imports/input/foo.js rename to turbopack/crates/turbopack-tests/tests/execution/turbopack/resolving/subpath-imports/input/foo.js diff --git a/turbopack/crates/turbopack-tests/tests/snapshot/imports/subpath-imports/input/import.mjs b/turbopack/crates/turbopack-tests/tests/execution/turbopack/resolving/subpath-imports/input/import.mjs similarity index 100% rename from turbopack/crates/turbopack-tests/tests/snapshot/imports/subpath-imports/input/import.mjs rename to turbopack/crates/turbopack-tests/tests/execution/turbopack/resolving/subpath-imports/input/import.mjs diff --git a/turbopack/crates/turbopack-tests/tests/execution/turbopack/resolving/subpath-imports/input/index.js b/turbopack/crates/turbopack-tests/tests/execution/turbopack/resolving/subpath-imports/input/index.js new file mode 100644 index 000000000000..8a2ad06b7868 --- /dev/null +++ b/turbopack/crates/turbopack-tests/tests/execution/turbopack/resolving/subpath-imports/input/index.js @@ -0,0 +1,13 @@ +import foo from '#foo' +import dep from '#dep' +import pattern from '#pattern/pat.js' +import conditionalImport from '#conditional' +const conditionalRequire = require('#conditional') + +it('should resolve subpath imports', () => { + expect(foo).toBe('foo') + expect(dep).toBe('dep') + expect(pattern).toBe('pat') + expect(conditionalImport).toBe('import') + expect(conditionalRequire).toBe('require') +}) diff --git a/turbopack/crates/turbopack-tests/tests/snapshot/imports/subpath-imports/input/package.json b/turbopack/crates/turbopack-tests/tests/execution/turbopack/resolving/subpath-imports/input/package.json similarity index 100% rename from turbopack/crates/turbopack-tests/tests/snapshot/imports/subpath-imports/input/package.json rename to turbopack/crates/turbopack-tests/tests/execution/turbopack/resolving/subpath-imports/input/package.json diff --git a/turbopack/crates/turbopack-tests/tests/snapshot/imports/subpath-imports/input/pat.js b/turbopack/crates/turbopack-tests/tests/execution/turbopack/resolving/subpath-imports/input/pat.js similarity index 100% rename from turbopack/crates/turbopack-tests/tests/snapshot/imports/subpath-imports/input/pat.js rename to turbopack/crates/turbopack-tests/tests/execution/turbopack/resolving/subpath-imports/input/pat.js diff --git a/turbopack/crates/turbopack-tests/tests/snapshot/imports/subpath-imports/input/require.cjs b/turbopack/crates/turbopack-tests/tests/execution/turbopack/resolving/subpath-imports/input/require.cjs similarity index 100% rename from turbopack/crates/turbopack-tests/tests/snapshot/imports/subpath-imports/input/require.cjs rename to turbopack/crates/turbopack-tests/tests/execution/turbopack/resolving/subpath-imports/input/require.cjs diff --git a/turbopack/crates/turbopack-tests/tests/snapshot/imports/subpath-imports-nested/input/index.js b/turbopack/crates/turbopack-tests/tests/snapshot/imports/subpath-imports-nested/input/index.js deleted file mode 100644 index 301fa979ead3..000000000000 --- a/turbopack/crates/turbopack-tests/tests/snapshot/imports/subpath-imports-nested/input/index.js +++ /dev/null @@ -1,3 +0,0 @@ -import foo from './nested' - -console.log(foo) diff --git a/turbopack/crates/turbopack-tests/tests/snapshot/imports/subpath-imports-nested/output/0_9x_turbopack-tests_tests_snapshot_imports_subpath-imports-nested_input_0w09esu._.js b/turbopack/crates/turbopack-tests/tests/snapshot/imports/subpath-imports-nested/output/0_9x_turbopack-tests_tests_snapshot_imports_subpath-imports-nested_input_0w09esu._.js deleted file mode 100644 index 4dafe4fa12ff..000000000000 --- a/turbopack/crates/turbopack-tests/tests/snapshot/imports/subpath-imports-nested/output/0_9x_turbopack-tests_tests_snapshot_imports_subpath-imports-nested_input_0w09esu._.js +++ /dev/null @@ -1,32 +0,0 @@ -(globalThis["TURBOPACK"] || (globalThis["TURBOPACK"] = [])).push(["output/0_9x_turbopack-tests_tests_snapshot_imports_subpath-imports-nested_input_0w09esu._.js", -"[project]/turbopack/crates/turbopack-tests/tests/snapshot/imports/subpath-imports-nested/input/foo.js [test] (ecmascript)", ((__turbopack_context__) => { -"use strict"; - -__turbopack_context__.s([ - "default", - ()=>__TURBOPACK__default__export__ -]); -const __TURBOPACK__default__export__ = 'foo'; -}), -"[project]/turbopack/crates/turbopack-tests/tests/snapshot/imports/subpath-imports-nested/input/nested/index.js [test] (ecmascript)", ((__turbopack_context__) => { -"use strict"; - -__turbopack_context__.s([ - "default", - ()=>__TURBOPACK__default__export__ -]); -var __TURBOPACK__imported__module__$5b$project$5d2f$turbopack$2f$crates$2f$turbopack$2d$tests$2f$tests$2f$snapshot$2f$imports$2f$subpath$2d$imports$2d$nested$2f$input$2f$foo$2e$js__$5b$test$5d$__$28$ecmascript$29$__ = __turbopack_context__.i("[project]/turbopack/crates/turbopack-tests/tests/snapshot/imports/subpath-imports-nested/input/foo.js [test] (ecmascript)"); -; -const __TURBOPACK__default__export__ = __TURBOPACK__imported__module__$5b$project$5d2f$turbopack$2f$crates$2f$turbopack$2d$tests$2f$tests$2f$snapshot$2f$imports$2f$subpath$2d$imports$2d$nested$2f$input$2f$foo$2e$js__$5b$test$5d$__$28$ecmascript$29$__["default"]; -}), -"[project]/turbopack/crates/turbopack-tests/tests/snapshot/imports/subpath-imports-nested/input/index.js [test] (ecmascript)", ((__turbopack_context__) => { -"use strict"; - -__turbopack_context__.s([]); -var __TURBOPACK__imported__module__$5b$project$5d2f$turbopack$2f$crates$2f$turbopack$2d$tests$2f$tests$2f$snapshot$2f$imports$2f$subpath$2d$imports$2d$nested$2f$input$2f$nested$2f$index$2e$js__$5b$test$5d$__$28$ecmascript$29$__ = __turbopack_context__.i("[project]/turbopack/crates/turbopack-tests/tests/snapshot/imports/subpath-imports-nested/input/nested/index.js [test] (ecmascript)"); -; -console.log(__TURBOPACK__imported__module__$5b$project$5d2f$turbopack$2f$crates$2f$turbopack$2d$tests$2f$tests$2f$snapshot$2f$imports$2f$subpath$2d$imports$2d$nested$2f$input$2f$nested$2f$index$2e$js__$5b$test$5d$__$28$ecmascript$29$__["default"]); -}), -]); - -//# sourceMappingURL=0_9x_turbopack-tests_tests_snapshot_imports_subpath-imports-nested_input_0w09esu._.js.map \ No newline at end of file diff --git a/turbop ... [truncated]