Skip to content

fix(storage): bound tar imports, contain storage keys across backends, stop export-file accumulation#346

Closed
rmyndharis wants to merge 1 commit into
mainfrom
fix/import-export-resource-limits
Closed

fix(storage): bound tar imports, contain storage keys across backends, stop export-file accumulation#346
rmyndharis wants to merge 1 commit into
mainfrom
fix/import-export-resource-limits

Conversation

@rmyndharis

Copy link
Copy Markdown
Owner

Four small storage import/export hardening fixes (grouped — all on the /infra/storage/* resource path). All ADMIN-gated; no wire-format change.

1. Tar import is bounded against decompression bombs

importFromStream buffered each tar entry into memory with no size limit and no entry-count limit, so a crafted .tar.gz (small archive, one entry expanding to GBs, or millions of entries) could exhaust process memory. Imports now enforce:

  • a per-entry byte capSTORAGE_IMPORT_MAX_BYTES (default 200 MiB), checked on the running accumulator so it aborts mid-stream, not after buffering
  • a max entry countSTORAGE_IMPORT_MAX_ENTRIES (default 100000)

On breach the whole import is torn down and rejected. A single bad/traversing entry is still skipped-and-continued (unchanged); import remains best-effort/non-atomic (documented in-code).

2. Storage-key containment now covers the S3 backend

The local backend rejected tar entry names that traversed the storage root; the S3 path (media/${key}) had no such guard. Containment is now checked at the backend-agnostic getFile/putFile boundary via isSafeStorageKey — rejecting .. traversal, absolute paths, and NUL/control characters (which are harmless on the local FS but would reach the raw S3 object key). Ordinary media keys are unaffected.

3. Plugin storage is sandbox-contained

A plugin's ctx.storage get/set/delete built a file path from the raw key, so a key containing .. could read/write/delete outside the plugin's own data directory (clobber the registry, another plugin's data, or .env.generated). Keys that escape the sandbox are now rejected; JID-style keys (group:sid:chat@g.us) are preserved.

4. Storage export no longer accumulates on the data volume

GET /infra/storage/export wrote a timestamped tar.gz of all media into data/ and never deleted it, so repeated exports grew without bound on the same volume that holds the live SQLite DBs + session state (a full disk corrupts the gateway). The export now:

  • writes under data/exports/ with a collision-proof Date.now()-${uuid} filename
  • is auto-removed after a TTL (STORAGE_EXPORT_TTL_MS, default 1h)
  • reads each file with fs.promises.readFile instead of a synchronous read that blocked the event loop per file

Kept inside data/ (not the OS temp dir) on purpose: the import handler only accepts paths under data/, and the documented backend-migration flow (docs/14-migration-guide.md) re-imports this file after a container restart — the OS temp dir would be wiped. The migration guide's example path is updated.

Tests

New/updated unit tests: import per-entry + entry-count caps (incl. a multi-chunk 256 KiB entry proving mid-stream abort); isSafeStorageKey (traversal/absolute/NUL/control + JID-key acceptance); get/putFile reject an unsafe key before any S3 call (S3 inherits the guard); plugin-storage refuses a traversing get even when a real file exists at the target, and rejects traversing set/delete; export lands under data/exports, stays import-able, and is TTL-swept (poll-based assertion, not a fixed sleep). Full backend 752/752; dashboard build + lint clean.

An adversarial review of the first cut caught a real regression: moving the export to the OS temp dir broke the documented export→import migration roundtrip (import rejects any path outside data/, and the temp file is wiped on the restart the guide prescribes). Fixed by writing under data/exports/ (import-able + persistent) with the TTL sweep, before this was opened.

…, stop export-file accumulation

- A tar.gz import buffered each entry with no size or count limit (decompression-bomb DoS). Imports now enforce a per-entry byte cap (STORAGE_IMPORT_MAX_BYTES, default 200 MiB) and a max entry count (STORAGE_IMPORT_MAX_ENTRIES, default 100000), aborting the whole import on breach.

- Storage-key containment is now enforced at the backend-agnostic get/putFile boundary (isSafeStorageKey: rejects '..' traversal, absolute paths, and NUL/control chars), so the S3 object key inherits the guard the local path already had.

- A plugin's ctx.storage get/set/delete built a path from the raw key, so a '..' key could escape the plugin's sandbox dir; keys that escape are now rejected (JID-style keys preserved).

- GET /infra/storage/export wrote a timestamped tar.gz of all media into data/ and never deleted it (unbounded disk growth on the volume that holds the live DBs). The export now writes under data/exports/ (kept inside the import-allowed root so the documented backend-migration roundtrip still works across a restart) with a collision-proof filename, is auto-removed after STORAGE_EXPORT_TTL_MS (default 1h), and reads each file asynchronously instead of blocking the event loop.
rmyndharis added a commit that referenced this pull request Jun 19, 2026
* fix(webhook): deliver session lifecycle events and key webhook hardening (#335)

* fix(security): pin outbound webhook and media fetches to validated IP (#338)

* fix(plugins): persist plugin enable/config and restore (#339)

* fix(message): persist bulk-sent messages, sanitize SSRF (#340)

* fix(engine): return the real id for forwarded messages (#341)

* fix(security): harden outbound requests, IPv6 SSRF (#344)

* fix(security): secret-file perms, key pepper, allowedIps (#345)

* fix(storage): bound tar imports, contain storage keys (#346)

* fix(session): reconcile late acks, serialize reactions (#348)

* fix(contract): webhook timeout, bounded shutdown, 501 (#350)

* feat(session): force-kill a stuck session (#352)

* merge #343

* merge #351

* chore(release): v0.4.3 — CHANGELOG, version bump (package.json/dashboard/swagger), README + docs
@rmyndharis

Copy link
Copy Markdown
Owner Author

Shipped in v0.4.3 (integrated via the release PR #354 and tagged v0.4.3). Thanks! 🎉

@rmyndharis rmyndharis closed this Jun 19, 2026
@rmyndharis rmyndharis deleted the fix/import-export-resource-limits branch June 19, 2026 18:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant