Skip to content

Commit f05f501

Browse files
sapphi-redOchk0dominikg
authored
fix: disallow referencing files outside the package from sourcemap (#22158)
Co-authored-by: Ochk0 <120699601+Ochk0@users.noreply.github.com> Co-authored-by: "Dominik G." <dominik.goepel@gmx.de>
1 parent fd6ab02 commit f05f501

16 files changed

Lines changed: 269 additions & 13 deletions

File tree

packages/vite/src/node/plugins/forwardConsole.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,11 @@ function formatError(
9292
if (result && !result.map) {
9393
try {
9494
const filePath = id.split('?')[0]
95-
const extracted = extractSourcemapFromFile(result.code, filePath)
95+
const extracted = extractSourcemapFromFile(
96+
result.code,
97+
filePath,
98+
environment.config.logger,
99+
)
96100
sourceMapCache.set(id, extracted?.map)
97101
return extracted?.map
98102
} catch {
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
import { describe, expect, test } from 'vitest'
2+
import { getNodeModulesPackageRoot } from '../sourcemap'
3+
import { isWindows } from '../../../shared/utils'
4+
5+
describe('getNodeModulesPackageRoot', () => {
6+
const cases = [
7+
{
8+
name: 'returns undefined for path outside node_modules',
9+
input: '/project/src/foo.ts',
10+
expected: undefined,
11+
},
12+
{
13+
name: 'returns undefined for plain filename',
14+
input: 'foo.js',
15+
expected: undefined,
16+
},
17+
{
18+
name: 'unscoped package',
19+
input: '/project/node_modules/foo/index.js',
20+
expected: '/project/node_modules/foo',
21+
},
22+
{
23+
name: 'unscoped package in nested directory',
24+
input: '/project/node_modules/foo/dist/bar.js',
25+
expected: '/project/node_modules/foo',
26+
},
27+
{
28+
name: 'scoped package',
29+
input: '/project/node_modules/@scope/pkg/dist/foo.js',
30+
expected: '/project/node_modules/@scope/pkg',
31+
},
32+
{
33+
name: 'scoped package at root level',
34+
input: '/project/node_modules/@scope/pkg/index.js',
35+
expected: '/project/node_modules/@scope/pkg',
36+
},
37+
{
38+
name: 'nested node_modules uses the last segment',
39+
input: '/project/node_modules/foo/node_modules/bar/index.js',
40+
expected: '/project/node_modules/foo/node_modules/bar',
41+
},
42+
{
43+
name: 'Windows-style path',
44+
input: 'D:\\project\\node_modules\\foo\\dist\\bar.js',
45+
expected: 'D:/project/node_modules/foo',
46+
skip: !isWindows,
47+
},
48+
{
49+
name: 'Windows-style path with scoped package',
50+
input: 'D:\\project\\node_modules\\@scope\\pkg\\index.js',
51+
expected: 'D:/project/node_modules/@scope/pkg',
52+
skip: !isWindows,
53+
},
54+
{
55+
name: 'package name without subdirectory',
56+
input: '/project/node_modules/foo',
57+
expected: '/project/node_modules/foo',
58+
},
59+
{
60+
name: 'scoped package name without subdirectory',
61+
input: '/project/node_modules/@scope/pkg',
62+
expected: '/project/node_modules/@scope/pkg',
63+
},
64+
]
65+
66+
for (const { name, input, expected, skip } of cases) {
67+
test.skipIf(skip)(name, () => {
68+
expect(getNodeModulesPackageRoot(input)).toBe(expected)
69+
})
70+
}
71+
})

packages/vite/src/node/server/sourcemap.ts

Lines changed: 71 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,47 @@ import fs from 'node:fs'
33
import fsp from 'node:fs/promises'
44
import convertSourceMap from 'convert-source-map'
55
import type { ExistingRawSourceMap, SourceMap } from 'rolldown'
6+
import colors from 'picocolors'
67
import type { Logger } from '../logger'
7-
import { blankReplacer, createDebugger } from '../utils'
8+
import {
9+
blankReplacer,
10+
createDebugger,
11+
isParentDirectory,
12+
normalizePath,
13+
} from '../utils'
814
import { cleanUrl } from '../../shared/utils'
915

1016
const debug = createDebugger('vite:sourcemap', {
1117
onlyWhenFocused: true,
1218
})
1319

20+
/**
21+
* Given a file path inside node_modules, returns the package root directory.
22+
* For scoped packages like `node_modules/@scope/pkg/dist/foo.js`, returns `node_modules/@scope/pkg`.
23+
* Returns `undefined` if the file is not inside node_modules.
24+
*/
25+
export function getNodeModulesPackageRoot(
26+
filePath: string,
27+
): string | undefined {
28+
const normalized = normalizePath(filePath)
29+
const nodeModulesIndex = normalized.lastIndexOf('/node_modules/')
30+
if (nodeModulesIndex === -1) return undefined
31+
32+
const packageStart = nodeModulesIndex + '/node_modules/'.length
33+
const rest = normalized.slice(packageStart)
34+
const firstSlash = rest.indexOf('/')
35+
36+
let packageName: string
37+
if (rest.startsWith('@')) {
38+
// scoped package: @scope/pkg
39+
const secondSlash = rest.indexOf('/', firstSlash + 1)
40+
packageName = secondSlash === -1 ? rest : rest.slice(0, secondSlash)
41+
} else {
42+
packageName = firstSlash === -1 ? rest : rest.slice(0, firstSlash)
43+
}
44+
return normalized.slice(0, packageStart) + packageName
45+
}
46+
1447
// Virtual modules should be prefixed with a null byte to avoid a
1548
// false positive "missing source" warning. We also check for certain
1649
// prefixes used for special handling in esbuildDepPlugin.
@@ -40,6 +73,7 @@ export async function injectSourcesContent(
4073
): Promise<void> {
4174
let sourceRootPromise: Promise<string | undefined>
4275

76+
const packageRoot = getNodeModulesPackageRoot(file)
4377
const missingSources: string[] = []
4478
const sourcesContent = map.sourcesContent || []
4579
const sourcesContentPromises: Promise<void>[] = []
@@ -59,7 +93,22 @@ export async function injectSourcesContent(
5993
if (sourceRoot) {
6094
resolvedSourcePath = path.resolve(sourceRoot, resolvedSourcePath)
6195
}
62-
96+
// Block path traversal outside the package boundary for node_modules
97+
// A malicious package may point to a sensitive file
98+
if (packageRoot) {
99+
const resolvedSourcePathNormalized = normalizePath(
100+
path.resolve(resolvedSourcePath),
101+
)
102+
if (!isParentDirectory(packageRoot, resolvedSourcePathNormalized)) {
103+
sourcesContent[index] = null
104+
logger.warnOnce(
105+
colors.yellow(
106+
`Sourcemap for ${JSON.stringify(file)} points to a source file outside its package: ${JSON.stringify(resolvedSourcePathNormalized)}`,
107+
),
108+
)
109+
return
110+
}
111+
}
63112
sourcesContent[index] = await fsp
64113
.readFile(resolvedSourcePath, 'utf-8')
65114
.catch(() => {
@@ -153,12 +202,13 @@ export function applySourcemapIgnoreList(
153202
export function extractSourcemapFromFile(
154203
code: string,
155204
filePath: string,
205+
logger: Logger,
156206
): { code: string; map: SourceMap } | undefined {
157207
const map = (
158208
convertSourceMap.fromSource(code) ||
159209
convertSourceMap.fromMapFileSource(
160210
code,
161-
createConvertSourceMapReadMap(filePath),
211+
createConvertSourceMapReadMap(filePath, logger),
162212
)
163213
)?.toObject()
164214

@@ -170,11 +220,24 @@ export function extractSourcemapFromFile(
170220
}
171221
}
172222

173-
function createConvertSourceMapReadMap(originalFileName: string) {
223+
function createConvertSourceMapReadMap(
224+
originalFileName: string,
225+
logger: Logger,
226+
) {
227+
const packageRoot = getNodeModulesPackageRoot(originalFileName)
174228
return (filename: string) => {
175-
return fs.readFileSync(
176-
path.resolve(path.dirname(originalFileName), filename),
177-
'utf-8',
178-
)
229+
const resolvedPath = path.resolve(path.dirname(originalFileName), filename)
230+
if (
231+
packageRoot &&
232+
!isParentDirectory(packageRoot, normalizePath(resolvedPath))
233+
) {
234+
logger.warnOnce(
235+
colors.yellow(
236+
`Sourcemap in "${originalFileName}" references a map file outside its package: "${filename}"`,
237+
),
238+
)
239+
return '{}'
240+
}
241+
return fs.readFileSync(resolvedPath, 'utf-8')
179242
}
180243
}

packages/vite/src/node/server/transformRequest.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,7 @@ async function loadAndTransform(
293293
}
294294
if (code) {
295295
try {
296-
const extracted = extractSourcemapFromFile(code, file)
296+
const extracted = extractSourcemapFromFile(code, file, logger)
297297
if (extracted) {
298298
code = extracted.code
299299
map = extracted.map

playground/js-sourcemap/__tests__/js-sourcemap.spec.ts

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,14 @@ import {
1616
serverLogs,
1717
} from '~utils'
1818

19+
function createMapFileReader(moduleUrl: string) {
20+
return async (filename: string): Promise<string> => {
21+
const base = new URL(moduleUrl, page.url())
22+
const res = await page.request.get(new URL(filename, base).href)
23+
return res.text()
24+
}
25+
}
26+
1927
if (!isBuild) {
2028
test('js', async () => {
2129
const res = await page.request.get(new URL('./foo.js', page.url()).href)
@@ -144,6 +152,44 @@ if (!isBuild) {
144152
expect(log).not.toMatch(/Sourcemap for .+ points to missing source files/)
145153
})
146154
})
155+
156+
test('should not leak file contents via sourcemap path traversal in node_modules', async () => {
157+
const res = await page.request.get(
158+
new URL('./malicious-import.js', page.url()).href,
159+
)
160+
const js = await res.text()
161+
// Find the rewritten import URL for the malicious dep
162+
const depUrlMatch = js.match(/from\s+"([^"]*malicious-sourcemap[^"]*)"/)
163+
expect(depUrlMatch).toBeTruthy()
164+
const depUrl = depUrlMatch![1]
165+
const depRes = await page.request.get(new URL(depUrl, page.url()).href)
166+
const depJs = await depRes.text()
167+
const map = extractSourcemap(depJs)
168+
expect(map.sourcesContent).toBeDefined()
169+
expect(map.sourcesContent).not.toContainEqual(
170+
expect.stringContaining('defineConfig'),
171+
)
172+
})
173+
174+
test('should not leak file contents via sourcemap path traversal in optimized deps', async () => {
175+
const res = await page.request.get(
176+
new URL('./optimized-malicious-import.js', page.url()).href,
177+
)
178+
const js = await res.text()
179+
// Find the rewritten import URL for the optimized malicious dep
180+
const depUrlMatch = js.match(/from\s+"([^"]*optimized-malicious[^"]*)"/)
181+
expect(depUrlMatch).toBeTruthy()
182+
const depUrl = depUrlMatch![1]
183+
// Ensure the dep was actually optimized (served from .vite/deps)
184+
expect(depUrl).toContain('.vite/deps')
185+
const depRes = await page.request.get(new URL(depUrl, page.url()).href)
186+
const depJs = await depRes.text()
187+
const map = await extractSourcemap(depJs, createMapFileReader(depUrl))
188+
expect(map.sourcesContent).toBeDefined()
189+
expect(map.sourcesContent).not.toContainEqual(
190+
expect.stringContaining('defineConfig'),
191+
)
192+
})
147193
}
148194

149195
describe.runIf(isBuild)('build tests', () => {

playground/js-sourcemap/dep-malicious-sourcemap/index.js

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"name": "@vitejs/test-dep-malicious-sourcemap",
3+
"private": true,
4+
"version": "0.0.0",
5+
"type": "module",
6+
"main": "index.js"
7+
}

playground/js-sourcemap/dep-optimized-malicious/index.js

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"name": "@vitejs/test-dep-optimized-malicious",
3+
"private": true,
4+
"version": "0.0.0",
5+
"main": "index.js"
6+
}

playground/js-sourcemap/index.html

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,3 +11,5 @@ <h1>JS Sourcemap</h1>
1111
<script type="module" src="./with-multiline-import.ts"></script>
1212
<script type="module" src="./zoo.js"></script>
1313
<script type="module" src="./with-define-object.ts"></script>
14+
<script type="module" src="./malicious-import.js"></script>
15+
<script type="module" src="./optimized-malicious-import.js"></script>

0 commit comments

Comments
 (0)