fix(security): sanitize file paths in agent-state and skills-remove endpoints (#60)
Co-authored-by: Shams <shams@openclaw.dev>
This commit is contained in:
@@ -83,10 +83,14 @@ export const restoreAgentStateLocally = (params: {
|
||||
}
|
||||
|
||||
const base = resolveStateDir();
|
||||
const trashRoot = path.join(base, "trash", "studio-delete-agent");
|
||||
if (!fs.existsSync(trashDirRaw)) {
|
||||
throw new Error(`trashDir does not exist: ${trashDirRaw}`);
|
||||
}
|
||||
const { resolvedCandidate: resolvedTrashDir } = ensureUnderBase(base, trashDirRaw);
|
||||
// Validate trashDir is strictly under the expected trash root, not just anywhere under base.
|
||||
// This prevents path traversal where an attacker could reference legitimate directories
|
||||
// (e.g. base/agents/) as a "trashDir" and cause unintended file moves.
|
||||
const { resolvedCandidate: resolvedTrashDir } = ensureUnderBase(trashRoot, trashDirRaw);
|
||||
|
||||
const moves: GatewayAgentStateMove[] = [];
|
||||
const restoreIfExists = (src: string, dest: string) => {
|
||||
|
||||
@@ -2,7 +2,7 @@ import fs from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
|
||||
import { resolveUserPath } from "@/lib/clawdbot/paths";
|
||||
import { resolveStateDir, resolveUserPath } from "@/lib/clawdbot/paths";
|
||||
import type { RemovableSkillSource, SkillRemoveRequest, SkillRemoveResult } from "@/lib/skills/types";
|
||||
|
||||
const resolveComparablePath = (input: string): string => {
|
||||
@@ -57,6 +57,18 @@ export const removeSkillLocally = (params: SkillRemoveRequest): SkillRemoveResul
|
||||
const workspaceDir = normalizeRequiredPath(params.workspaceDir, "workspaceDir");
|
||||
const managedSkillsDir = normalizeRequiredPath(params.managedSkillsDir, "managedSkillsDir");
|
||||
|
||||
// Security: validate that workspaceDir and managedSkillsDir are under the
|
||||
// server-side state directory. Without this check, an attacker can supply
|
||||
// arbitrary values for these fields, effectively controlling the "allowed root"
|
||||
// and bypassing the isPathInside containment check below.
|
||||
const stateDir = resolveStateDir();
|
||||
if (!isPathInside(stateDir, workspaceDir)) {
|
||||
throw new Error(`workspaceDir is not under the state directory: ${workspaceDir}`);
|
||||
}
|
||||
if (!isPathInside(stateDir, managedSkillsDir)) {
|
||||
throw new Error(`managedSkillsDir is not under the state directory: ${managedSkillsDir}`);
|
||||
}
|
||||
|
||||
const allowedRoot = resolveAllowedRoot({
|
||||
source,
|
||||
workspaceDir,
|
||||
|
||||
Reference in New Issue
Block a user