Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 21 additions & 5 deletions src/lib/sandbox-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,9 +180,19 @@ export function validateTarEntries(

/**
* Walk a directory and return violations for any symlinks whose
* resolved targets escape rootPath.
* resolved targets don't land within any of the allowed roots.
*
* `allowedRoots` always includes the extraction directory (the local host
* path). Callers pass additional roots — notably `/sandbox` — to permit
* legitimate intra-sandbox symlinks baked into the sandbox base image
* (e.g. `/sandbox/.openclaw` → `/sandbox/.openclaw-data`). Those look
* like "escapes" relative to the extraction temp dir on the host, but
* are intra-sandbox once the backup is restored. See issue #2268.
*/
function auditExtractedSymlinks(dirPath: string, rootPath: string): string[] {
function auditExtractedSymlinks(
dirPath: string,
allowedRoots: string[],
): string[] {
const violations: string[] = [];
if (!existsSync(dirPath)) return violations;

Expand All @@ -194,7 +204,10 @@ function auditExtractedSymlinks(dirPath: string, rootPath: string): string[] {
if (stat.isSymbolicLink()) {
const linkTarget = readlinkSync(fullPath);
const resolvedTarget = path.resolve(path.dirname(fullPath), linkTarget);
if (!isWithinRoot(resolvedTarget, rootPath)) {
const inAnyAllowedRoot = allowedRoots.some((root) =>
isWithinRoot(resolvedTarget, root),
);
if (!inAnyAllowedRoot) {
violations.push(`symlink escape: ${fullPath} -> ${linkTarget} (resolves to ${resolvedTarget})`);
}
} else if (stat.isDirectory()) {
Expand Down Expand Up @@ -282,8 +295,11 @@ export function safeTarExtract(
}

// Phase 3: Post-extraction symlink audit (symlink targets are not
// visible in `tar -tf` output, so we must check after extraction)
const symlinkViolations = auditExtractedSymlinks(targetDir, targetDir);
// visible in `tar -tf` output, so we must check after extraction).
// Allow targets inside either the host extraction dir OR the canonical
// sandbox root (/sandbox) — the latter covers legitimate intra-sandbox
// symlinks baked into the base image (see #2268).
const symlinkViolations = auditExtractedSymlinks(targetDir, [targetDir, "/sandbox"]);
if (symlinkViolations.length > 0) {
// Nuke the extraction — do not leave attacker-controlled symlinks on host
try {
Expand Down
59 changes: 59 additions & 0 deletions test/security-sandbox-tar-traversal.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,65 @@ describe("Fix: safeTarExtract blocks malicious archives and extracts safe ones",
fs.rmSync(workDir, { recursive: true, force: true });
}
});

// Regression for #2268 — the sandbox base image places intra-sandbox
// symlinks like /sandbox/.openclaw → /sandbox/.openclaw-data. When the
// backup tar is extracted on the host, those absolute symlinks point
// OUTSIDE the extraction temp dir, but INSIDE the canonical sandbox
// root — which is where they'll be legitimately resolved on restore.
// Treating them as escape violations breaks every rebuild / snapshot
// create on v0.0.22.
it("allows symlinks whose target resolves within /sandbox (intra-sandbox layout)", async () => {
const { safeTarExtract } = await loadSandboxState();
const workDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-sandbox-link-"));
try {
const targetDir = path.join(workDir, "backup");
fs.mkdirSync(targetDir, { recursive: true });

const tar = buildTar([
{ path: "sandbox/.openclaw-data/", type: "5" },
{
path: "sandbox/.openclaw",
type: "2",
linkTarget: "/sandbox/.openclaw-data",
},
]);

const result = safeTarExtract(tar, targetDir);

expect(result.success).toBe(true);
expect(result.error).toBeUndefined();
} finally {
fs.rmSync(workDir, { recursive: true, force: true });
}
});

// Security guardrail: /sandbox/ is allowed, but a crafted symlink whose
// target *looks* absolute must not escape beyond the sandbox root.
// /sandbox/../etc/passwd resolves to /etc/passwd — still must be blocked.
it("blocks symlinks that escape /sandbox even with an absolute target", async () => {
const { safeTarExtract } = await loadSandboxState();
const workDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-sandbox-escape-"));
try {
const targetDir = path.join(workDir, "backup");
fs.mkdirSync(targetDir, { recursive: true });

const tar = buildTar([
{
path: "evil-abs-link",
type: "2",
linkTarget: "/etc/passwd",
},
]);

const result = safeTarExtract(tar, targetDir);

expect(result.success).toBe(false);
expect(result.error).toContain("symlink");
} finally {
fs.rmSync(workDir, { recursive: true, force: true });
}
});
});

describe("Fix: rejectHardLinks blocks hard-link entries at validation time", () => {
Expand Down
Loading