SiYuan MCP asset.upload Reads Arbitrary Absolute File Paths (Workspace Boundary Bypass)

Summary

# Security Advisory — SiYuan MCP `asset.upload` Reads Arbitrary Absolute File Paths (Workspace Boundary Bypass) | Field | Value | |---|---| | **Disclosed by** | joysinleung (`[email protected]`) | | **Report date** | 2026-08-13 | | **Product** | SiYuan (思源笔记) — `siyuan-note/siyuan` | | **Go module** | `github.com/siyuan-note/siyuan/kernel` | | **Affected versions** | `<= 3.8.0` (latest release at report time; statically confirmed on v3.8.0) | | **Patched versions** | 3.8.1 | | **Component** | `kernel/mcp/tools/asset.go` (`assetUpload`), `kernel/model/upload.go` (`InsertLocalAssets`) | | **Relationship to prior advisory** | **Residual of CVE-2026-66012** (GHSA-cvhv-7xhj-xjp8) MCP remediation. See §Relationship. | | **EPSS (exploitation probability)** | Low. Requires the AI Agent to invoke `asset.upload` and the user to approve the (category-level) confirmation; reachable via prompt-injection of the agent. | | **KEV (CISA Known Exploited)** | No (not listed in CISA KEV at report time). | | **Default-config reachable** | **Partial** — requires the Agent/MCP surface to be configured (admin) and a user approval click; the boundary check itself is entirely absent, so any approved upload reads outside the workspace. | --- ## Summary SiYuan exposes a native MCP tool `asset` → `asset.upload`. Its `files` argument is documented as a **comma-separated list of absolute file paths**. The handler (`kernel/mcp/tools/asset.go:195`) only normalizes each entry with `filepath.Abs(...)` — it performs **no workspace boundary check** (`IsSubPath`) and **no sensitive-path check** (`IsSensitivePath`). The downstream `model.InsertLocalAssets` (`kernel/model/upload.go:97`) then `os.Open`s each path and copies its bytes into the workspace `assets/` directory. Every other AI-plane file primitive in SiYuan is workspace-constrained: - `file` / `unzip` tools resolve paths via `resolvePath` (workspace-relative, escape-proof). `asset.upload` is the **only** AI tool that accepts arbitrary absolute paths with zero boundary validation. An attacker who can steer the Agent (prompt injection) can induce it to upload sensitive files — e.g. `/Users/victim/.ssh/id_rsa`, `~/.aws/credentials`, `/etc/passwd` — into the workspace, from where they are reachable via notes / export / sync. ## Relationship to Prior Advisories - **CVE-2026-66012** (GHSA-cvhv-7xhj-xjp8) remediated the MCP endpoint by requiring `CheckAdminRole` on `/mcp` and adding `refuseToAccess` for `conf/conf.json` in the *file* tool. However, the **`asset.upload` tool was never given a workspace boundary check** — it still accepts and reads any absolute path. This is a **residual boundary omission** from that remediation: the admin gate restricts *who* may call MCP, but does not constrain *which files* `asset.upload` may read. We report it as the unpatched half of the MCP file-surface hardening. ## Affected Version Statically confirmed on the latest release **v3.8.0** (tag `v3.8.0`, commit `251596fc0`): - `kernel/mcp/tools/asset.go:195` `assetUpload`: `abs, _ := filepath.Abs(strings.TrimSpace(f))` — normalization only. - `kernel/model/upload.go:97` `InsertLocalAssets`: iterates the path list and `os.Open(assetAbsPath)` → `writeAssetFile(writePath, ...)` into `assetsDirPath`; the `IsSubPath(assetsDirPath, assetAbsPath)` check at line 130 is **only a dedup guard**, not a boundary limit. No workspace/external restriction is applied. ## Component - `kernel/mcp/tools/asset.go:195` — `assetUpload` (`files` arg → `filepath.Abs` only). - `kernel/model/upload.go:97` — `InsertLocalAssets` (`os.Open` + copy to `assets/`). - Contrast (safe): `kernel/mcp/tools/unzip.go:52` `unzipHandler` uses `resolvePath` (workspace-relative, escape-proof). ## Attack Vector **AI Agent / prompt injection.** Victim lets the Agent process attacker-controlled web/note content → agent is induced to call `asset.upload` with `/Users/victim/.ssh/id_rsa` (or similar) as a `files` entry. The user sees a **category-level** confirmation ("upload asset") that does **not** display the specific source path, so the approval is effectively blind to the actual file being read. Once approved, the file is copied into the workspace and exfiltrated via notes/export/sync. No admin role is required beyond the existing Agent/MCP configuration. ## Proof of Concept ```jsonc // Agent tool call (asset.upload), attacker-influenced files argument: { "id": "<someBlockID>", "files": "/Users/victim/.ssh/id_rsa,/Users/victim/.aws/credentials,/etc/passwd" } ``` Handler flow: 1. `filepath.Abs("/Users/victim/.ssh/id_rsa")` → `/Users/victim/.ssh/id_rsa` (no `IsSubPath`/`IsSensitivePath` rejection). 2. `InsertLocalAssets` → `os.Open` → bytes copied into `<workspace>/data/assets/...`. 3. The private key is now readable via the workspace file API / export / sync. ## Impact Confidentiality breach: arbitrary local file read (including SSH keys, cloud credentials, OS secrets) through the AI plane. Integrity/availability not directly impacted. Severity is moderated by the required user approval step, but the approval dialog does not reveal the real source path, so the user cannot make an informed decision. ## Scope Reachable only when the Agent/MCP surface is configured and the user approves the upload action. The boundary check is absent regardless of config — any approved upload reads outside the workspace. ## Remediation 1. **Add boundary enforcement in `assetUpload`**: reject any entry where `!util.IsSubPath(util.WorkspaceDir, abs)` or `util.IsSensitivePath(abs)` (same checks used elsewhere). 2. **Show the real source path in the confirmation dialog** so the user can make an informed decision (currently the `LocalWrite` gate is action-category based and hides the specific path). 3. Mirror the `resolvePath` workspace-relative constraint already applied to the `file` / `unzip` tools. > Note: patch authored against v3.8.0 source; not compiled into a full SiYuan release build. Provided for the maintainer to validate in CI. --- ## Appendix: Suggested Patch (F10) ```diff diff --git a/kernel/mcp/tools/asset.go b/kernel/mcp/tools/asset.go index aaa..bbb 100644 --- a/kernel/mcp/tools/asset.go +++ b/kernel/mcp/tools/asset.go @@ -195,8 +195,16 @@ func assetUpload(args map[string]any) (CallToolResult, error) { fileList := strings.Split(filesStr, ",") for i, f := range fileList { abs, err := filepath.Abs(strings.TrimSpace(f)) - if err != nil { + if err != nil { return CallToolResult{}, err } + // 边界校验：仅允许工作区内的资产，拒绝任意绝对路径越界读 + if !util.IsSubPath(util.WorkspaceDir, abs) || util.IsSensitivePath(abs) { + ret.Code = -1 + ret.Msg = fmt.Sprintf("asset path %s is outside the workspace or sensitive", abs) + return CallToolResult{}, fmt.Errorf("asset path outside workspace: %s", abs) + } fileList[i] = abs } succMap, err := model.InsertLocalAssets(id, fileList, true) ```