downloadFile (src/download/index.ts:71) falls back to file.metadata.title as the output
path when no outPath is given, and downloadThumbnail (:84) uses it with a thumb_
prefix. file.metadata.title is decrypted from server-supplied metadata; it is fully
attacker-controlled if the account is compromised or the server is hostile, and quak's entire
premise is that the server is not trusted with plaintext.
A title of ../../.ssh/authorized_keys writes outside the intended directory. bin/quak.ts
passes opts.out straight through, which is undefined unless --out was given, so quak get <id> in a shell hits this path directly.
src/backup.ts has a sanitizePath helper for the backup tree, but the download layer does
not use it and sanitizePath itself is not applied to path separators in a way that has been
tested against traversal payloads.
src/model/decrypt.ts:100 compounds this: the decrypted metadata JSON is parsed as implicit any and every field is read unchecked, so a non-string title propagates as-is.
Definition of done
When no explicit output path is supplied, the filename derived from server metadata is
sanitized so it can only ever name a file in the intended directory: no path separators, no .. components, no absolute paths, no leading dots, no NUL bytes, no reserved device names.
An explicitly supplied outPath is honoured verbatim — the caller is trusted, the server
is not. This distinction is documented in a code comment.
A title that sanitizes to the empty string falls back to a deterministic safe name derived
from the file ID.
decryptFile validates the shape of the decrypted metadata JSON rather than trusting it:
a non-string title, or a missing one, produces a well-defined result instead of
propagating an arbitrary value.
Sanitization lives in one place and is shared with src/backup.ts rather than duplicated.
Tests cover, as literate documentation of the threat model: ../ traversal, absolute path,
embedded separators, NUL byte, empty title, non-string title, and a normal title passing
through unchanged.
make check green.
TODO.md updated in the same commit.
## Problem
`downloadFile` (`src/download/index.ts:71`) falls back to `file.metadata.title` as the output
path when no `outPath` is given, and `downloadThumbnail` (`:84`) uses it with a `thumb_`
prefix. `file.metadata.title` is decrypted from server-supplied metadata; it is fully
attacker-controlled if the account is compromised or the server is hostile, and quak's entire
premise is that the server is not trusted with plaintext.
A title of `../../.ssh/authorized_keys` writes outside the intended directory. `bin/quak.ts`
passes `opts.out` straight through, which is `undefined` unless `--out` was given, so
`quak get <id>` in a shell hits this path directly.
`src/backup.ts` has a `sanitizePath` helper for the backup tree, but the download layer does
not use it and `sanitizePath` itself is not applied to path separators in a way that has been
tested against traversal payloads.
`src/model/decrypt.ts:100` compounds this: the decrypted metadata JSON is parsed as implicit
`any` and every field is read unchecked, so a non-string `title` propagates as-is.
## Definition of done
1. When no explicit output path is supplied, the filename derived from server metadata is
sanitized so it can only ever name a file in the intended directory: no path separators, no
`..` components, no absolute paths, no leading dots, no NUL bytes, no reserved device names.
2. An explicitly supplied `outPath` is honoured verbatim — the caller is trusted, the server
is not. This distinction is documented in a code comment.
3. A title that sanitizes to the empty string falls back to a deterministic safe name derived
from the file ID.
4. `decryptFile` validates the shape of the decrypted metadata JSON rather than trusting it:
a non-string `title`, or a missing one, produces a well-defined result instead of
propagating an arbitrary value.
5. Sanitization lives in one place and is shared with `src/backup.ts` rather than duplicated.
6. Tests cover, as literate documentation of the threat model: `../` traversal, absolute path,
embedded separators, NUL byte, empty title, non-string title, and a normal title passing
through unchanged.
7. `make check` green.
8. `TODO.md` updated in the same commit.
clawbot
added this to the 1.0.0 milestone 2026-08-09 03:44:48 +02:00
clawbot
self-assigned this 2026-08-09 03:44:48 +02:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Problem
downloadFile(src/download/index.ts:71) falls back tofile.metadata.titleas the outputpath when no
outPathis given, anddownloadThumbnail(:84) uses it with athumb_prefix.
file.metadata.titleis decrypted from server-supplied metadata; it is fullyattacker-controlled if the account is compromised or the server is hostile, and quak's entire
premise is that the server is not trusted with plaintext.
A title of
../../.ssh/authorized_keyswrites outside the intended directory.bin/quak.tspasses
opts.outstraight through, which isundefinedunless--outwas given, soquak get <id>in a shell hits this path directly.src/backup.tshas asanitizePathhelper for the backup tree, but the download layer doesnot use it and
sanitizePathitself is not applied to path separators in a way that has beentested against traversal payloads.
src/model/decrypt.ts:100compounds this: the decrypted metadata JSON is parsed as implicitanyand every field is read unchecked, so a non-stringtitlepropagates as-is.Definition of done
sanitized so it can only ever name a file in the intended directory: no path separators, no
..components, no absolute paths, no leading dots, no NUL bytes, no reserved device names.outPathis honoured verbatim — the caller is trusted, the serveris not. This distinction is documented in a code comment.
from the file ID.
decryptFilevalidates the shape of the decrypted metadata JSON rather than trusting it:a non-string
title, or a missing one, produces a well-defined result instead ofpropagating an arbitrary value.
src/backup.tsrather than duplicated.../traversal, absolute path,embedded separators, NUL byte, empty title, non-string title, and a normal title passing
through unchanged.
make checkgreen.TODO.mdupdated in the same commit.