Claudio Ortolina
118 lines
5.0 KiB
Markdown
118 lines
5.0 KiB
Markdown
---
|
|
id: doc-21
|
|
title: ML-181 Research - Public Asset Transform Hardening Routes
|
|
type: other
|
|
created_date: "2026-05-13 18:36"
|
|
---
|
|
|
|
# ML-181 Research: Public Asset Transform Hardening Routes
|
|
|
|
## Problem Summary
|
|
|
|
The unauthenticated `GET /public/assets/:transform_payload` endpoint (router.ex:61) decodes arbitrary Base64 JSON via `Transform.decode/1` without validating the `width` field. An attacker with a valid asset hash — obtainable from public email image URLs generated by `RecordsOnThisDayEmail.cover_image_url/2` — can craft variant payloads that:
|
|
|
|
1. **Force repeated native libvips resize attempts** — Each unique payload that misses the ETS cache triggers `Vix.Vips.Operation.thumbnail_buffer/3` (`Image.resize/3`).
|
|
2. **Amplify ETS cache entries** — Cache key is `{payload, format}` where `payload` is the raw path segment string. Variant JSON representations (whitespace, key ordering, extra fields) create distinct cache entries for the same logical transform.
|
|
|
|
**Current legitimate width values:**
|
|
|
|
- `nil` — Original size (authenticated `/assets/` routes, API)
|
|
- `96` — Email thumbnails (`cover_image_url/2`)
|
|
- `150` — API mini covers (`collection_json.ex`)
|
|
- `480` — API thumbnails (`collection_json.ex`)
|
|
|
|
The public route only uses `width: 96` (email). The authenticated routes and API routes are behind session/auth.
|
|
|
|
## Route 1: Width Validation in Transform.decode/1 (Minimal)
|
|
|
|
**Change:** Add validation in `Transform.decode/1` that `width` must be nil or a positive integer within a bounded range (e.g., ≤ 2048 or explicit allowlist `[96, 150, 480]`). Reject with `{:error, :invalid_payload}`.
|
|
|
|
**Files touched:**
|
|
|
|
- `lib/music_library/assets/transform.ex` — Add `validate_width/1` guard in decode
|
|
|
|
**Pros:**
|
|
|
|
- Single file change
|
|
- Directly blocks unbounded resource consumption
|
|
- Existing controller error handling already returns 400 for `:invalid_payload`
|
|
|
|
**Cons:**
|
|
|
|
- Does not address cache amplification from variant payloads (whitespace, key ordering, extra fields) for the same valid width
|
|
- Allowlist approach requires updating when new legitimate widths are added
|
|
|
|
**Cache amplification remains:** `{"hash":"abc","width":96}` and `{"width":96,"hash":"abc"}` produce different cache entries for the same logical transform.
|
|
|
|
---
|
|
|
|
## Route 2: Canonical Cache Key (Structural)
|
|
|
|
**Change:** Derive ETS cache key from a canonical representation of the validated Transform struct instead of the raw payload string. E.g., `"hash:width"` or a binary term.
|
|
|
|
**Files touched:**
|
|
|
|
- `lib/music_library/assets/cache.ex` — Update key format and get/set functions
|
|
- `lib/music_library_web/controllers/asset_controller.ex` — Pass canonical key instead of raw payload to Cache
|
|
|
|
**Pros:**
|
|
|
|
- Eliminates cache amplification entirely — unique cache entries = unique (hash, width) pairs
|
|
- ETag can remain the raw payload for HTTP caching correctness, or switch to canonical key
|
|
|
|
**Cons:**
|
|
|
|
- Does not validate width — still needs Route 1
|
|
- Requires coordinated change in Cache and Controller
|
|
- ETag change: if ETag switches to canonical key, existing cached browser responses are invalidated (acceptable — they'd re-fetch)
|
|
|
|
---
|
|
|
|
## Route 3: Combined — Width Validation + Canonical Cache Key (Recommended)
|
|
|
|
**Change:** Validate width in `Transform.decode/1` AND derive cache key from the canonical struct.
|
|
|
|
**Files touched:**
|
|
|
|
- `lib/music_library/assets/transform.ex` — Add `validate_width/1` in decode; add `canonical_key/1`
|
|
- `lib/music_library/assets/cache.ex` — No changes needed (key format stays `{payload, format}`, but payload is canonical)
|
|
- `lib/music_library_web/controllers/asset_controller.ex` — Use canonical key for Cache calls
|
|
- `test/music_library/assets/transform_test.exs` — Add validation tests
|
|
- `test/music_library_web/controllers/asset_controller_test.exs` — Add attacks as tests
|
|
|
|
**Pros:**
|
|
|
|
- Addresses both resource consumption and cache amplification
|
|
- Each change is small and independently verifiable
|
|
- No architectural changes — same modules, same data flow
|
|
|
|
**Cons:**
|
|
|
|
- Changes the cache key derivation for existing cached entries (acceptable — entries are ephemeral and TTL-based)
|
|
|
|
---
|
|
|
|
## Route 4: Payload Signing (Comprehensive, Deferred)
|
|
|
|
**Change:** HMAC-sign transform payloads using the app's secret key. Only server-generated URLs with valid signatures are accepted.
|
|
|
|
**Pros:**
|
|
|
|
- Strongest defense: attackers cannot construct any valid payload
|
|
- No need for width allowlists (signature proves server origin)
|
|
|
|
**Cons:**
|
|
|
|
- Adds key management complexity (signing key must be consistent across deploys)
|
|
- Requires changes at every URL generation site (email, API JSON, LiveView tests)
|
|
- Overkill for single-operator threat model — asset hash obscurity is sufficient
|
|
- Signature in URL makes email URLs longer and less cacheable
|
|
|
|
**Verdict:** Deferred. Revisit if the app becomes multi-user or asset hashes become guessable.
|
|
|
|
---
|
|
|
|
## Recommendation
|
|
|
|
**Route 3 (Combined)** is the right trade-off. It addresses the finding completely with minimal complexity. Route 4 is documented for future consideration.
|