From a5376e9747815e764085a76cacc9881e01d8ab7d Mon Sep 17 00:00:00 2001 From: jcj46548-ux Date: Sat, 6 Jun 2026 22:08:30 +0800 Subject: [PATCH 1/3] fix: prevent unsafe plugin package extraction --- pkg/plugin_packager/decoder/zip.go | 101 +++++++++++++++--- .../decoder/zip_extract_test.go | 100 +++++++++++++++++ 2 files changed, 184 insertions(+), 17 deletions(-) create mode 100644 pkg/plugin_packager/decoder/zip_extract_test.go diff --git a/pkg/plugin_packager/decoder/zip.go b/pkg/plugin_packager/decoder/zip.go index d991ccdc3..7a4c87c64 100644 --- a/pkg/plugin_packager/decoder/zip.go +++ b/pkg/plugin_packager/decoder/zip.go @@ -18,6 +18,8 @@ import ( "github.com/langgenius/dify-plugin-daemon/pkg/utils/parser" ) +var errUnsafeZipPath = errors.New("unsafe path in plugin package") + type ZipPluginDecoder struct { PluginDecoder PluginDecoderHelper @@ -297,27 +299,57 @@ func (z *ZipPluginDecoder) UniqueIdentity() (plugin_entities.PluginUniqueIdentif func (z *ZipPluginDecoder) ExtractTo(dst string) error { // copy to working directory - if err := z.Walk(func(filename, dir string) error { - workingPath := path.Join(dst, dir) - // check if directory exists - if err := os.MkdirAll(workingPath, 0755); err != nil { - return err - } - - bytes, err := z.ReadFile(filepath.Join(dir, filename)) - if err != nil { - return err - } - - filename = filepath.Join(workingPath, filename) + if z.reader == nil { + return z.err + } - // copy file - if err := os.WriteFile(filename, bytes, 0644); err != nil { - return err + if err := func() error { + for _, file := range z.reader.File { + targetPath, err := safeExtractPath(dst, file.Name) + if err != nil { + return err + } + + if file.FileInfo().IsDir() { + if err := os.MkdirAll(targetPath, 0755); err != nil { + return err + } + continue + } + + workingPath := filepath.Dir(targetPath) + // check if directory exists + if err := os.MkdirAll(workingPath, 0755); err != nil { + return err + } + + reader, err := file.Open() + if err != nil { + return err + } + + writer, err := os.OpenFile(targetPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0644) + if err != nil { + reader.Close() + return err + } + + if _, err := io.Copy(writer, reader); err != nil { + reader.Close() + writer.Close() + return err + } + if err := reader.Close(); err != nil { + writer.Close() + return err + } + if err := writer.Close(); err != nil { + return err + } } return nil - }); err != nil { + }(); err != nil { // if error, delete the working directory os.RemoveAll(dst) return errors.Join(fmt.Errorf("copy plugin to working directory error: %v", err), err) @@ -326,6 +358,41 @@ func (z *ZipPluginDecoder) ExtractTo(dst string) error { return nil } +func safeExtractPath(dst, entryName string) (string, error) { + entryPath := path.Clean(entryName) + if entryPath == "." || + path.IsAbs(entryPath) || + strings.HasPrefix(entryPath, "../") || + strings.Contains(entryPath, "/../") || + strings.Contains(entryPath, `\`) || + filepath.IsAbs(filepath.FromSlash(entryPath)) { + return "", fmt.Errorf("%w: %s", errUnsafeZipPath, entryName) + } + + targetPath := filepath.Join(dst, filepath.FromSlash(entryPath)) + if !pathIsInside(dst, targetPath) { + return "", fmt.Errorf("%w: %s", errUnsafeZipPath, entryName) + } + + return targetPath, nil +} + +func pathIsInside(root, target string) bool { + rootAbs, err := filepath.Abs(root) + if err != nil { + return false + } + targetAbs, err := filepath.Abs(target) + if err != nil { + return false + } + rel, err := filepath.Rel(rootAbs, targetAbs) + if err != nil { + return false + } + return rel == "." || (rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator))) +} + func (z *ZipPluginDecoder) CheckAssetsValid() error { return z.PluginDecoderHelper.CheckAssetsValid(z) } diff --git a/pkg/plugin_packager/decoder/zip_extract_test.go b/pkg/plugin_packager/decoder/zip_extract_test.go new file mode 100644 index 000000000..aeafeb83a --- /dev/null +++ b/pkg/plugin_packager/decoder/zip_extract_test.go @@ -0,0 +1,100 @@ +package decoder + +import ( + "archive/zip" + "bytes" + "errors" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func buildZipPlugin(t *testing.T, files map[string][]byte) []byte { + t.Helper() + + var buffer bytes.Buffer + zipWriter := zip.NewWriter(&buffer) + for name, content := range files { + if len(content) == 0 && strings.HasSuffix(name, "/") { + _, err := zipWriter.Create(name) + require.NoError(t, err) + continue + } + + writer, err := zipWriter.Create(name) + require.NoError(t, err) + _, err = writer.Write(content) + require.NoError(t, err) + } + require.NoError(t, zipWriter.Close()) + + return buffer.Bytes() +} + +func minimalPluginFiles(t *testing.T) map[string][]byte { + t.Helper() + + manifest, err := os.ReadFile(filepath.Join("..", "testdata", "manifest.yaml")) + require.NoError(t, err) + endpoint, err := os.ReadFile(filepath.Join("..", "testdata", "neko.yaml")) + require.NoError(t, err) + + return map[string][]byte{ + "manifest.yaml": manifest, + "neko.yaml": endpoint, + } +} + +func TestZipPluginDecoderExtractToRejectsParentPath(t *testing.T) { + files := minimalPluginFiles(t) + files["../escaped.txt"] = []byte("escaped") + + zipDecoder, err := NewZipPluginDecoder(buildZipPlugin(t, files)) + require.NoError(t, err) + + parent := t.TempDir() + dst := filepath.Join(parent, "plugin") + err = zipDecoder.ExtractTo(dst) + + require.Error(t, err) + assert.True(t, errors.Is(err, errUnsafeZipPath)) + assert.NoFileExists(t, filepath.Join(parent, "escaped.txt")) + assert.NoDirExists(t, dst) +} + +func TestZipPluginDecoderExtractToRejectsBackslashPath(t *testing.T) { + files := minimalPluginFiles(t) + files[`..\escaped.txt`] = []byte("escaped") + + zipDecoder, err := NewZipPluginDecoder(buildZipPlugin(t, files)) + require.NoError(t, err) + + parent := t.TempDir() + dst := filepath.Join(parent, "plugin") + err = zipDecoder.ExtractTo(dst) + + require.Error(t, err) + assert.True(t, errors.Is(err, errUnsafeZipPath)) + assert.NoFileExists(t, filepath.Join(parent, "escaped.txt")) + assert.NoDirExists(t, dst) +} + +func TestZipPluginDecoderExtractToAllowsNestedPath(t *testing.T) { + files := minimalPluginFiles(t) + files["nested/"] = nil + files["nested/file.txt"] = []byte("ok") + + zipDecoder, err := NewZipPluginDecoder(buildZipPlugin(t, files)) + require.NoError(t, err) + + dst := filepath.Join(t.TempDir(), "plugin") + require.NoError(t, zipDecoder.ExtractTo(dst)) + + extracted, err := os.ReadFile(filepath.Join(dst, "nested", "file.txt")) + require.NoError(t, err) + assert.Equal(t, []byte("ok"), extracted) +} From 3d36020876a96e28b7fd381eb707af2bebfb526a Mon Sep 17 00:00:00 2001 From: jcj46548-ux Date: Sat, 6 Jun 2026 22:18:14 +0800 Subject: [PATCH 2/3] fix: bound zip entry extraction by declared size --- pkg/plugin_packager/decoder/zip.go | 20 +++++++++++++- .../decoder/zip_extract_test.go | 27 +++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/pkg/plugin_packager/decoder/zip.go b/pkg/plugin_packager/decoder/zip.go index 7a4c87c64..dae9c241d 100644 --- a/pkg/plugin_packager/decoder/zip.go +++ b/pkg/plugin_packager/decoder/zip.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "io/fs" + "math" "os" "path" "path/filepath" @@ -334,7 +335,7 @@ func (z *ZipPluginDecoder) ExtractTo(dst string) error { return err } - if _, err := io.Copy(writer, reader); err != nil { + if err := copyZipFile(writer, reader, file.UncompressedSize64); err != nil { reader.Close() writer.Close() return err @@ -361,6 +362,7 @@ func (z *ZipPluginDecoder) ExtractTo(dst string) error { func safeExtractPath(dst, entryName string) (string, error) { entryPath := path.Clean(entryName) if entryPath == "." || + entryPath == ".." || path.IsAbs(entryPath) || strings.HasPrefix(entryPath, "../") || strings.Contains(entryPath, "/../") || @@ -377,6 +379,22 @@ func safeExtractPath(dst, entryName string) (string, error) { return targetPath, nil } +func copyZipFile(writer io.Writer, reader io.Reader, uncompressedSize uint64) error { + if uncompressedSize > math.MaxInt64-1 { + return fmt.Errorf("zip entry is too large: %d bytes", uncompressedSize) + } + + limit := int64(uncompressedSize) + 1 + written, err := io.Copy(writer, io.LimitReader(reader, limit)) + if err != nil { + return err + } + if written > int64(uncompressedSize) { + return fmt.Errorf("zip entry exceeds declared uncompressed size: %d bytes", uncompressedSize) + } + return nil +} + func pathIsInside(root, target string) bool { rootAbs, err := filepath.Abs(root) if err != nil { diff --git a/pkg/plugin_packager/decoder/zip_extract_test.go b/pkg/plugin_packager/decoder/zip_extract_test.go index aeafeb83a..4d1e151f4 100644 --- a/pkg/plugin_packager/decoder/zip_extract_test.go +++ b/pkg/plugin_packager/decoder/zip_extract_test.go @@ -4,6 +4,7 @@ import ( "archive/zip" "bytes" "errors" + "io" "os" "path/filepath" "strings" @@ -98,3 +99,29 @@ func TestZipPluginDecoderExtractToAllowsNestedPath(t *testing.T) { require.NoError(t, err) assert.Equal(t, []byte("ok"), extracted) } + +func TestSafeExtractPathRejectsParentDirectoryEntry(t *testing.T) { + _, err := safeExtractPath(t.TempDir(), "..") + + require.Error(t, err) + assert.True(t, errors.Is(err, errUnsafeZipPath)) +} + +func TestCopyZipFileRejectsContentBeyondDeclaredSize(t *testing.T) { + var out bytes.Buffer + + err := copyZipFile(&out, strings.NewReader("toolarge"), 3) + + require.Error(t, err) + assert.Contains(t, err.Error(), "exceeds declared uncompressed size") + assert.Equal(t, "tool", out.String()) +} + +func TestCopyZipFileAllowsDeclaredSize(t *testing.T) { + var out bytes.Buffer + + err := copyZipFile(&out, io.NopCloser(strings.NewReader("ok")), 2) + + require.NoError(t, err) + assert.Equal(t, "ok", out.String()) +} From efd884163645de6d168de34f9bde435ad7b5d11b Mon Sep 17 00:00:00 2001 From: jcj46548-ux Date: Tue, 9 Jun 2026 23:02:04 +0800 Subject: [PATCH 3/3] fix: constrain zip extraction to root directory --- pkg/plugin_packager/decoder/zip.go | 97 ++++++++----------- .../decoder/zip_extract_test.go | 4 +- 2 files changed, 44 insertions(+), 57 deletions(-) diff --git a/pkg/plugin_packager/decoder/zip.go b/pkg/plugin_packager/decoder/zip.go index dae9c241d..5d180d05f 100644 --- a/pkg/plugin_packager/decoder/zip.go +++ b/pkg/plugin_packager/decoder/zip.go @@ -10,7 +10,6 @@ import ( "math" "os" "path" - "path/filepath" "strconv" "strings" @@ -305,46 +304,35 @@ func (z *ZipPluginDecoder) ExtractTo(dst string) error { } if err := func() error { + if err := os.MkdirAll(dst, 0755); err != nil { + return err + } + + root, err := os.OpenRoot(dst) + if err != nil { + return err + } + defer root.Close() + for _, file := range z.reader.File { - targetPath, err := safeExtractPath(dst, file.Name) + entryPath, err := safeEntryPath(file.Name) if err != nil { return err } if file.FileInfo().IsDir() { - if err := os.MkdirAll(targetPath, 0755); err != nil { + if err := root.MkdirAll(entryPath, 0755); err != nil { return err } continue } - workingPath := filepath.Dir(targetPath) // check if directory exists - if err := os.MkdirAll(workingPath, 0755); err != nil { - return err - } - - reader, err := file.Open() - if err != nil { - return err - } - - writer, err := os.OpenFile(targetPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0644) - if err != nil { - reader.Close() + if err := root.MkdirAll(path.Dir(entryPath), 0755); err != nil { return err } - if err := copyZipFile(writer, reader, file.UncompressedSize64); err != nil { - reader.Close() - writer.Close() - return err - } - if err := reader.Close(); err != nil { - writer.Close() - return err - } - if err := writer.Close(); err != nil { + if err := extractZipFile(root, entryPath, file); err != nil { return err } } @@ -359,24 +347,39 @@ func (z *ZipPluginDecoder) ExtractTo(dst string) error { return nil } -func safeExtractPath(dst, entryName string) (string, error) { +func safeEntryPath(entryName string) (string, error) { + if entryName == "" || strings.Contains(entryName, `\`) { + return "", fmt.Errorf("%w: %q", errUnsafeZipPath, entryName) + } + + for _, part := range strings.Split(entryName, "/") { + if part == ".." { + return "", fmt.Errorf("%w: %q", errUnsafeZipPath, entryName) + } + } + entryPath := path.Clean(entryName) - if entryPath == "." || - entryPath == ".." || - path.IsAbs(entryPath) || - strings.HasPrefix(entryPath, "../") || - strings.Contains(entryPath, "/../") || - strings.Contains(entryPath, `\`) || - filepath.IsAbs(filepath.FromSlash(entryPath)) { - return "", fmt.Errorf("%w: %s", errUnsafeZipPath, entryName) + if entryPath == "." || path.IsAbs(entryPath) { + return "", fmt.Errorf("%w: %q", errUnsafeZipPath, entryName) + } + + return entryPath, nil +} + +func extractZipFile(root *os.Root, entryPath string, file *zip.File) error { + reader, err := file.Open() + if err != nil { + return err } + defer reader.Close() - targetPath := filepath.Join(dst, filepath.FromSlash(entryPath)) - if !pathIsInside(dst, targetPath) { - return "", fmt.Errorf("%w: %s", errUnsafeZipPath, entryName) + writer, err := root.OpenFile(entryPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0644) + if err != nil { + return err } + defer writer.Close() - return targetPath, nil + return copyZipFile(writer, reader, file.UncompressedSize64) } func copyZipFile(writer io.Writer, reader io.Reader, uncompressedSize uint64) error { @@ -395,22 +398,6 @@ func copyZipFile(writer io.Writer, reader io.Reader, uncompressedSize uint64) er return nil } -func pathIsInside(root, target string) bool { - rootAbs, err := filepath.Abs(root) - if err != nil { - return false - } - targetAbs, err := filepath.Abs(target) - if err != nil { - return false - } - rel, err := filepath.Rel(rootAbs, targetAbs) - if err != nil { - return false - } - return rel == "." || (rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator))) -} - func (z *ZipPluginDecoder) CheckAssetsValid() error { return z.PluginDecoderHelper.CheckAssetsValid(z) } diff --git a/pkg/plugin_packager/decoder/zip_extract_test.go b/pkg/plugin_packager/decoder/zip_extract_test.go index 4d1e151f4..770e4ca0b 100644 --- a/pkg/plugin_packager/decoder/zip_extract_test.go +++ b/pkg/plugin_packager/decoder/zip_extract_test.go @@ -100,8 +100,8 @@ func TestZipPluginDecoderExtractToAllowsNestedPath(t *testing.T) { assert.Equal(t, []byte("ok"), extracted) } -func TestSafeExtractPathRejectsParentDirectoryEntry(t *testing.T) { - _, err := safeExtractPath(t.TempDir(), "..") +func TestSafeEntryPathRejectsParentDirectoryEntry(t *testing.T) { + _, err := safeEntryPath("..") require.Error(t, err) assert.True(t, errors.Is(err, errUnsafeZipPath))