diff --git a/context.go b/context.go index 5174033e..66917d79 100644 --- a/context.go +++ b/context.go @@ -717,6 +717,46 @@ func (c *Context) MultipartForm() (*multipart.Form, error) { // SaveUploadedFile uploads the form file to specific dst. func (c *Context) SaveUploadedFile(file *multipart.FileHeader, dst string, perm ...fs.FileMode) error { + return c.saveUploadedFile(file, dst, perm, + os.MkdirAll, + os.Chmod, + func(name string) (*os.File, error) { + return os.OpenFile(name, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o666) + }, + ) +} + +// SaveUploadedFileToRoot uploads the form file to dst within root. +// +// Unlike SaveUploadedFile, all filesystem operations are constrained to root, +// so path traversal and symlink escapes outside root are rejected by os.Root. +// This method requires Go 1.25+. +func (c *Context) SaveUploadedFileToRoot(file *multipart.FileHeader, dst string, root *os.Root, perm ...fs.FileMode) error { + if root == nil { + return errors.New("root is nil") + } + + return c.saveUploadedFile(file, dst, perm, + func(name string, mode os.FileMode) error { + return root.MkdirAll(name, mode) + }, + func(name string, mode os.FileMode) error { + return root.Chmod(name, mode) + }, + func(name string) (*os.File, error) { + return root.OpenFile(name, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o666) + }, + ) +} + +func (c *Context) saveUploadedFile( + file *multipart.FileHeader, + dst string, + perm []fs.FileMode, + mkdirAll func(string, os.FileMode) error, + chmod func(string, os.FileMode) error, + openFile func(string) (*os.File, error), +) error { src, err := file.Open() if err != nil { return err @@ -725,17 +765,21 @@ func (c *Context) SaveUploadedFile(file *multipart.FileHeader, dst string, perm var mode os.FileMode = 0o750 if len(perm) > 0 { - mode = perm[0] + mode = os.FileMode(perm[0]) } + dst = filepath.Clean(dst) dir := filepath.Dir(dst) - if err = os.MkdirAll(dir, mode); err != nil { - return err - } - if err = os.Chmod(dir, mode); err != nil { - return err - } - out, err := os.Create(dst) + if dir != "." && dir != "/" { + if err = mkdirAll(dir, mode); err != nil { + return err + } + + if err = chmod(dir, mode); err != nil { + return err + } + } + out, err := openFile(dst) if err != nil { return err } @@ -743,6 +787,7 @@ func (c *Context) SaveUploadedFile(file *multipart.FileHeader, dst string, perm _, err = io.Copy(out, src) return err + } // Bind checks the Method and Content-Type to select a binding engine automatically, diff --git a/context_test.go b/context_test.go index ef60379d..536e79f3 100644 --- a/context_test.go +++ b/context_test.go @@ -274,6 +274,110 @@ func TestSaveUploadedFileWithPermissionFailed(t *testing.T) { require.Error(t, c.SaveUploadedFile(f, "test/permission_test", mode)) } +func TestSaveUploadedFileToRoot(t *testing.T) { + buf := new(bytes.Buffer) + mw := multipart.NewWriter(buf) + w, err := mw.CreateFormFile("file", "permission_test") + require.NoError(t, err) + _, err = w.Write([]byte("permission_test")) + require.NoError(t, err) + mw.Close() + + c, _ := CreateTestContext(httptest.NewRecorder()) + c.Request, _ = http.NewRequest(http.MethodPost, "/", buf) + c.Request.Header.Set("Content-Type", mw.FormDataContentType()) + f, err := c.FormFile("file") + require.NoError(t, err) + + rootDir := t.TempDir() + root, err := os.OpenRoot(rootDir) + require.NoError(t, err) + t.Cleanup(func() { + require.NoError(t, root.Close()) + }) + + var mode fs.FileMode = 0o755 + require.NoError(t, c.SaveUploadedFileToRoot(f, "test/permission_test", root, mode)) + + content, err := os.ReadFile(filepath.Join(rootDir, "test", "permission_test")) + require.NoError(t, err) + assert.Equal(t, "permission_test", string(content)) + + info, err := os.Stat(filepath.Join(rootDir, "test")) + require.NoError(t, err) + assert.Equal(t, mode, info.Mode().Perm()) +} + +func TestSaveUploadedFileToRootRejectsPathTraversal(t *testing.T) { + buf := new(bytes.Buffer) + mw := multipart.NewWriter(buf) + w, err := mw.CreateFormFile("file", "escape.txt") + require.NoError(t, err) + _, err = w.Write([]byte("escape")) + require.NoError(t, err) + mw.Close() + + c, _ := CreateTestContext(httptest.NewRecorder()) + c.Request, _ = http.NewRequest(http.MethodPost, "/", buf) + c.Request.Header.Set("Content-Type", mw.FormDataContentType()) + f, err := c.FormFile("file") + require.NoError(t, err) + + baseDir := t.TempDir() + rootDir := filepath.Join(baseDir, "root") + require.NoError(t, os.Mkdir(rootDir, 0o755)) + root, err := os.OpenRoot(rootDir) + require.NoError(t, err) + t.Cleanup(func() { + require.NoError(t, root.Close()) + }) + + err = c.SaveUploadedFileToRoot(f, "../escape.txt", root) + require.Error(t, err) + assert.ErrorContains(t, err, "path escapes") + + _, err = os.Stat(filepath.Join(baseDir, "escape.txt")) + assert.ErrorIs(t, err, os.ErrNotExist) +} + +func TestSaveUploadedFileToRootRejectsSymlinkEscape(t *testing.T) { + buf := new(bytes.Buffer) + mw := multipart.NewWriter(buf) + w, err := mw.CreateFormFile("file", "escape.txt") + require.NoError(t, err) + _, err = w.Write([]byte("escape")) + require.NoError(t, err) + mw.Close() + + c, _ := CreateTestContext(httptest.NewRecorder()) + c.Request, _ = http.NewRequest(http.MethodPost, "/", buf) + c.Request.Header.Set("Content-Type", mw.FormDataContentType()) + f, err := c.FormFile("file") + require.NoError(t, err) + + baseDir := t.TempDir() + rootDir := filepath.Join(baseDir, "root") + outsideDir := filepath.Join(baseDir, "outside") + require.NoError(t, os.Mkdir(rootDir, 0o755)) + require.NoError(t, os.Mkdir(outsideDir, 0o755)) + + if err := os.Symlink(outsideDir, filepath.Join(rootDir, "link")); err != nil { + t.Skipf("symlink unsupported: %v", err) + } + + root, err := os.OpenRoot(rootDir) + require.NoError(t, err) + t.Cleanup(func() { + require.NoError(t, root.Close()) + }) + + err = c.SaveUploadedFileToRoot(f, "link/escape.txt", root) + require.Error(t, err) + + _, err = os.Stat(filepath.Join(outsideDir, "escape.txt")) + assert.ErrorIs(t, err, os.ErrNotExist) +} + func TestContextReset(t *testing.T) { router := New() c := router.allocateContext(0) diff --git a/docs/doc.md b/docs/doc.md index 7201df5c..47632965 100644 --- a/docs/doc.md +++ b/docs/doc.md @@ -273,6 +273,23 @@ References issue [#774](https://github.com/gin-gonic/gin/issues/774) and detail > The filename is always optional and must not be used blindly by the application: path information should be stripped, and conversion to the server file system rules should be done. +If `dst` comes from user input, prefer constraining writes with `os.OpenRoot` and `SaveUploadedFileToRoot` so `..` traversal and symlink escapes cannot write outside your upload directory. + +`os.Root` is available in Go 1.25+. + +```go +root, err := os.OpenRoot("./uploads") +if err != nil { + log.Fatal(err) +} +defer root.Close() + +if err := c.SaveUploadedFileToRoot(file, dst, root); err != nil { + c.String(http.StatusBadRequest, "upload failed: %v", err) + return +} +``` + ```go func main() { router := gin.Default()