Skip to content

S3 key built with raw user-supplied filename, enabling key injection #44

@windoze95

Description

@windoze95

Issue

In internal/handlers/image.go (line 65), the S3 object key is constructed using the raw filename from the multipart upload header:

s3Key := fmt.Sprintf("uploads/%d/images/%s", user.ID, header.Filename)

header.Filename is user-controlled. A malicious user can supply a filename containing path separators (e.g., ../../admin/config.json), unicode control characters, or null bytes, resulting in an S3 key like uploads/123/images/../../admin/config.json. While S3 doesn't have a traditional filesystem, this can:

  1. Overwrite objects in unintended key prefixes outside uploads/<user_id>/images/
  2. Bypass key-based access policies that assume a predictable prefix structure
  3. Leak information via crafted keys that violate expected naming conventions

Note: issue #28 (missing Content-Type on S3 uploads) covers a related but distinct problem.

Suggested Fix

Sanitize the filename before using it in the key. The simplest fix is to use only the base filename and generate a UUID for uniqueness:

import (
    "github.com/google/uuid"
    "path/filepath"
    "strings"
)

ext := strings.ToLower(filepath.Ext(header.Filename))
s3Key := fmt.Sprintf("uploads/%d/images/%s%s", user.ID, uuid.New().String(), ext)

This removes any path traversal risk while preserving the file extension for Content-Type inference.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions