fix: validate image name before building registry path #4

Merged
marvin merged 1 commits from fix/registry-path-validation into master 2026-08-26 22:18:43 +02:00
Collaborator

Fixes sec-2 (minor, security) from the code review.

Problem

The user-supplied image/name CLI argument was interpolated unsanitized into registry/{image}/initrd, so a name containing ../ (e.g. slim run ../../.ssh/authorized_keys) escaped the XDG data dir: place_data_file/create_dir_all created attacker-chosen directories, and vmlinuz/initrd were read from / written to arbitrary locations.

Fix

  • validate_image_name: rejects empty names, ., .., and anything outside [A-Za-z0-9._:-] (covers normal image refs like alpine:3.15)
  • Applied at the registry_dir chokepoint used by both build and run, so validation happens before any file operation
  • registry_dir now returns Result (also removes the double-.unwrap() panic on unwritable XDG dirs)
  • Defense in depth: containment check that the resolved path stays under the registry root
  • Unit tests for accepted and rejected names (first tests in the repo)

Verification

  • slim run '../../.ssh/authorized_keys'invalid image name, exit 1
  • slim build '../../tmp/evil' → rejected (also caught by podman's own reference parsing)
  • slim build mock-vm → still succeeds end-to-end
  • cargo fmt --check, cargo clippy -- -D warnings, cargo test (2 tests) pass

@hulthe please review — will not merge until approved.

Fixes **sec-2 (minor, security)** from the code review. ## Problem The user-supplied `image`/`name` CLI argument was interpolated unsanitized into `registry/{image}/initrd`, so a name containing `../` (e.g. `slim run ../../.ssh/authorized_keys`) escaped the XDG data dir: `place_data_file`/`create_dir_all` created attacker-chosen directories, and `vmlinuz`/`initrd` were read from / written to arbitrary locations. ## Fix - `validate_image_name`: rejects empty names, `.`, `..`, and anything outside `[A-Za-z0-9._:-]` (covers normal image refs like `alpine:3.15`) - Applied at the `registry_dir` chokepoint used by both `build` and `run`, so validation happens before any file operation - `registry_dir` now returns `Result` (also removes the double-`.unwrap()` panic on unwritable XDG dirs) - Defense in depth: containment check that the resolved path stays under the registry root - Unit tests for accepted and rejected names (first tests in the repo) ## Verification - `slim run '../../.ssh/authorized_keys'` → `invalid image name`, exit 1 - `slim build '../../tmp/evil'` → rejected (also caught by podman's own reference parsing) - `slim build mock-vm` → still succeeds end-to-end - `cargo fmt --check`, `cargo clippy -- -D warnings`, `cargo test` (2 tests) pass @hulthe please review — will not merge until approved.
marvin added 1 commit 2026-08-26 22:12:17 +02:00
fix: validate image name before building registry path
CI / build (pull_request) Successful in 10s
CI / build (push) Successful in 10s
5b80885207
The user-supplied image/name argument was interpolated unsanitized into
registry/{name}/initrd, so a name containing '../' (e.g. 'slim run
../../.ssh/authorized_keys') escaped the XDG data dir and let slim
create directories and read/write files at attacker-chosen locations.

validate_image_name now rejects empty names, '.', '..' and anything
outside [A-Za-z0-9._:-] at the registry_dir chokepoint used by both
build and run, plus a defense-in-depth containment check that the
resolved path stays under the registry root.

Fixes sec-2 from the code review.
marvin force-pushed fix/registry-path-validation from cb87541151 to 5b80885207 2026-08-26 22:12:17 +02:00 Compare
hulthe approved these changes 2026-08-26 22:18:03 +02:00
marvin merged commit 5b80885207 into master 2026-08-26 22:18:43 +02:00
hulthe deleted branch fix/registry-path-validation 2026-09-07 17:03:03 +02:00
Sign in to join this conversation.
No Reviewers
No labels
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: hulthe/boco#4