From 7857a950eaa55d132e93aa072a22c5c7943c8f08 Mon Sep 17 00:00:00 2001 From: Sahinur Islam Date: Mon, 19 Oct 2026 10:43:07 +0600 Subject: [PATCH 1/2] Keep Storage emulator object keys literal instead of path.join-normalizing them The Storage emulator checked security rules against the requested object name but stored and looked up objects under path.join(bucket, object), which collapses '.' and '..' segments. A name like users/A/../B/photo.png was authorized as users/A/... and then read, overwrote or deleted users/B/photo.png. Cloud Storage treats object names literally, so key objects by bucket and literal name instead. Imports of old Windows exports, whose names use backslash separators, are still mapped to '/'. Fixes #11301 --- CHANGELOG.md | 1 + src/emulator/storage/files.spec.ts | 126 +++++++++++++++++++++++++++++ src/emulator/storage/files.ts | 15 ++-- 3 files changed, 137 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b1192375095..a02aa68cf26 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1 +1,2 @@ - Ensured the Firebase Hosting API is enabled before deploying and running other Hosting commands. +- Fixed the Storage emulator storing objects whose names contain `.` or `..` segments under the normalized name, so they could read, overwrite or delete a different object. (#11301) diff --git a/src/emulator/storage/files.spec.ts b/src/emulator/storage/files.spec.ts index 5e6fdcd4444..f227e5e3dfd 100644 --- a/src/emulator/storage/files.spec.ts +++ b/src/emulator/storage/files.spec.ts @@ -1,5 +1,7 @@ import { expect } from "chai"; +import { mkdtempSync } from "fs"; import { tmpdir } from "os"; +import * as path from "path"; import { StoredFileMetadata } from "./metadata"; import { StorageCloudFunctions } from "./cloudFunctions"; @@ -171,6 +173,130 @@ describe("files", () => { }); }); + describe("object names with dot segments", () => { + // Cloud Storage object names are literal, so "a/../b" and "b" are two different objects. + // These upload with the decoded name, the way the Firebase API handler passes it on. + async function uploadDecoded( + storageLayer: StorageLayer, + bucketId: string, + objectId: string, + data: string, + ) { + const upload = _uploadService.multipartUpload({ + bucketId, + objectId, + dataRaw: Buffer.from(data), + metadata: {}, + }); + await storageLayer.uploadObject(upload); + } + + async function readData(storageLayer: StorageLayer, objectId: string) { + const { data } = await storageLayer.getObject({ + bucketId: "bucket", + decodedObjectId: objectId, + }); + return data.toString(); + } + + it("should not overwrite another object on upload", async () => { + const storageLayer = getStorageLayer(ALWAYS_TRUE_RULES_VALIDATOR); + await uploadDecoded(storageLayer, "bucket", "users/B/photo.png", "from B"); + + await uploadDecoded(storageLayer, "bucket", "users/A/../B/photo.png", "from A"); + + expect(await readData(storageLayer, "users/B/photo.png")).to.equal("from B"); + expect(await readData(storageLayer, "users/A/../B/photo.png")).to.equal("from A"); + }); + + it("should not find another object on read", async () => { + const storageLayer = getStorageLayer(ALWAYS_TRUE_RULES_VALIDATOR); + await uploadDecoded(storageLayer, "bucket", "users/B/photo.png", "from B"); + + await expect( + storageLayer.getObject({ bucketId: "bucket", decodedObjectId: "users/A/../B/photo.png" }), + ).to.be.rejectedWith(NotFoundError); + await expect( + storageLayer.getObject({ bucketId: "bucket", decodedObjectId: "users/B/./photo.png" }), + ).to.be.rejectedWith(NotFoundError); + }); + + it("should not delete another object", async () => { + const storageLayer = getStorageLayer(ALWAYS_TRUE_RULES_VALIDATOR); + await uploadDecoded(storageLayer, "bucket", "users/B/photo.png", "from B"); + + await expect( + storageLayer.deleteObject({ + bucketId: "bucket", + decodedObjectId: "users/A/../B/photo.png", + }), + ).to.be.rejectedWith(NotFoundError); + expect(await readData(storageLayer, "users/B/photo.png")).to.equal("from B"); + }); + + it("should not copy over another object", async () => { + const storageLayer = getStorageLayer(ALWAYS_TRUE_RULES_VALIDATOR); + await uploadDecoded(storageLayer, "bucket", "users/B/photo.png", "from B"); + await uploadDecoded(storageLayer, "bucket", "users/A/source.png", "from A"); + + storageLayer.copyObject({ + sourceBucket: "bucket", + sourceObject: "users/A/source.png", + destinationBucket: "bucket", + destinationObject: "users/A/../B/photo.png", + }); + + expect(await readData(storageLayer, "users/B/photo.png")).to.equal("from B"); + expect(await readData(storageLayer, "users/A/../B/photo.png")).to.equal("from A"); + }); + + it("should keep the objects apart through export and import", async () => { + const storageLayer = getStorageLayer(ALWAYS_TRUE_RULES_VALIDATOR); + await uploadDecoded(storageLayer, "bucket", "users/B/photo.png", "from B"); + await uploadDecoded(storageLayer, "bucket", "users/A/../B/photo.png", "from A"); + const exportDir = mkdtempSync(path.join(tmpdir(), "storage-export-")); + await storageLayer.export(exportDir, { initiatedBy: "test" }); + + const imported = new StorageLayer( + "project", + new Map(), + new Map(), + ALWAYS_TRUE_RULES_VALIDATOR, + ALWAYS_TRUE_ADMIN_CREDENTIAL_VALIDATOR, + new Persistence(mkdtempSync(path.join(tmpdir(), "storage-import-"))), + new StorageCloudFunctions("project"), + ); + imported.import(exportDir, { initiatedBy: "test" }); + + expect(await readData(imported, "users/B/photo.png")).to.equal("from B"); + expect(await readData(imported, "users/A/../B/photo.png")).to.equal("from A"); + }); + }); + + describe("#import()", () => { + it("should import object names exported on Windows with backslash separators", async () => { + const storageLayer = getStorageLayer(ALWAYS_TRUE_RULES_VALIDATOR); + + storageLayer.import( + path.join( + __dirname, + "../../../scripts/storage-emulator-integration/import/windows-emulator-data/storage_export", + ), + { initiatedBy: "test" }, + ); + + const { metadata } = await storageLayer.getObject({ + bucketId: "fake-project-id.appspot.com", + decodedObjectId: "test-directory/test_nested_upload.jpg", + }); + expect(metadata.bucket).to.equal("fake-project-id.appspot.com"); + await storageLayer.getObject({ + bucketId: "fake-project-id.appspot.com", + decodedObjectId: "test_upload.jpg", + }); + }); + }); + const getStorageLayer = (rulesValidator: FirebaseRulesValidator) => new StorageLayer( "project", diff --git a/src/emulator/storage/files.ts b/src/emulator/storage/files.ts index d66084cccdb..8d9447eb3ff 100644 --- a/src/emulator/storage/files.ts +++ b/src/emulator/storage/files.ts @@ -522,8 +522,14 @@ export class StorageLayer { return metadata; } + /** + * Returns the key an object is stored under. Object names are literal in Cloud Storage, so the + * key must keep the name as is: `path.join` would collapse `.` and `..` segments (and, on + * Windows, treat `\` as a separator), letting two different names share one stored object. + * Bucket names cannot contain `/`, so the key is unique per bucket and object. + */ private path(bucket: string, object: string): string { - return path.join(bucket, object); + return `${bucket}/${object}`; } public get dirPath(): string { @@ -628,11 +634,10 @@ export class StorageLayer { continue; } + // Exports written on Windows by older versions stored object names with `\` separators. let fileName = metadata.name; - const objectNameSep = getPathSep(fileName); - // Replace all file separators with that of current platform for compatibility - if (fileName !== path.sep) { - fileName = fileName.split(objectNameSep).join(path.sep); + if (getPathSep(fileName) === "\\") { + fileName = fileName.split("\\").join("/"); } const filepath = this.path(metadata.bucket, fileName); From 78daee2241538556339fd2a8d8495cd147b21397 Mon Sep 17 00:00:00 2001 From: SahinurDEV Date: Mon, 19 Oct 2026 11:11:54 +0600 Subject: [PATCH 2/2] Detect old Windows export separators without the getPathSep regex and clean up spec temp dirs getPathSep's character class [\/|\\] also matched a literal |, so a name like a|b\c.txt from an old Windows export was seen as using | and its backslashes were not mapped to /. Check the first / or \ directly instead, and add a test for it. Also remove the temporary export/import directories the new specs create. --- src/emulator/storage/files.spec.ts | 72 +++++++++++++++++++++++------- src/emulator/storage/files.ts | 10 +---- 2 files changed, 58 insertions(+), 24 deletions(-) diff --git a/src/emulator/storage/files.spec.ts b/src/emulator/storage/files.spec.ts index f227e5e3dfd..537361b92c2 100644 --- a/src/emulator/storage/files.spec.ts +++ b/src/emulator/storage/files.spec.ts @@ -1,5 +1,5 @@ import { expect } from "chai"; -import { mkdtempSync } from "fs"; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "fs"; import { tmpdir } from "os"; import * as path from "path"; @@ -255,21 +255,27 @@ describe("files", () => { await uploadDecoded(storageLayer, "bucket", "users/B/photo.png", "from B"); await uploadDecoded(storageLayer, "bucket", "users/A/../B/photo.png", "from A"); const exportDir = mkdtempSync(path.join(tmpdir(), "storage-export-")); - await storageLayer.export(exportDir, { initiatedBy: "test" }); - - const imported = new StorageLayer( - "project", - new Map(), - new Map(), - ALWAYS_TRUE_RULES_VALIDATOR, - ALWAYS_TRUE_ADMIN_CREDENTIAL_VALIDATOR, - new Persistence(mkdtempSync(path.join(tmpdir(), "storage-import-"))), - new StorageCloudFunctions("project"), - ); - imported.import(exportDir, { initiatedBy: "test" }); - - expect(await readData(imported, "users/B/photo.png")).to.equal("from B"); - expect(await readData(imported, "users/A/../B/photo.png")).to.equal("from A"); + const importDir = mkdtempSync(path.join(tmpdir(), "storage-import-")); + try { + await storageLayer.export(exportDir, { initiatedBy: "test" }); + + const imported = new StorageLayer( + "project", + new Map(), + new Map(), + ALWAYS_TRUE_RULES_VALIDATOR, + ALWAYS_TRUE_ADMIN_CREDENTIAL_VALIDATOR, + new Persistence(importDir), + new StorageCloudFunctions("project"), + ); + imported.import(exportDir, { initiatedBy: "test" }); + + expect(await readData(imported, "users/B/photo.png")).to.equal("from B"); + expect(await readData(imported, "users/A/../B/photo.png")).to.equal("from A"); + } finally { + rmSync(exportDir, { recursive: true, force: true }); + rmSync(importDir, { recursive: true, force: true }); + } }); }); @@ -295,6 +301,40 @@ describe("files", () => { decodedObjectId: "test_upload.jpg", }); }); + + it("should not treat | in a Windows-exported object name as a separator", async () => { + const storageLayer = getStorageLayer(ALWAYS_TRUE_RULES_VALIDATOR); + const exportDir = mkdtempSync(path.join(tmpdir(), "storage-export-")); + try { + mkdirSync(path.join(exportDir, "metadata")); + mkdirSync(path.join(exportDir, "blobs")); + writeFileSync( + path.join(exportDir, "buckets.json"), + JSON.stringify({ buckets: [{ id: "bucket" }] }), + ); + writeFileSync( + path.join(exportDir, "metadata", "object.json"), + StoredFileMetadata.toJSON( + new StoredFileMetadata( + { name: "a|b\\c.txt", bucket: "bucket", contentType: "text/plain" }, + new StorageCloudFunctions("project"), + Buffer.from("data"), + ), + ), + ); + writeFileSync(path.join(exportDir, "blobs", "object"), "data"); + + storageLayer.import(exportDir, { initiatedBy: "test" }); + + const { data } = await storageLayer.getObject({ + bucketId: "bucket", + decodedObjectId: "a|b/c.txt", + }); + expect(data.toString()).to.equal("data"); + } finally { + rmSync(exportDir, { recursive: true, force: true }); + } + }); }); const getStorageLayer = (rulesValidator: FirebaseRulesValidator) => diff --git a/src/emulator/storage/files.ts b/src/emulator/storage/files.ts index 8d9447eb3ff..f3db176bf87 100644 --- a/src/emulator/storage/files.ts +++ b/src/emulator/storage/files.ts @@ -635,8 +635,9 @@ export class StorageLayer { } // Exports written on Windows by older versions stored object names with `\` separators. + // Such a name is recognized by its first separator being `\`. let fileName = metadata.name; - if (getPathSep(fileName) === "\\") { + if (/[/\\]/.exec(fileName)?.[0] === "\\") { fileName = fileName.split("\\").join("/"); } @@ -659,10 +660,3 @@ export class StorageLayer { } } } - -/** Returns file separator used in given path, either '\\' or '/'. */ -function getPathSep(decodedPath: string): string { - // Checks for the first matching file separator - const firstSepIndex = decodedPath.search(/[\/|\\\\]/g); - return decodedPath[firstSepIndex]; -}