diff --git a/TODO.md b/TODO.md index 95414b3..fd125d4 100644 --- a/TODO.md +++ b/TODO.md @@ -157,13 +157,13 @@ A single linear checklist of tasks to implement the complete pixa caching image - [x] Implement image resizing with size options (WxH, 0x0, orig) - [x] Implement format conversion (JPEG, PNG, WebP, AVIF) - [x] Implement quality parameter support -- [ ] Implement max input dimensions validation -- [ ] Implement max output dimensions validation +- [x] Implement max input dimensions validation +- [x] Implement max output dimensions validation - [ ] Implement EXIF/metadata stripping - [x] Implement fit modes (cover, contain, fill, inside, outside) ## Security -- [ ] Implement path traversal prevention +- [x] Implement path traversal prevention - [ ] Implement request sanitization - [ ] Implement response header sanitization - [ ] Implement referer blacklist diff --git a/internal/imgcache/processor.go b/internal/imgcache/processor.go index 7ce8721..cf629ad 100644 --- a/internal/imgcache/processor.go +++ b/internal/imgcache/processor.go @@ -3,6 +3,7 @@ package imgcache import ( "bytes" "context" + "errors" "fmt" "image" "image/gif" @@ -14,6 +15,13 @@ import ( "golang.org/x/image/webp" ) +// MaxInputDimension is the maximum allowed width or height for input images. +// Images larger than this are rejected to prevent DoS via decompression bombs. +const MaxInputDimension = 8192 + +// ErrInputTooLarge is returned when input image dimensions exceed MaxInputDimension. +var ErrInputTooLarge = errors.New("input image dimensions exceed maximum") + // ImageProcessor implements the Processor interface using pure Go libraries. type ImageProcessor struct{} @@ -45,6 +53,11 @@ func (p *ImageProcessor) Process( origWidth := bounds.Dx() origHeight := bounds.Dy() + // Validate input dimensions to prevent DoS via decompression bombs + if origWidth > MaxInputDimension || origHeight > MaxInputDimension { + return nil, ErrInputTooLarge + } + // Determine target dimensions targetWidth := req.Size.Width targetHeight := req.Size.Height diff --git a/internal/imgcache/processor_test.go b/internal/imgcache/processor_test.go index 209fd93..135ca80 100644 --- a/internal/imgcache/processor_test.go +++ b/internal/imgcache/processor_test.go @@ -240,3 +240,74 @@ func TestImageProcessor_SupportedFormats(t *testing.T) { t.Error("SupportedOutputFormats() returned empty slice") } } + +func TestImageProcessor_RejectsOversizedInput(t *testing.T) { + proc := NewImageProcessor() + ctx := context.Background() + + // Create an image that exceeds MaxInputDimension (e.g., 10000x100) + // This should be rejected before processing to prevent DoS + input := createTestJPEG(t, 10000, 100) + + req := &ImageRequest{ + Size: Size{Width: 100, Height: 100}, + Format: FormatJPEG, + Quality: 85, + FitMode: FitCover, + } + + _, err := proc.Process(ctx, bytes.NewReader(input), req) + if err == nil { + t.Error("Process() should reject oversized input images") + } + + if err != ErrInputTooLarge { + t.Errorf("Process() error = %v, want ErrInputTooLarge", err) + } +} + +func TestImageProcessor_RejectsOversizedInputHeight(t *testing.T) { + proc := NewImageProcessor() + ctx := context.Background() + + // Create an image with oversized height + input := createTestJPEG(t, 100, 10000) + + req := &ImageRequest{ + Size: Size{Width: 100, Height: 100}, + Format: FormatJPEG, + Quality: 85, + FitMode: FitCover, + } + + _, err := proc.Process(ctx, bytes.NewReader(input), req) + if err == nil { + t.Error("Process() should reject oversized input images") + } + + if err != ErrInputTooLarge { + t.Errorf("Process() error = %v, want ErrInputTooLarge", err) + } +} + +func TestImageProcessor_AcceptsMaxDimensionInput(t *testing.T) { + proc := NewImageProcessor() + ctx := context.Background() + + // Create an image at exactly MaxInputDimension - should be accepted + // Using smaller dimensions to keep test fast + input := createTestJPEG(t, MaxInputDimension, 100) + + req := &ImageRequest{ + Size: Size{Width: 100, Height: 100}, + Format: FormatJPEG, + Quality: 85, + FitMode: FitCover, + } + + result, err := proc.Process(ctx, bytes.NewReader(input), req) + if err != nil { + t.Fatalf("Process() should accept images at MaxInputDimension, got error: %v", err) + } + defer result.Content.Close() +} diff --git a/internal/imgcache/urlparser.go b/internal/imgcache/urlparser.go index 30c9356..ec69d9c 100644 --- a/internal/imgcache/urlparser.go +++ b/internal/imgcache/urlparser.go @@ -3,6 +3,7 @@ package imgcache import ( "errors" "fmt" + "net/url" "regexp" "strconv" "strings" @@ -16,6 +17,7 @@ var ( ErrInvalidSize = errors.New("invalid size format") ErrInvalidFormat = errors.New("invalid or unsupported format") ErrDimensionTooLarge = errors.New("dimension exceeds maximum") + ErrPathTraversal = errors.New("path traversal detected") ) // MaxDimension is the maximum allowed width or height. @@ -73,6 +75,11 @@ func ParseImageURL(urlPath string) (*ParsedURL, error) { // parseImageComponents parses //. structure. func parseImageComponents(remainder string) (*ParsedURL, error) { + // Check for path traversal before any other processing + if err := checkPathTraversal(remainder); err != nil { + return nil, err + } + // Find the last path segment which contains size.format lastSlash := strings.LastIndex(remainder, "/") if lastSlash == -1 { @@ -130,6 +137,64 @@ func parseImageComponents(remainder string) (*ParsedURL, error) { }, nil } +// checkPathTraversal detects path traversal attempts in a URL path. +// It checks for various attack vectors including: +// - Direct ../ sequences +// - URL-encoded variants (%2e%2e, %252e%252e) +// - Backslash variants (..\) +// - Null byte injection (%00) +func checkPathTraversal(path string) error { + // First, URL-decode the path to catch encoded attacks + // Decode multiple times to catch double-encoding + decoded := path + for range 3 { + newDecoded, err := url.PathUnescape(decoded) + if err != nil { + // Malformed encoding is suspicious + return ErrPathTraversal + } + + if newDecoded == decoded { + break + } + + decoded = newDecoded + } + + // Normalize backslashes to forward slashes + normalized := strings.ReplaceAll(decoded, "\\", "/") + + // Check for null bytes + if strings.Contains(normalized, "\x00") { + return ErrPathTraversal + } + + // Check for parent directory traversal + // Look for "/.." or "../" patterns + if strings.Contains(normalized, "/../") || + strings.Contains(normalized, "/..") || + strings.HasPrefix(normalized, "../") || + strings.HasSuffix(normalized, "/..") || + normalized == ".." { + return ErrPathTraversal + } + + // Also check for ".." as a path segment in the original path + // This catches cases where the path hasn't been normalized + segments := strings.Split(path, "/") + for _, seg := range segments { + // URL decode the segment + decodedSeg, _ := url.PathUnescape(seg) + decodedSeg = strings.ReplaceAll(decodedSeg, "\\", "/") + + if decodedSeg == ".." { + return ErrPathTraversal + } + } + + return nil +} + // parseSizeFormat parses strings like "800x600.webp" or "orig.png" func parseSizeFormat(s string) (Size, ImageFormat, error) { matches := sizeFormatRegex.FindStringSubmatch(s) diff --git a/internal/imgcache/urlparser_test.go b/internal/imgcache/urlparser_test.go index a7cf141..bcfc61f 100644 --- a/internal/imgcache/urlparser_test.go +++ b/internal/imgcache/urlparser_test.go @@ -247,6 +247,94 @@ func TestParsedURL_ToImageRequest(t *testing.T) { } } +func TestParseImageURL_PathTraversal(t *testing.T) { + // All path traversal attempts should be rejected + tests := []struct { + name string + input string + }{ + { + name: "simple parent directory", + input: "/v1/image/cdn.example.com/../etc/passwd/800x600.jpeg", + }, + { + name: "double parent directory", + input: "/v1/image/cdn.example.com/../../etc/passwd/800x600.jpeg", + }, + { + name: "parent in middle of path", + input: "/v1/image/cdn.example.com/photos/../../../etc/passwd/800x600.jpeg", + }, + { + name: "encoded parent directory", + input: "/v1/image/cdn.example.com/photos/%2e%2e/secret/800x600.jpeg", + }, + { + name: "double encoded parent", + input: "/v1/image/cdn.example.com/photos/%252e%252e/secret/800x600.jpeg", + }, + { + name: "backslash traversal", + input: "/v1/image/cdn.example.com/photos/..\\secret/800x600.jpeg", + }, + { + name: "mixed slashes", + input: "/v1/image/cdn.example.com/photos/../\\../secret/800x600.jpeg", + }, + { + name: "null byte injection", + input: "/v1/image/cdn.example.com/photos/image.jpg%00.png/800x600.jpeg", + }, + { + name: "parent at start of path", + input: "/v1/image/cdn.example.com/../800x600.jpeg", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := ParseImageURL(tt.input) + if err == nil { + t.Error("ParseImageURL() should reject path traversal attempts") + } + + if err != ErrPathTraversal { + t.Errorf("ParseImageURL() error = %v, want ErrPathTraversal", err) + } + }) + } +} + +func TestParseImagePath_PathTraversal(t *testing.T) { + // Test path traversal via ParseImagePath (chi wildcard) + tests := []struct { + name string + input string + }{ + { + name: "parent directory in path", + input: "cdn.example.com/photos/../secret/800x600.jpeg", + }, + { + name: "encoded traversal", + input: "cdn.example.com/photos/%2e%2e/secret/800x600.jpeg", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := ParseImagePath(tt.input) + if err == nil { + t.Error("ParseImagePath() should reject path traversal attempts") + } + + if err != ErrPathTraversal { + t.Errorf("ParseImagePath() error = %v, want ErrPathTraversal", err) + } + }) + } +} + // errorIs checks if err matches target (handles wrapped errors). func errorIs(err, target error) bool { if err == target {