Description

handleImagePayload() now permits inline data:image/*;base64,... URLs when payload.source !== "file".

• Input: payload.path comes from parsed structured payload JSON (QQBOT_PAYLOAD:) without any size limit enforcement.
• Validation: isInlineImageDataUrl() only checks a prefix regex and does not enforce a maximum length, allowed subtypes, or base64 validity.
• Impact: An attacker who can influence model output (or any upstream producer of structured payloads) can cause the process to handle/log extremely large strings and attempt to send them to the QQ Bot API, potentially leading to excessive memory usage, request size failures, and service degradation/DoS.

Vulnerable code:

function isInlineImageDataUrl(p: string): boolean {
  return /^data:image\/[^;]+;base64,/i.test(p);
}
...
} else if (isRemoteHttpUrl(normalizedPath) || isInlineImageDataUrl(normalizedPath)) {
  imageUrl = normalizedPath;
}

Recommendation

Add strict validation and size limits for inline data URLs.

Minimum recommended changes:

• Enforce a maximum payload.path length for data: URLs (e.g., <= 1–2MB, or aligned with QQ Bot API limits).
• Restrict allowed image MIME subtypes to a safe allowlist (e.g., image/png, image/jpeg, image/gif, image/webp) and explicitly reject image/svg+xml.
• Optionally validate that the base64 payload decodes successfully and does not exceed a decoded byte limit.

Example:

const MAX_INLINE_IMAGE_BYTES = 1 * 1024 * 1024; // 1MB decoded
const MAX_INLINE_IMAGE_CHARS = Math.ceil(MAX_INLINE_IMAGE_BYTES * 4 / 3) + 128;
const ALLOWED_INLINE_MIMES = new Set(["image/png","image/jpeg","image/gif","image/webp"]);

function parseInlineImageDataUrl(p: string): { mime: string; b64: string } | null {
  const m = /^data:(image\/[a-z0-9.+-]+);base64,([a-z0-9+/=]+)$/i.exec(p);
  if (!m) return null;
  const mime = m[1].toLowerCase();
  if (!ALLOWED_INLINE_MIMES.has(mime)) return null;
  if (p.length > MAX_INLINE_IMAGE_CHARS) return null;
  const buf = Buffer.from(m[2], "base64");
  if (buf.length > MAX_INLINE_IMAGE_BYTES) return null;
  return { mime, b64: m[2] };
}

Then only allow data: URLs that pass parseInlineImageDataUrl().

Description

resolveQQBotPayloadLocalFilePath() restricts structured-payload local file reads to the QQ Bot media directory by checking realpath + isPathWithinRoot. However, the subsequent read path (readStructuredPayloadLocalFile) only blocks symlinks via O_NOFOLLOW and checks stat.isFile().

This still allows hardlink attacks:

• A file inside the allowed media directory can be a hardlink to an arbitrary sensitive file outside the directory (e.g., /etc/passwd), and realpath will still point to the hardlink path inside the allowed root.
• O_NOFOLLOW does not prevent hardlinks (it only prevents symlink traversal).
• Result: if an attacker can create/plant hardlinks within the QQ Bot media directory (or otherwise influence files created there), they can trick the bot into reading and sending back contents of sensitive files.

Vulnerable flow:

• Input: payload.path (structured payload) → resolveQQBotPayloadLocalFilePath() allowlist check
• Sink: fs.promises.open(filePath, O_RDONLY|O_NOFOLLOW) + handle.readFile() reads and exfiltrates bytes

Vulnerable code:

​// reply-dispatcher.ts
const openFlags = fs.constants.O_RDONLY | ("O_NOFOLLOW" in fs.constants ? fs.constants.O_NOFOLLOW : 0);
const handle = await fs.promises.open(filePath, openFlags);
const stat = await handle.stat();
if (!stat.isFile()) throw new Error("Path is not a regular file");
return handle.readFile();

Even though platform.ts uses realpathSync(), hardlinks remain within-root paths:

​// platform.ts
const canonicalCandidate = fs.realpathSync(resolvedCandidate);
if (isPathWithinRoot(canonicalCandidate, canonicalRoot)) return canonicalCandidate;

Recommendation

Mitigate hardlink-based exfiltration by adding an explicit hardlink defense and by tightening how files enter the media directory.

Options (combine where possible):

1. Disallow hardlinked files (best-effort on Unix): reject files with stat.nlink > 1.

const stat = await handle.stat();
if (!stat.isFile()) throw new Error("Path is not a regular file");
​// Best-effort hardlink mitigation on platforms that expose nlink.
if (typeof (stat as any).nlink === "number" && (stat as any).nlink > 1) {
  throw new Error("Hardlinked files are not allowed");
}

2. Ensure the bot is the only writer to the QQ Bot media directory:

• create media files yourself with restrictive permissions (e.g., 0o600)
• avoid processing user-controlled arbitrary local paths; prefer IDs referencing internally-created files

3. Where feasible, open and verify using file descriptors and avoid any pre-open checks that can be raced. If additional root checks are needed, perform them based on the opened file (e.g., verify the path via fs.realpath on the opened FD where supported, or store a DB mapping from IDs to internally created files).

Description

resolveQQBotPayloadLocalFilePath() attempts to restrict structured-payload local file reads to the QQ Bot media directory, but its check is vulnerable to TOCTOU and symlink/junction traversal:

• The path is canonicalized with realpathSync() only if it exists at validation time; otherwise the non-canonical path.resolve() result is used.
• Even when the candidate exists, the later read/open is a separate operation; an attacker who can modify the filesystem under ~/.openclaw/media/qqbot can swap directories or replace intermediate components with symlinks after validation.
• readStructuredPayloadLocalFile() opens the path by name; O_NOFOLLOW (when available) only prevents the final path component being a symlink and does not protect intermediate directories. On Windows, O_NOFOLLOW is typically unavailable, and junction/symlink traversal can lead outside the intended root.

Impact: a crafted structured payload referencing a path under the QQ Bot media directory can be raced/redirected to read arbitrary files on the host and exfiltrate them by sending as an image/video/file payload.

Vulnerable code (path validation):

const resolvedCandidate = path.resolve(candidate);
const canonicalCandidate = fs.existsSync(resolvedCandidate)
  ? fs.realpathSync(resolvedCandidate)
  : resolvedCandidate;
...
if (isPathWithinRoot(canonicalCandidate, canonicalRoot)) {
  return canonicalCandidate;
}

Recommendation

Make the authorization check and the open operation refer to the same object, and prevent symlink traversal for all path components.

Recommended approach:

1. Require the file to exist at the time of use and resolve it via realpath (or equivalent) immediately before opening.
2. Open relative to a trusted directory file descriptor/handle (best on POSIX), preventing symlink traversal in any component.

Example (POSIX-oriented) using a root FD and openat-style behavior (Node doesn’t expose openat directly, but you can emulate by resolving and verifying realpath at open-time and rejecting any path containing symlinks):

export function resolveQQBotPayloadLocalFilePath(p: string): string | null {
  const candidate = resolveQQBotLocalMediaPath(normalizePath(p));
  if (!candidate.trim()) return null;

  ​// Must exist; always canonicalize.
  let canonicalCandidate: string;
  try {
    canonicalCandidate = fs.realpathSync(candidate);
  } catch {
    return null;
  }

  const canonicalRoot = fs.realpathSync(getQQBotMediaDir());
  if (!isPathWithinRoot(canonicalCandidate, canonicalRoot)) return null;

  return canonicalCandidate;
}

And in readStructuredPayloadLocalFile(), consider additionally rejecting any path that is not equal to its own realpathSync() at open-time, and on Windows explicitly detect/reject reparse points (junctions/symlinks) within the media tree.

If supporting Windows securely, consider copying the target file into a fresh, newly-created temp file within the media dir and reading that copy (after verifying it is a regular file), rather than reading arbitrary existing paths.

Description

Several log statements include user-controlled URL/path values with insufficient redaction.

• For remote media, describeMediaTargetForLog() logs protocol//host + pathname (only removes userinfo/query/hash). Many signed URLs and internal endpoints embed tokens/identifiers in the pathname (not the query), so secrets and internal routing can still be exposed in logs.
• When a local path is rejected, validateStructuredPayloadLocalPath() logs the (sanitized but otherwise full) payloadPath, which can disclose local filesystem layout or attacker-supplied sensitive strings.

Vulnerable code:

ctx.log?.error(
  `... Blocked ... local path ...: ${sanitizeForLog(payloadPath)}`,
);
...
return sanitizeForLog(`${url.protocol}//${url.host}${url.pathname}`);

Impact depends on who can access logs; in multi-tenant deployments or shared log aggregation this can leak internal hosts, path structure, or secrets embedded in URL path segments.

Recommendation

Reduce logged detail for user-controlled URLs/paths.

• For URLs, log only the origin and a coarse indicator (e.g., basename or a fixed placeholder), or log a one-way hash of the full URL for correlation.
• For local paths, avoid logging the path; log only the basename or a generic reason.

Example safer implementation:

import crypto from "node:crypto";

function describeMediaTargetForLog(pathValue: string, isHttpUrl: boolean): string {
  if (!isHttpUrl) return "<local-file>";
  try {
    const url = new URL(pathValue);
    url.username = "";
    url.password = "";
    ​// keep only origin; do not log pathname
    const id = crypto.createHash("sha256").update(url.toString()).digest("hex").slice(0, 12);
    return `${url.protocol}//${url.host}#${id}`;
  } catch {
    return "<invalid-url>";
  }
}

​// When blocking:
ctx.log?.error(`[qqbot:${ctx.account.accountId}] Blocked ${mediaType} payload local path outside QQ Bot media storage`);

Also consider gating such details behind a debug log level and ensuring log access controls match tenant boundaries.

Description

Structured media payloads allow arbitrary http(s) URLs when payload.source !== "file", and the URL is forwarded to QQ's media upload endpoints (/v2/users/{openid}/files, /v2/groups/{groupOpenid}/files) as body.url.

• Input: payload.path in handleImagePayload/handleVideoPayload/handleFilePayload
• Validation: only startsWith("http://") || startsWith("https://")
• Sink: uploadC2CMedia / uploadGroupMedia send { url: <user_url> } to QQ API, which will typically cause the upstream service to fetch the URL to ingest the media.

This enables SSRF-like behavior by proxy (the QQ platform performs the fetch) and can be abused to target:

• private/loopback IPs (e.g., http://127.0.0.1/, http://169.254.169.254/)
• internal hostnames, non-standard ports, or URLs with embedded credentials
• redirects to disallowed targets (if QQ follows redirects)

Vulnerable code (accepts URL with only prefix check and forwards it):

} else if (isRemoteHttpUrl(normalizedPath)) {
  imageUrl = normalizedPath;
}
...
uploadResult = await uploadC2CMedia(accessToken, openid, MediaFileType.IMAGE, imageUrl, undefined, false);

Even if the fetch occurs on QQ infrastructure (not the bot host), it is still a security risk and can violate network egress / acceptable use policies, and can be used to trigger network requests to attacker-chosen targets.

Recommendation

Treat remote-URL media uploads as a privileged feature.

1. Default-deny remote URLs unless explicitly enabled by configuration.
2. When enabled, parse and validate with new URL() and enforce restrictions:
   • protocol must be exactly http: or https:
   • disallow usernames/passwords in URL
   • disallow non-standard ports (or allowlist ports)
   • resolve DNS and block private/loopback/link-local/reserved IP ranges
   • optionally enforce an allowlist of domains/CDNs
   • consider blocking redirects or re-validating the final destination after redirects

Example (skeletal):

function validateRemoteMediaUrl(raw: string): string | null {
  let u: URL;
  try { u = new URL(raw); } catch { return null; }
  if (u.protocol !== "https:" && u.protocol !== "http:") return null;
  if (u.username || u.password) return null;
  if (u.port && !["80","443"].includes(u.port)) return null;
  ​// Resolve u.hostname and block private/link-local/etc.
  return u.toString();
}

​// usage
const safeUrl = validateRemoteMediaUrl(payload.path);
if (!safeUrl) { /* reject */ }

If remote URL uploads are required, document clearly that the upstream QQ service will fetch the URL and that only safe/public URLs should be used.

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".