security: enforce upload size limits (ai:gpt-5) #48

New issue

Closed

opened 2026-05-23 21:23:01 +02:00 by heiko · 0 comments

heiko commented

2026-05-23 21:23:01 +02:00

Owner

Finding from a whole-codebase security review.

Affected code:

cmd/once-server/upload.go:113-144 parses the multipart form and copies a single uploaded file without a byte limit.
cmd/once-server/upload.go:163-183 copies each selected file into a generated zip without per-file or aggregate limits.

Impact:
r.ParseMultipartForm(2 << 20) only controls how much multipart data stays in memory. The request body and copied file content are otherwise unbounded, so an authenticated user, compromised credentials, or an automated client can fill disk and consume CPU by streaming large uploads or large multi-file zip uploads. Partial failures also leave cleanup-dependent store entries.

Suggested fix:

Add explicit config for maximum upload bytes, maximum file count, and optionally maximum zip aggregate size.
Wrap r.Body with http.MaxBytesReader in Upload before parsing.
Use limited copy helpers for both direct file writes and zip entries, return HTTP 413 when exceeded, and purge the partially-created store entry on failure.
Add tests covering oversized single-file and multi-file uploads.

Finding from a whole-codebase security review. Affected code: - cmd/once-server/upload.go:113-144 parses the multipart form and copies a single uploaded file without a byte limit. - cmd/once-server/upload.go:163-183 copies each selected file into a generated zip without per-file or aggregate limits. Impact: `r.ParseMultipartForm(2 << 20)` only controls how much multipart data stays in memory. The request body and copied file content are otherwise unbounded, so an authenticated user, compromised credentials, or an automated client can fill disk and consume CPU by streaming large uploads or large multi-file zip uploads. Partial failures also leave cleanup-dependent store entries. Suggested fix: - Add explicit config for maximum upload bytes, maximum file count, and optionally maximum zip aggregate size. - Wrap `r.Body` with `http.MaxBytesReader` in `Upload` before parsing. - Use limited copy helpers for both direct file writes and zip entries, return HTTP 413 when exceeded, and purge the partially-created store entry on failure. - Add tests covering oversized single-file and multi-file uploads.

heiko added the

security

label

2026-05-24 18:44:09 +02:00

heiko referenced this issue from a commit

2026-05-24 20:40:00 +02:00

security: enforce upload size limits with MaxUploadBytes and MaxFiles config (fixes #48) ai:claude-sonnet-4-5

heiko referenced this issue from a commit

2026-05-24 20:43:23 +02:00

Merge security fixes #51 and #48: combine zip-slip and upload-limit tests

heiko referenced this issue from a pull request that will close it,

2026-05-24 20:44:00 +02:00