Merge pull request #5420 from ollama/mxyng/insecure-path

err on insecure path

server/model.go

@@ -11,7 +11,6 @@ import (
 	"net/http"
 	"os"
 	"path/filepath"
-	"strings"
 
 	"github.com/ollama/ollama/api"
 	"github.com/ollama/ollama/convert"
@@ -91,12 +90,11 @@ func extractFromZipFile(p string, file *os.File, fn func(api.ProgressResponse))
 
 	fn(api.ProgressResponse{Status: "unpacking model metadata"})
 	for _, f := range r.File {
-		n := filepath.Join(p, f.Name)
-		if !strings.HasPrefix(n, p) {
-			slog.Warn("skipped extracting file outside of context", "name", f.Name)
-			continue
+		if !filepath.IsLocal(f.Name) {
+			return fmt.Errorf("%w: %s", zip.ErrInsecurePath, f.Name)
 		}
 
+		n := filepath.Join(p, f.Name)
 		if err := os.MkdirAll(filepath.Dir(n), 0o750); err != nil {
 			return err
 		}

server/model_test.go

@@ -3,10 +3,12 @@ package server
 import (
 	"archive/zip"
 	"bytes"
+	"errors"
 	"io"
 	"os"
 	"path/filepath"
 	"slices"
+	"strings"
 	"testing"
 
 	"github.com/ollama/ollama/api"
@@ -39,13 +41,31 @@ func TestExtractFromZipFile(t *testing.T) {
 	cases := []struct {
 		name   string
 		expect []string
+		err    error
 	}{
 		{
 			name:   "good",
 			expect: []string{"good"},
 		},
 		{
-			name: filepath.Join("..", "..", "..", "..", "..", "..", "..", "..", "..", "..", "..", "..", "..", "..", "..", "..", "bad"),
+			name:   strings.Join([]string{"path", "..", "to", "good"}, string(os.PathSeparator)),
+			expect: []string{filepath.Join("to", "good")},
+		},
+		{
+			name:   strings.Join([]string{"path", "..", "to", "..", "good"}, string(os.PathSeparator)),
+			expect: []string{"good"},
+		},
+		{
+			name:   strings.Join([]string{"path", "to", "..", "..", "good"}, string(os.PathSeparator)),
+			expect: []string{"good"},
+		},
+		{
+			name: strings.Join([]string{"..", "..", "..", "..", "..", "..", "..", "..", "..", "..", "..", "..", "..", "..", "..", "..", "bad"}, string(os.PathSeparator)),
+			err:  zip.ErrInsecurePath,
+		},
+		{
+			name: strings.Join([]string{"path", "..", "..", "to", "bad"}, string(os.PathSeparator)),
+			err:  zip.ErrInsecurePath,
 		},
 	}
 
@@ -55,7 +75,7 @@ func TestExtractFromZipFile(t *testing.T) {
 			defer f.Close()
 
 			tempDir := t.TempDir()
-			if err := extractFromZipFile(tempDir, f, func(api.ProgressResponse) {}); err != nil {
+			if err := extractFromZipFile(tempDir, f, func(api.ProgressResponse) {}); !errors.Is(err, tt.err) {
 				t.Fatal(err)
 			}