[Security][Hardening] Contain path traversal: identity/upload-derived path segments in blog-media move #437

New Issue

2026-07-06T06:41:19Z

spikerj commented

2026-07-06 06:41:19 +00:00

Observation

SpikerSoft.EventHandlers.BlogMediaProcessor/Services/BlogMediaMoveConsumer.cs builds the destination path for uploaded blog media directly from event fields:

var userDir = Path.Combine(finalBasePath, evt.Username);          // finalBasePath = /app/blogs
var finalPath = Path.Combine(userDir, $"{evt.MediaId}{evt.FileExtension}");

evt.Username (and, to a lesser extent, FileExtension) originate from upload-time input. They are not run through any containment check, so a value containing ../ — or an absolute path, which Path.Combine silently lets replace the base — could write/create directories outside /app/blogs, and the same evt.Username is reflected into finalUrl = /blog-pictures/{Username}/{file}.

Exploitability depends on what the Keycloak realm permits in a username, but relying on that is fragile; the sink should be hardened regardless.

For contrast, the ebook mover (FileMovementService) is already safe — ISBNs are validated by BookUploadValidator (regex + checksum) and the filename is Path.GetFileName-stripped — so no change was needed there. The Ops query builders similarly demonstrate the escape-at-the-sink discipline.

Fix

Added SpikerSoft.Common.IO.PathSafety.CombineContained(baseDir, ...segments) — resolves the combined path and throws UnauthorizedAccessException if it escapes the base (catches ../ traversal and absolute-path replacement) while leaving normal names/sub-paths untouched. Routed both blog-media combines through it.

Verification

PathSafetyTests (8): ../, multi-level ../../, embedded a/../../b, and absolute-path segments all throw; normal single/multi-segment names stay within base. Build clean.

Resolved by spikersoft-backend PR (linked below). Filing for traceability.

## Observation `SpikerSoft.EventHandlers.BlogMediaProcessor/Services/BlogMediaMoveConsumer.cs` builds the destination path for uploaded blog media directly from event fields: ```csharp var userDir = Path.Combine(finalBasePath, evt.Username); // finalBasePath = /app/blogs var finalPath = Path.Combine(userDir, $"{evt.MediaId}{evt.FileExtension}"); ``` `evt.Username` (and, to a lesser extent, `FileExtension`) originate from upload-time input. They are **not** run through any containment check, so a value containing `../` — or an absolute path, which `Path.Combine` silently lets *replace* the base — could write/create directories **outside** `/app/blogs`, and the same `evt.Username` is reflected into `finalUrl = /blog-pictures/{Username}/{file}`. Exploitability depends on what the Keycloak realm permits in a username, but relying on that is fragile; the sink should be hardened regardless. For contrast, the ebook mover (`FileMovementService`) is **already safe** — ISBNs are validated by `BookUploadValidator` (regex + checksum) and the filename is `Path.GetFileName`-stripped — so no change was needed there. The Ops query builders similarly demonstrate the escape-at-the-sink discipline. ## Fix Added `SpikerSoft.Common.IO.PathSafety.CombineContained(baseDir, ...segments)` — resolves the combined path and throws `UnauthorizedAccessException` if it escapes the base (catches `../` traversal and absolute-path replacement) while leaving normal names/sub-paths untouched. Routed both blog-media combines through it. ## Verification `PathSafetyTests` (8): `../`, multi-level `../../`, embedded `a/../../b`, and absolute-path segments all throw; normal single/multi-segment names stay within base. Build clean. Resolved by spikersoft-backend PR (linked below). Filing for traceability.

spikerj commented

2026-07-06 14:12:59 +00:00

Resolved in spikersoft-backend PR #127 (merged to master as 7364964). PathSafety.CombineContained now contains the blog-media move to /app/blogs (rejects ../ traversal + absolute-path replacement); FileMovementService confirmed already safe. 8/8 tests. Closing.

Resolved in spikersoft-backend PR #127 (merged to `master` as `7364964`). `PathSafety.CombineContained` now contains the blog-media move to `/app/blogs` (rejects `../` traversal + absolute-path replacement); `FileMovementService` confirmed already safe. 8/8 tests. Closing.

spikerj closed this issue

2026-07-06 14:13:33 +00:00

spikerj referenced this issue

2026-07-20 05:58:32 +00:00

EPIC: Angular 22 + Nx 23 full-leverage modernization (signal forms, resource APIs, affected CI, remote cache) #722

spikerj referenced this issue

2026-07-20 20:11:46 +00:00

bug(blog): media removal is a placeholder — no backend endpoint, viewer button does nothing #747

spikerj referenced this issue

2026-07-20 20:11:46 +00:00

bug(blog): comment endpoints are anonymous and comment author identity is client-spoofable #748

Sign in to join this conversation.