Skip to content

[Security][High] Annotation/conversation id used unvalidated in filesystem paths — arbitrary file read (GET), arbitrary file delete (DELETE) #3

Description

@docxology

Title: [Security][High] Annotation/conversation id used unvalidated in filesystem paths — arbitrary file read (GET), arbitrary file delete (DELETE)

Severity: High (CWE-22, CWE-73)
Locations: packages/core/src/annotations/store.ts:13-15, 175-180, 200-204; packages/core/src/annotations/conversation.ts:16-24; packages/server/src/routes/annotations.ts:174-176, 245-247, 396-400

Summary

annotationPath() and conversationPath() build file paths by string-interpolating the raw :id request parameter. IDs are created server-side as UUIDs but that is never enforced on read/delete. Express decodes %2f inside path segments, so ../ sequences reach the path builder.

Evidence

packages/core/src/annotations/store.ts:13-15:

function annotationPath(projectRoot: string, id: string): string {
  return join(annotationsDir(projectRoot), `${id}.json`)
}

store.ts:200-204 (delete): unlinkSync(filePath) unconditionally when the crafted path exists — no extension restriction.
routes/annotations.ts:174: loadAnnotation(root, req.params.id) — raw param, no validation.
Same concatenation in conversation.ts:16-18 (${annotationId}.conversation.json), reached via GET /api/annotations/:id/conversation.

Impact

  • GET /api/annotations/..%2f..%2f..%2f.config%2fgraphcoder%2ftemporal%2fcache → reads any *.json file under the project's .graphcoder (temporal cache, kind registry, conversation logs).
  • DELETE /api/annotations/..%2f..%2f..%2f..%2f..%2fimportant → unlinks an arbitrary file the process can write; integrity/availability break on the developer's machine.
  • Latent write variant: saveAnnotation uses annotation.id from file contents (normalizeAnnotation copies raw.id verbatim), and the kind-rename cascade re-saves every matching annotation — an on-disk file with id: '../x' round-trips into a write outside the directory.

Repro sketch

curl --path-as-is -X DELETE http://localhost:3357/api/annotations/..%2f..%2f..%2f..%2f..%2fUsers%2Fvictim%2Fimportant-file

Recommended fix

Validate ids against a UUID pattern (or path.basename(id) === id) in loadAnnotation / deleteAnnotation / loadConversation / deleteConversation / saveAnnotation + normalizeAnnotation before any fs call. Applies to both call sites (store.ts and conversation.ts).

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

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions