Skip to content

Commit 503359b

Browse files
committed
Harden source-map stack remapping safety
1 parent 65dc620 commit 503359b

4 files changed

Lines changed: 136 additions & 70 deletions

File tree

packages/react-on-rails-pro-node-renderer/src/worker.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,10 +196,10 @@ export function releaseExecutionContextWhenStreamFinishes(
196196
releaseExecutionContext();
197197
});
198198

199-
refreshResponseFinishTimeout();
200199
stream.once('close', endProgressStream);
201200
stream.once('error', forwardSourceError);
202201
stream.pipe(progressStream);
202+
refreshResponseFinishTimeout();
203203

204204
return {
205205
release,

packages/react-on-rails-pro-node-renderer/src/worker/vm.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -237,13 +237,13 @@ async function buildVM(filePath: string): Promise<VMContext> {
237237
const bundleContents = await readFileAsync(filePath, 'utf8');
238238
const firstLineColumnOffset =
239239
additionalContextIsObject || supportModules ? MODULE_WRAP_FIRST_LINE_PREFIX_LENGTH : 0;
240-
const preloadedSourceMapJson = await preloadSourceMapJsonForBundle(filePath, bundleContents);
240+
const preloadedSourceMap = await preloadSourceMapJsonForBundle(filePath, bundleContents);
241241
const currentSourceMapRegistration = registerBundleForSourceMaps(
242242
filePath,
243243
firstLineColumnOffset,
244244
bundleContents,
245-
preloadedSourceMapJson,
246-
preloadedSourceMapJson === undefined,
245+
preloadedSourceMap.sourceMapJson,
246+
preloadedSourceMap.retryMissingSourceMap,
247247
);
248248
sourceMapRegistration = currentSourceMapRegistration;
249249

packages/react-on-rails-pro-node-renderer/src/worker/vmSourceMapSupport.ts

Lines changed: 62 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ export interface BundleSourceMapRegistration {
6767
* a newer map.
6868
*/
6969
sourceMapJson?: string | null;
70+
realBundleDirectory: string;
7071
/**
7172
* External source maps can arrive after a bundle first becomes visible. When
7273
* true, a missing external map is retried briefly before caching the miss.
@@ -99,6 +100,7 @@ const sourceMapLookupAttempts = new WeakSet<SourceMapLookupAttempt>();
99100
let warnedMissingSourceMapConstructor = false;
100101

101102
const MAX_MISSING_SOURCE_MAP_RETRIES = 5;
103+
const MAX_INLINE_SOURCE_MAP_BYTES = 50 * 1024 * 1024;
102104

103105
function shouldRetryMissingSourceMap(registration: BundleSourceMapRegistration) {
104106
return registration.retryMissingSourceMap === true && !retiredMissingSourceMapRetries.has(registration);
@@ -193,14 +195,20 @@ function parseDataUrlSourceMap(url: string): string | undefined {
193195
return undefined;
194196
}
195197
const payload = url.slice(commaIndex + 1);
198+
if (payload.length > MAX_INLINE_SOURCE_MAP_BYTES) {
199+
return undefined;
200+
}
201+
196202
if (metadata.split(';').includes('base64')) {
197-
return Buffer.from(payload, 'base64').toString('utf8');
203+
const decoded = Buffer.from(payload, 'base64').toString('utf8');
204+
return decoded.length <= MAX_INLINE_SOURCE_MAP_BYTES ? decoded : undefined;
198205
}
199206

200207
try {
201-
return decodeURIComponent(payload);
208+
const decoded = decodeURIComponent(payload);
209+
return decoded.length <= MAX_INLINE_SOURCE_MAP_BYTES ? decoded : undefined;
202210
} catch {
203-
return payload;
211+
return payload.length <= MAX_INLINE_SOURCE_MAP_BYTES ? payload : undefined;
204212
}
205213
}
206214

@@ -282,29 +290,24 @@ function candidateSourceMapPaths(bundleFilePath: string, sourceMappingUrl: strin
282290
return Array.from(new Set(candidatePaths));
283291
}
284292

285-
function resolveReadableSourceMapPath(bundleFilePath: string, candidatePath: string) {
293+
function resolveReadableSourceMapPath(
294+
bundleFilePath: string,
295+
candidatePath: string,
296+
realBundleDirectory: string,
297+
) {
286298
const bundleDirectory = path.dirname(bundleFilePath);
287299
try {
288300
const resolvedPath = path.resolve(candidatePath);
289301
if (!isPathInsideOrEqual(resolvedPath, bundleDirectory)) {
290302
return undefined;
291303
}
292304

293-
const realBundleDirectory = fs.realpathSync(bundleDirectory);
294305
const realSourceMapPath = fs.realpathSync(resolvedPath);
295306
if (!isPathInsideOrEqual(realSourceMapPath, realBundleDirectory)) {
296-
const linkStats = fs.lstatSync(resolvedPath);
297-
const targetStats = fs.statSync(resolvedPath);
298-
// Pro pre-stage symlink mode creates trusted symlink entries inside the
299-
// bundle directory. The sourceMappingURL still has to be a plain file name.
300-
// SECURITY: that bundle directory must not be writable by untrusted
301-
// parties; an attacker-controlled symlink would let the loader read any
302-
// file the renderer process can access. The symlink path is returned for
303-
// read-time compatibility with trusted Pro pre-stage tooling, so this does
304-
// not defend against TOCTOU changes by an untrusted bundle-directory writer.
305-
if (linkStats.isSymbolicLink() && targetStats.isFile()) {
306-
return resolvedPath;
307-
}
307+
return undefined;
308+
}
309+
310+
if (!fs.statSync(realSourceMapPath).isFile()) {
308311
return undefined;
309312
}
310313

@@ -314,33 +317,38 @@ function resolveReadableSourceMapPath(bundleFilePath: string, candidatePath: str
314317
}
315318
}
316319

317-
function readSourceMapFile(bundleFilePath: string, candidatePath: string) {
318-
const sourceMapPath = resolveReadableSourceMapPath(bundleFilePath, candidatePath);
320+
function readSourceMapFile(bundleFilePath: string, candidatePath: string, realBundleDirectory: string) {
321+
const sourceMapPath = resolveReadableSourceMapPath(bundleFilePath, candidatePath, realBundleDirectory);
319322
if (!sourceMapPath) {
320323
return undefined;
321324
}
322325

323-
// `sourceMapPath` is filename-only and either realpath-checked under the bundle
324-
// directory or a symlink entry staged inside that directory by trusted Pro tooling.
326+
// `sourceMapPath` is filename-only and read through a final realpath that must
327+
// stay inside the real bundle directory, closing symlink-swap races.
325328
// codeql[js/path-injection]
326329
return fs.readFileSync(sourceMapPath, 'utf8');
327330
}
328331

329-
async function readSourceMapFileAsync(bundleFilePath: string, candidatePath: string) {
330-
const sourceMapPath = resolveReadableSourceMapPath(bundleFilePath, candidatePath);
332+
async function readSourceMapFileAsync(
333+
bundleFilePath: string,
334+
candidatePath: string,
335+
realBundleDirectory: string,
336+
) {
337+
const sourceMapPath = resolveReadableSourceMapPath(bundleFilePath, candidatePath, realBundleDirectory);
331338
if (!sourceMapPath) {
332339
return undefined;
333340
}
334341

335-
// `sourceMapPath` is filename-only and either realpath-checked under the bundle
336-
// directory or a symlink entry staged inside that directory by trusted Pro tooling.
342+
// `sourceMapPath` is filename-only and read through a final realpath that must
343+
// stay inside the real bundle directory, closing symlink-swap races.
337344
// codeql[js/path-injection]
338345
return fs.promises.readFile(sourceMapPath, 'utf8');
339346
}
340347

341348
function readSourceMapJsonForBundle(
342349
bundleFilePath: string,
343350
sourceMappingUrl: string | undefined,
351+
realBundleDirectory: string,
344352
): string | undefined {
345353
try {
346354
// Stack formatting is synchronous, so source-map discovery stays sync and
@@ -353,7 +361,7 @@ function readSourceMapJsonForBundle(
353361
// uploaded alongside it under that name is also worth checking.
354362
const candidatePaths = candidateSourceMapPaths(bundleFilePath, sourceMappingUrl);
355363
for (const candidatePath of candidatePaths) {
356-
const sourceMapJson = readSourceMapFile(bundleFilePath, candidatePath);
364+
const sourceMapJson = readSourceMapFile(bundleFilePath, candidatePath, realBundleDirectory);
357365
if (sourceMapJson) {
358366
return sourceMapJson;
359367
}
@@ -368,6 +376,7 @@ function readSourceMapJsonForBundle(
368376
async function readSourceMapJsonForBundleAsync(
369377
bundleFilePath: string,
370378
sourceMappingUrl: string | undefined,
379+
realBundleDirectory: string,
371380
): Promise<string | undefined> {
372381
try {
373382
if (sourceMappingUrl && sourceMappingUrl.startsWith('data:')) {
@@ -377,7 +386,7 @@ async function readSourceMapJsonForBundleAsync(
377386
const candidatePaths = candidateSourceMapPaths(bundleFilePath, sourceMappingUrl);
378387
for (const candidatePath of candidatePaths) {
379388
// eslint-disable-next-line no-await-in-loop
380-
const sourceMapJson = await readSourceMapFileAsync(bundleFilePath, candidatePath);
389+
const sourceMapJson = await readSourceMapFileAsync(bundleFilePath, candidatePath, realBundleDirectory);
381390
if (sourceMapJson) {
382391
return sourceMapJson;
383392
}
@@ -392,20 +401,38 @@ async function readSourceMapJsonForBundleAsync(
392401
/** @internal Used by VM build to avoid synchronous external-map reads. */
393402
export async function preloadSourceMapJsonForBundle(bundleFilePath: string, bundleContents: string) {
394403
const sourceMappingUrl = extractSourceMappingUrl(bundleContents);
404+
const realBundleDirectory = fs.realpathSync(path.dirname(bundleFilePath));
395405
if (sourceMappingUrl && sourceMappingUrl.startsWith('data:')) {
396-
return parseDataUrlSourceMap(sourceMappingUrl) ?? null;
406+
return {
407+
retryMissingSourceMap: false,
408+
sourceMapJson: parseDataUrlSourceMap(sourceMappingUrl) ?? null,
409+
};
397410
}
398411

399412
// External maps can arrive just after the bundle during upload/pre-stage flows.
400413
// Leave misses lazy; VM registrations mark them retryable so a first error
401414
// before the map copy finishes does not cache a permanent miss.
402-
const sourceMapJson = await readSourceMapJsonForBundleAsync(bundleFilePath, sourceMappingUrl);
415+
const candidatePaths = candidateSourceMapPaths(bundleFilePath, sourceMappingUrl);
416+
const existingCandidate = candidatePaths.some((candidatePath) =>
417+
Boolean(resolveReadableSourceMapPath(bundleFilePath, candidatePath, realBundleDirectory)),
418+
);
419+
const sourceMapJson = await readSourceMapJsonForBundleAsync(
420+
bundleFilePath,
421+
sourceMappingUrl,
422+
realBundleDirectory,
423+
);
403424
if (sourceMapJson !== undefined && !isValidJson(sourceMapJson)) {
404425
log.debug('Preloaded source map for bundle %s is not valid JSON yet; retrying lazily.', bundleFilePath);
405-
return undefined;
426+
return { retryMissingSourceMap: true, sourceMapJson: undefined };
406427
}
407428

408-
return sourceMapJson;
429+
return {
430+
// Without a sourceMappingURL or an observed fallback map, do not keep
431+
// retrying every error-path stack lookup for bundles that have no map.
432+
retryMissingSourceMap:
433+
sourceMapJson === undefined && (sourceMappingUrl !== undefined || existingCandidate),
434+
sourceMapJson,
435+
};
409436
}
410437

411438
/**
@@ -434,6 +461,7 @@ export function registerBundleForSourceMaps(
434461
const registration = {
435462
bundleFilePath,
436463
firstLineColumnOffset,
464+
realBundleDirectory: fs.realpathSync(path.dirname(bundleFilePath)),
437465
sourceMappingUrl,
438466
sourceMapJson:
439467
preloadedSourceMapJson !== undefined
@@ -530,7 +558,7 @@ function loadSourceMapForBundle(
530558

531559
const sourceMapJson =
532560
registration.sourceMapJson === undefined
533-
? readSourceMapJsonForBundle(bundleFilePath, sourceMappingUrl)
561+
? readSourceMapJsonForBundle(bundleFilePath, sourceMappingUrl, registration.realBundleDirectory)
534562
: (registration.sourceMapJson ?? undefined);
535563

536564
if (!sourceMapJson) {
@@ -644,6 +672,7 @@ export function resolveOriginalPositionForRegistration(
644672
const entry = sourceMap.sourceMap.findEntry(lineNumber - 1, zeroBasedColumn) as Partial<SourceMapping>;
645673
if (
646674
entry.originalSource === undefined ||
675+
entry.originalSource === '' ||
647676
entry.originalLine === undefined ||
648677
entry.generatedLine !== lineNumber - 1
649678
) {
@@ -733,15 +762,7 @@ export function remapStackTrace(
733762
return remapStackTraceForRegistration(stack, registration, lookupAttempt);
734763
}
735764

736-
// Callers without a specific registration opt into scanning every registered
737-
// bundle path in the stack text. Request paths pass a registration-specific
738-
// closure so unrelated bundle registrations cannot rewrite their stacks.
739-
let remappedStack = stack;
740-
registeredBundles.forEach((currentRegistration) => {
741-
remappedStack = remapStackTraceForRegistration(remappedStack, currentRegistration, lookupAttempt);
742-
});
743-
744-
return remappedStack;
765+
return undefined;
745766
}
746767

747768
export function remapErrorStack(error: unknown, registration?: BundleSourceMapRegistration) {

0 commit comments

Comments
 (0)