mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-07 13:21:25 +00:00
fix(security): harden root file guards and host writes
This commit is contained in:
@@ -2,7 +2,12 @@ import fs from "node:fs/promises";
|
||||
import path from "node:path";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import { createTrackedTempDirs } from "../test-utils/tracked-temp-dirs.js";
|
||||
import { SafeOpenError, openFileWithinRoot, readLocalFileSafely } from "./fs-safe.js";
|
||||
import {
|
||||
SafeOpenError,
|
||||
openFileWithinRoot,
|
||||
readLocalFileSafely,
|
||||
writeFileWithinRoot,
|
||||
} from "./fs-safe.js";
|
||||
|
||||
const tempDirs = createTrackedTempDirs();
|
||||
|
||||
@@ -81,6 +86,83 @@ describe("fs-safe", () => {
|
||||
).rejects.toMatchObject({ code: "invalid-path" });
|
||||
});
|
||||
|
||||
it.runIf(process.platform !== "win32")("blocks hardlink aliases under root", async () => {
|
||||
const root = await tempDirs.make("openclaw-fs-safe-root-");
|
||||
const outside = await tempDirs.make("openclaw-fs-safe-outside-");
|
||||
const outsideFile = path.join(outside, "outside.txt");
|
||||
const hardlinkPath = path.join(root, "link.txt");
|
||||
await fs.writeFile(outsideFile, "outside");
|
||||
try {
|
||||
try {
|
||||
await fs.link(outsideFile, hardlinkPath);
|
||||
} catch (err) {
|
||||
if ((err as NodeJS.ErrnoException).code === "EXDEV") {
|
||||
return;
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
await expect(
|
||||
openFileWithinRoot({
|
||||
rootDir: root,
|
||||
relativePath: "link.txt",
|
||||
}),
|
||||
).rejects.toMatchObject({ code: "invalid-path" });
|
||||
} finally {
|
||||
await fs.rm(hardlinkPath, { force: true });
|
||||
await fs.rm(outsideFile, { force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("writes a file within root safely", async () => {
|
||||
const root = await tempDirs.make("openclaw-fs-safe-root-");
|
||||
await writeFileWithinRoot({
|
||||
rootDir: root,
|
||||
relativePath: "nested/out.txt",
|
||||
data: "hello",
|
||||
});
|
||||
await expect(fs.readFile(path.join(root, "nested", "out.txt"), "utf8")).resolves.toBe("hello");
|
||||
});
|
||||
|
||||
it("rejects write traversal outside root", async () => {
|
||||
const root = await tempDirs.make("openclaw-fs-safe-root-");
|
||||
await expect(
|
||||
writeFileWithinRoot({
|
||||
rootDir: root,
|
||||
relativePath: "../escape.txt",
|
||||
data: "x",
|
||||
}),
|
||||
).rejects.toMatchObject({ code: "invalid-path" });
|
||||
});
|
||||
|
||||
it.runIf(process.platform !== "win32")("rejects writing through hardlink aliases", async () => {
|
||||
const root = await tempDirs.make("openclaw-fs-safe-root-");
|
||||
const outside = await tempDirs.make("openclaw-fs-safe-outside-");
|
||||
const outsideFile = path.join(outside, "outside.txt");
|
||||
const hardlinkPath = path.join(root, "alias.txt");
|
||||
await fs.writeFile(outsideFile, "outside");
|
||||
try {
|
||||
try {
|
||||
await fs.link(outsideFile, hardlinkPath);
|
||||
} catch (err) {
|
||||
if ((err as NodeJS.ErrnoException).code === "EXDEV") {
|
||||
return;
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
await expect(
|
||||
writeFileWithinRoot({
|
||||
rootDir: root,
|
||||
relativePath: "alias.txt",
|
||||
data: "pwned",
|
||||
}),
|
||||
).rejects.toMatchObject({ code: "invalid-path" });
|
||||
await expect(fs.readFile(outsideFile, "utf8")).resolves.toBe("outside");
|
||||
} finally {
|
||||
await fs.rm(hardlinkPath, { force: true });
|
||||
await fs.rm(outsideFile, { force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("returns not-found for missing files", async () => {
|
||||
const dir = await tempDirs.make("openclaw-fs-safe-");
|
||||
const missing = path.join(dir, "missing.txt");
|
||||
|
||||
@@ -4,6 +4,7 @@ import type { FileHandle } from "node:fs/promises";
|
||||
import fs from "node:fs/promises";
|
||||
import path from "node:path";
|
||||
import { sameFileIdentity } from "./file-identity.js";
|
||||
import { assertNoPathAliasEscape } from "./path-alias-guards.js";
|
||||
import { isNotFoundPathError, isPathInside, isSymlinkOpenError } from "./path-guards.js";
|
||||
|
||||
export type SafeOpenErrorCode =
|
||||
@@ -38,10 +39,20 @@ export type SafeLocalReadResult = {
|
||||
|
||||
const SUPPORTS_NOFOLLOW = process.platform !== "win32" && "O_NOFOLLOW" in fsConstants;
|
||||
const OPEN_READ_FLAGS = fsConstants.O_RDONLY | (SUPPORTS_NOFOLLOW ? fsConstants.O_NOFOLLOW : 0);
|
||||
const OPEN_WRITE_FLAGS =
|
||||
fsConstants.O_WRONLY |
|
||||
fsConstants.O_CREAT |
|
||||
fsConstants.O_TRUNC |
|
||||
(SUPPORTS_NOFOLLOW ? fsConstants.O_NOFOLLOW : 0);
|
||||
|
||||
const ensureTrailingSep = (value: string) => (value.endsWith(path.sep) ? value : value + path.sep);
|
||||
|
||||
async function openVerifiedLocalFile(filePath: string): Promise<SafeOpenResult> {
|
||||
async function openVerifiedLocalFile(
|
||||
filePath: string,
|
||||
options?: {
|
||||
rejectHardlinks?: boolean;
|
||||
},
|
||||
): Promise<SafeOpenResult> {
|
||||
let handle: FileHandle;
|
||||
try {
|
||||
handle = await fs.open(filePath, OPEN_READ_FLAGS);
|
||||
@@ -63,12 +74,18 @@ async function openVerifiedLocalFile(filePath: string): Promise<SafeOpenResult>
|
||||
if (!stat.isFile()) {
|
||||
throw new SafeOpenError("not-file", "not a file");
|
||||
}
|
||||
if (options?.rejectHardlinks && stat.nlink > 1) {
|
||||
throw new SafeOpenError("invalid-path", "hardlinked path not allowed");
|
||||
}
|
||||
if (!sameFileIdentity(stat, lstat)) {
|
||||
throw new SafeOpenError("path-mismatch", "path changed during read");
|
||||
}
|
||||
|
||||
const realPath = await fs.realpath(filePath);
|
||||
const realStat = await fs.stat(realPath);
|
||||
if (options?.rejectHardlinks && realStat.nlink > 1) {
|
||||
throw new SafeOpenError("invalid-path", "hardlinked path not allowed");
|
||||
}
|
||||
if (!sameFileIdentity(stat, realStat)) {
|
||||
throw new SafeOpenError("path-mismatch", "path mismatch");
|
||||
}
|
||||
@@ -89,6 +106,7 @@ async function openVerifiedLocalFile(filePath: string): Promise<SafeOpenResult>
|
||||
export async function openFileWithinRoot(params: {
|
||||
rootDir: string;
|
||||
relativePath: string;
|
||||
rejectHardlinks?: boolean;
|
||||
}): Promise<SafeOpenResult> {
|
||||
let rootReal: string;
|
||||
try {
|
||||
@@ -120,6 +138,11 @@ export async function openFileWithinRoot(params: {
|
||||
throw err;
|
||||
}
|
||||
|
||||
if (params.rejectHardlinks !== false && opened.stat.nlink > 1) {
|
||||
await opened.handle.close().catch(() => {});
|
||||
throw new SafeOpenError("invalid-path", "hardlinked path not allowed");
|
||||
}
|
||||
|
||||
if (!isPathInside(rootWithSep, opened.realPath)) {
|
||||
await opened.handle.close().catch(() => {});
|
||||
throw new SafeOpenError("invalid-path", "path escapes root");
|
||||
@@ -146,3 +169,100 @@ export async function readLocalFileSafely(params: {
|
||||
await opened.handle.close().catch(() => {});
|
||||
}
|
||||
}
|
||||
|
||||
export async function writeFileWithinRoot(params: {
|
||||
rootDir: string;
|
||||
relativePath: string;
|
||||
data: string | Buffer;
|
||||
encoding?: BufferEncoding;
|
||||
mkdir?: boolean;
|
||||
}): Promise<void> {
|
||||
let rootReal: string;
|
||||
try {
|
||||
rootReal = await fs.realpath(params.rootDir);
|
||||
} catch (err) {
|
||||
if (isNotFoundPathError(err)) {
|
||||
throw new SafeOpenError("not-found", "root dir not found");
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
const rootWithSep = ensureTrailingSep(rootReal);
|
||||
const resolved = path.resolve(rootWithSep, params.relativePath);
|
||||
if (!isPathInside(rootWithSep, resolved)) {
|
||||
throw new SafeOpenError("invalid-path", "path escapes root");
|
||||
}
|
||||
try {
|
||||
await assertNoPathAliasEscape({
|
||||
absolutePath: resolved,
|
||||
rootPath: rootReal,
|
||||
boundaryLabel: "root",
|
||||
});
|
||||
} catch (err) {
|
||||
throw new SafeOpenError("invalid-path", "path alias escape blocked", { cause: err });
|
||||
}
|
||||
if (params.mkdir !== false) {
|
||||
await fs.mkdir(path.dirname(resolved), { recursive: true });
|
||||
}
|
||||
|
||||
let ioPath = resolved;
|
||||
try {
|
||||
const resolvedRealPath = await fs.realpath(resolved);
|
||||
if (!isPathInside(rootWithSep, resolvedRealPath)) {
|
||||
throw new SafeOpenError("invalid-path", "path escapes root");
|
||||
}
|
||||
ioPath = resolvedRealPath;
|
||||
} catch (err) {
|
||||
if (err instanceof SafeOpenError) {
|
||||
throw err;
|
||||
}
|
||||
if (!isNotFoundPathError(err)) {
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
|
||||
let handle: FileHandle;
|
||||
try {
|
||||
handle = await fs.open(ioPath, OPEN_WRITE_FLAGS, 0o600);
|
||||
} catch (err) {
|
||||
if (isNotFoundPathError(err)) {
|
||||
throw new SafeOpenError("not-found", "file not found");
|
||||
}
|
||||
if (isSymlinkOpenError(err)) {
|
||||
throw new SafeOpenError("invalid-path", "symlink open blocked", { cause: err });
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
|
||||
try {
|
||||
const [stat, lstat] = await Promise.all([handle.stat(), fs.lstat(ioPath)]);
|
||||
if (lstat.isSymbolicLink() || !stat.isFile()) {
|
||||
throw new SafeOpenError("invalid-path", "path is not a regular file under root");
|
||||
}
|
||||
if (stat.nlink > 1) {
|
||||
throw new SafeOpenError("invalid-path", "hardlinked path not allowed");
|
||||
}
|
||||
if (!sameFileIdentity(stat, lstat)) {
|
||||
throw new SafeOpenError("path-mismatch", "path changed during write");
|
||||
}
|
||||
|
||||
const realPath = await fs.realpath(ioPath);
|
||||
const realStat = await fs.stat(realPath);
|
||||
if (!sameFileIdentity(stat, realStat)) {
|
||||
throw new SafeOpenError("path-mismatch", "path mismatch");
|
||||
}
|
||||
if (realStat.nlink > 1) {
|
||||
throw new SafeOpenError("invalid-path", "hardlinked path not allowed");
|
||||
}
|
||||
if (!isPathInside(rootWithSep, realPath)) {
|
||||
throw new SafeOpenError("invalid-path", "path escapes root");
|
||||
}
|
||||
|
||||
if (typeof params.data === "string") {
|
||||
await handle.writeFile(params.data, params.encoding ?? "utf8");
|
||||
} else {
|
||||
await handle.writeFile(params.data);
|
||||
}
|
||||
} finally {
|
||||
await handle.close().catch(() => {});
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user