From 94b0c1df1486f871ec9c1639eb7e9b2ebe73a17b Mon Sep 17 00:00:00 2001 From: Madhosh Yagnik Date: Sat, 12 Sep 2026 03:35:11 +0530 Subject: [PATCH] pkg/api/handlers/libpod: close quadlet files per iteration and prevent duplicates processMultipartQuadlets previously deferred closing each multipart part and file inside the loop, leaking descriptors until the handler returned. It also opened files with os.Create, silently overwriting earlier files if a request contained parts with duplicate names. Align processMultipartQuadlets with the manifests multipart upload handler: - Wrap each part processing in a closure so part and file descriptors close immediately on each loop iteration. - Use os.OpenFile with os.O_CREATE|os.O_EXCL|os.O_WRONLY (0600) so duplicate filenames return an error instead of silently overwriting. - Sanitize filenames using filepath.Base to prevent directory traversal. - Add unit tests verifying multiple file extraction, duplicate name rejection, and path traversal sanitization. Fixes: #29752 Signed-off-by: Madhosh Yagnik --- pkg/api/handlers/libpod/quadlets.go | 32 ++++-- pkg/api/handlers/libpod/quadlets_test.go | 129 +++++++++++++++++++++++ 2 files changed, 151 insertions(+), 10 deletions(-) create mode 100644 pkg/api/handlers/libpod/quadlets_test.go diff --git a/pkg/api/handlers/libpod/quadlets.go b/pkg/api/handlers/libpod/quadlets.go index 2049e341fa..c83e12f761 100644 --- a/pkg/api/handlers/libpod/quadlets.go +++ b/pkg/api/handlers/libpod/quadlets.go @@ -136,25 +136,37 @@ func processMultipartQuadlets(tempDir string, r *http.Request) ([]string, error) if err != nil { return nil, fmt.Errorf("failed to read multipart: %w", err) } - defer part.Close() filename := part.FileName() if filename == "" { // Skip parts without filenames + _ = part.Close() + continue + } + filename = filepath.Base(filename) + if filename == "." || filename == ".." || filename == string(filepath.Separator) { + _ = part.Close() continue } - // Create file in temp directory + // Write the file in a scope that lets us close it per iteration filePath := filepath.Join(quadletDir, filename) - file, err := os.Create(filePath) - if err != nil { - return nil, fmt.Errorf("failed to create file %s: %w", filename, err) - } - defer file.Close() + if err := func() error { + defer part.Close() - _, err = io.Copy(file, part) - if err != nil { - return nil, fmt.Errorf("failed to write file %s: %w", filename, err) + file, err := os.OpenFile(filePath, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0o600) + if err != nil { + return fmt.Errorf("failed to create file %s: %w", filename, err) + } + defer file.Close() + + _, err = io.Copy(file, part) + if err != nil { + return fmt.Errorf("failed to write file %s: %w", filename, err) + } + return nil + }(); err != nil { + return nil, err } filePaths = append(filePaths, filePath) diff --git a/pkg/api/handlers/libpod/quadlets_test.go b/pkg/api/handlers/libpod/quadlets_test.go new file mode 100644 index 0000000000..32dc04bd8a --- /dev/null +++ b/pkg/api/handlers/libpod/quadlets_test.go @@ -0,0 +1,129 @@ +//go:build !remote && (linux || freebsd) + +package libpod + +import ( + "bytes" + "mime/multipart" + "net/http" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func createMultipartRequest(t *testing.T, files map[string]string) *http.Request { + t.Helper() + var body bytes.Buffer + writer := multipart.NewWriter(&body) + + for filename, content := range files { + part, err := writer.CreateFormFile("file", filename) + require.NoError(t, err) + _, err = part.Write([]byte(content)) + require.NoError(t, err) + } + require.NoError(t, writer.Close()) + + req, err := http.NewRequest(http.MethodPost, "/libpod/quadlets/install", &body) + require.NoError(t, err) + req.Header.Set("Content-Type", writer.FormDataContentType()) + return req +} + +func TestProcessMultipartQuadlets(t *testing.T) { + t.Run("multiple files extracted correctly", func(t *testing.T) { + tempDir := t.TempDir() + files := map[string]string{ + "app1.container": "[Container]\nImage=alpine\n", + "app2.volume": "[Volume]\n", + } + req := createMultipartRequest(t, files) + + paths, err := processMultipartQuadlets(tempDir, req) + require.NoError(t, err) + assert.Len(t, paths, 2) + + for filename, expectedContent := range files { + expectedPath := filepath.Join(tempDir, "quadlets", filename) + assert.Contains(t, paths, expectedPath) + + content, err := os.ReadFile(expectedPath) + require.NoError(t, err) + assert.Equal(t, expectedContent, string(content)) + + info, err := os.Stat(expectedPath) + require.NoError(t, err) + assert.Equal(t, os.FileMode(0o600), info.Mode().Perm()) + } + }) + + t.Run("duplicate filename returns error", func(t *testing.T) { + tempDir := t.TempDir() + var body bytes.Buffer + writer := multipart.NewWriter(&body) + + // Add two parts with the exact same filename + part1, err := writer.CreateFormFile("file", "test.container") + require.NoError(t, err) + _, err = part1.Write([]byte("first")) + require.NoError(t, err) + + part2, err := writer.CreateFormFile("file", "test.container") + require.NoError(t, err) + _, err = part2.Write([]byte("second")) + require.NoError(t, err) + + require.NoError(t, writer.Close()) + + req, err := http.NewRequest(http.MethodPost, "/libpod/quadlets/install", &body) + require.NoError(t, err) + req.Header.Set("Content-Type", writer.FormDataContentType()) + + _, err = processMultipartQuadlets(tempDir, req) + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to create file test.container") + }) + + t.Run("path traversal in filename is sanitized", func(t *testing.T) { + tempDir := t.TempDir() + files := map[string]string{ + "../../evil.container": "[Container]\nImage=evil\n", + } + req := createMultipartRequest(t, files) + + paths, err := processMultipartQuadlets(tempDir, req) + require.NoError(t, err) + require.Len(t, paths, 1) + + expectedPath := filepath.Join(tempDir, "quadlets", "evil.container") + assert.Equal(t, expectedPath, paths[0]) + + content, err := os.ReadFile(expectedPath) + require.NoError(t, err) + assert.Equal(t, "[Container]\nImage=evil\n", string(content)) + }) + + t.Run("dot and dotdot filenames are skipped", func(t *testing.T) { + tempDir := t.TempDir() + files := map[string]string{ + "..": "[Container]\nImage=parent\n", + ".": "[Container]\nImage=dot\n", + "app1.container": "[Container]\nImage=valid\n", + } + req := createMultipartRequest(t, files) + + paths, err := processMultipartQuadlets(tempDir, req) + require.NoError(t, err) + require.Len(t, paths, 1) + + expectedPath := filepath.Join(tempDir, "quadlets", "app1.container") + assert.Equal(t, expectedPath, paths[0]) + + content, err := os.ReadFile(expectedPath) + require.NoError(t, err) + assert.Equal(t, "[Container]\nImage=valid\n", string(content)) + }) +}