Merge pull request #3542 from jandubois/template-symlink

Don't use SecureJoin(templateDir, templateName)

Author: AkihiroSuda

cmd/limactl/start.go

@@ -7,6 +7,7 @@ import (
 	"errors"
 	"fmt"
 	"os"
+	"path"
 	"path/filepath"
 	"runtime"
 	"strings"
@@ -118,8 +119,7 @@ func loadOrCreateInstance(cmd *cobra.Command, args []string, createOnly bool) (*
 		return nil, err
 	}
 	if isTemplateURL, templateURL := limatmpl.SeemsTemplateURL(arg); isTemplateURL {
-		// No need to use SecureJoin here. https://github.com/lima-vm/lima/pull/805#discussion_r853411702
-		templateName := filepath.Join(templateURL.Host, templateURL.Path)
+		templateName := path.Join(templateURL.Host, templateURL.Path)
 		switch templateName {
 		case "experimental/vz":
 			logrus.Warn("template://experimental/vz was merged into the default template in Lima v1.0. See also <https://lima-vm.io/docs/config/vmtype/>.")

go.mod

@@ -16,7 +16,6 @@ require (
 	github.com/containers/gvisor-tap-vsock v0.8.6 // gomodjail:unconfined
 	github.com/coreos/go-semver v0.3.1
 	github.com/cpuguy83/go-md2man/v2 v2.0.7
-	github.com/cyphar/filepath-securejoin v0.4.1
 	github.com/digitalocean/go-qemu v0.0.0-20221209210016-f035778c97f7
 	github.com/diskfs/go-diskfs v1.6.0 // gomodjail:unconfined
 	github.com/docker/go-units v0.5.0

go.sum

@@ -51,8 +51,6 @@ github.com/cpuguy83/go-md2man/v2 v2.0.7 h1:zbFlGlXEAKlwXpmvle3d8Oe3YnkKIK4xSRTd3
 github.com/cpuguy83/go-md2man/v2 v2.0.7/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g=
 github.com/creack/pty v1.1.17 h1:QeVUsEDNrLBW4tMgZHvxy18sKtr6VI492kBhUfhDJNI=
 github.com/creack/pty v1.1.17/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr4O4=
-github.com/cyphar/filepath-securejoin v0.4.1 h1:JyxxyPEaktOD+GAnqIqTf9A8tHyAG22rowi7HkoSU1s=
-github.com/cyphar/filepath-securejoin v0.4.1/go.mod h1:Sdj7gXlvMcPZsbhwhQ33GguGLDGQL7h7bg04C/+u9jI=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM=

pkg/limatmpl/locator.go

@@ -40,8 +40,7 @@ func Read(ctx context.Context, name, locator string) (*Template, error) {
 	isTemplateURL, templateURL := SeemsTemplateURL(locator)
 	switch {
 	case isTemplateURL:
-		// No need to use SecureJoin here. https://github.com/lima-vm/lima/pull/805#discussion_r853411702
-		templateName := filepath.Join(templateURL.Host, templateURL.Path)
+		templateName := path.Join(templateURL.Host, templateURL.Path)
 		logrus.Debugf("interpreting argument %q as a template name %q", locator, templateName)
 		if tmpl.Name == "" {
 			// e.g., templateName = "deprecated/centos-7.yaml" , tmpl.Name = "centos-7"

pkg/templatestore/templatestore.go

@@ -14,7 +14,6 @@ import (
 	"strings"
 	"unicode"
 
-	securejoin "github.com/cyphar/filepath-securejoin"
 	"github.com/lima-vm/lima/pkg/store/dirnames"
 	"github.com/lima-vm/lima/pkg/usrlocalsharelima"
 )
@@ -42,7 +41,16 @@ func templatesPaths() ([]string, error) {
 	}, nil
 }
 
+// Read searches for template `name` in all template directories and returns the
+// contents of the first one found. Template names cannot contain the substring ".."
+// to make sure they don't reference files outside the template directories. We are
+// not using securejoin.SecureJoin because the actual template may be a symlink to a
+// directory elsewhere (e.g. when installed by Homebrew).
 func Read(name string) ([]byte, error) {
+	doubleDot := ".."
+	if strings.Contains(name, doubleDot) {
+		return nil, fmt.Errorf("template name %q must not contain %q", name, doubleDot)
+	}
 	paths, err := templatesPaths()
 	if err != nil {
 		return nil, err
@@ -54,10 +62,8 @@ func Read(name string) ([]byte, error) {
 		name += ".yaml"
 	}
 	for _, templatesDir := range paths {
-		filePath, err := securejoin.SecureJoin(templatesDir, name)
-		if err != nil {
-			return nil, err
-		}
+		// Normalize filePath for error messages because template names always use forward slashes
+		filePath := filepath.Clean(filepath.Join(templatesDir, name))
 		if b, err := os.ReadFile(filePath); !errors.Is(err, os.ErrNotExist) {
 			return b, err
 		}
@@ -67,6 +73,10 @@ func Read(name string) ([]byte, error) {
 
 const Default = "default"
 
+// Templates returns a list of Template structures containing the Name and Location for each template.
+// It searches all template directories, but only the first template of a given name is recorded.
+// Only non-hidden files with a ".yaml" file extension are considered templates.
+// The final result is sorted alphabetically by template name.
 func Templates() ([]Template, error) {
 	paths, err := templatesPaths()
 	if err != nil {