Skip to content

resolve: an unvalidated offload marker deletes files outside the store #5

Description

@idrmn

Summary

marker_id returns the text between <<swload: and >> with no validation, and
Store::resolve joins that text onto the store directory and deletes the result before
it checks the content hash. A caller of the MCP resolve tool can therefore delete a
file outside the store when the path ends in .blob and the file is older than the TTL.
The marker argument comes from the model, so injected text in any content the model
reads can trigger the deletion.

Environment

secondwind 0.3.2, built from ab3888a with cargo install --git. Linux, Claude Code
2.1.220 with the MCP server registered by secondwind setup.

The code

crates/optimize/src/offload.rs:325

fn marker_id(marker: &str) -> Option<&str> {
    marker.strip_prefix("<<swload:")?.strip_suffix(">>")
}

crates/optimize/src/offload.rs:143

let id = marker_id(marker)?;
let path = dir.join(format!("{id}.blob"));
if self.expired(&path) {
    let _ = std::fs::remove_file(&path);   // line 146
    return None;
}
let body = std::fs::read_to_string(&path).ok()?;
...
if &hash(&canonical)[..16] != id {         // line 155
    return None;
}

expired() returns true for a file older than the TTL, and also when it cannot read the
metadata. The deletion on line 146 runs before the hash check on line 155, so that check
does not guard this path.

The path from outside the process carries no validation either:
crates/cli/src/mcp.rs:165 reads arguments.marker, crates/cli/src/mcp.rs:167 calls
resolve_selected, crates/optimize/src/lib.rs:734 passes the value to the store.

The project already has the check this path needs. embedded_marker_id on
offload.rs:330 accepts an id only when it is 16 ASCII hex characters, but its only
caller is certificate.rs:24.

Reproduction

mkdir -p /tmp/victim
echo canary > /tmp/victim/secret.blob
touch -d '3 days ago' /tmp/victim/secret.blob

printf '{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"resolve","arguments":{"marker":"<<swload:../../../../tmp/victim/secret>>"}}}\n' \
  | secondwind --home "$HOME" mcp

ls /tmp/victim/secret.blob

The tool answers marker not found, expired, or selector matched nothing, and the file
is gone.

A second surface: CallbackStore

CallbackStore::resolve on offload.rs:244 passes the same unvalidated id straight to
the host callback and performs no hash check at all:

fn resolve(&self, marker: &str) -> Option<String> {
    (self.get)(marker_id(marker)?)
}

The MCP server builds Store, not CallbackStore, so this is not the path in the
reproduction above. It matters for a host that supplies its own backend: if that
callback maps an id onto a filesystem or an object key, the missing validation gives
content disclosure on this path, not only deletion.

Impact

Deletion of a file the user never named, triggered by model-controlled input. The limits
are real: only a path ending in .blob, only a file older than the TTL, and only where
the process can write. There is no disclosure through Store, because the read on
line 149 is followed by the hash check that rejects foreign content. CallbackStore has
no such check.

Preconditions for the deletion: a persistent store, a miss in the memory cache (always
true for a crafted marker, since resolve reads mem first on line 134), a path that
escapes the store directory, and a target older than the TTL.

Suggested fix

Validate inside marker_id the way embedded_marker_id already does: accept the id
only when it is 16 ASCII hex characters. As defence in depth, canonicalize the joined
path and confirm it stays inside the store directory before any read or delete, and
delete only after the content hash matches.

Note

I report this in the open because the exposure is limited and self-mitigating: a user
who is worried can remove the secondwind MCP server and lose only the resolve tool.
The repository has no security policy and no private advisory channel. Two reviewers
reproduced the analysis independently before I filed this.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions