diff --git a/api/cmd/portainer/main.go b/api/cmd/portainer/main.go index 47acddedb..2886468f8 100644 --- a/api/cmd/portainer/main.go +++ b/api/cmd/portainer/main.go @@ -506,7 +506,7 @@ func buildServer(flags *portainer.CLIFlags) portainer.Server { adminPasswordHash := "" if *flags.AdminPasswordFile != "" { - content, err := fileService.GetFileContent(*flags.AdminPasswordFile) + content, err := fileService.GetFileContent(*flags.AdminPasswordFile, "") if err != nil { log.Fatalf("failed getting admin password file: %v", err) } diff --git a/api/exec/compose_stack.go b/api/exec/compose_stack.go index 3bdb43430..4c9da2f72 100644 --- a/api/exec/compose_stack.go +++ b/api/exec/compose_stack.go @@ -5,7 +5,6 @@ import ( "fmt" "os" "path" - "path/filepath" "strings" "github.com/pkg/errors" @@ -16,6 +15,7 @@ import ( portainer "github.com/portainer/portainer/api" "github.com/portainer/portainer/api/http/proxy" "github.com/portainer/portainer/api/http/proxy/factory" + "github.com/portainer/portainer/api/internal/stackutils" ) // ComposeStackManager is a wrapper for docker-compose binary @@ -58,7 +58,7 @@ func (manager *ComposeStackManager) Up(ctx context.Context, stack *portainer.Sta return errors.Wrap(err, "failed to create env file") } - filePaths := getStackFiles(stack) + filePaths := stackutils.GetStackFilePaths(stack) err = manager.deployer.Deploy(ctx, stack.ProjectPath, url, stack.Name, filePaths, envFilePath) return errors.Wrap(err, "failed to deploy a stack") } @@ -73,7 +73,7 @@ func (manager *ComposeStackManager) Down(ctx context.Context, stack *portainer.S defer proxy.Close() } - filePaths := getStackFiles(stack) + filePaths := stackutils.GetStackFilePaths(stack) err = manager.deployer.Remove(ctx, stack.ProjectPath, url, stack.Name, filePaths) return errors.Wrap(err, "failed to remove a stack") } @@ -115,27 +115,3 @@ func createEnvFile(stack *portainer.Stack) (string, error) { return "stack.env", nil } - -// getStackFiles returns list of stack's confile file paths. -// items in the list would be sanitized according to following criterias: -// 1. no empty paths -// 2. no "../xxx" paths that are trying to escape stack folder -// 3. no dir paths -// 4. root paths would be made relative -func getStackFiles(stack *portainer.Stack) []string { - paths := make([]string, 0, len(stack.AdditionalFiles)+1) - - for _, p := range append([]string{stack.EntryPoint}, stack.AdditionalFiles...) { - if strings.HasPrefix(p, "/") { - p = `.` + p - } - - if p == `` || p == `.` || strings.HasPrefix(p, `..`) || strings.HasSuffix(p, string(filepath.Separator)) { - continue - } - - paths = append(paths, p) - } - - return paths -} diff --git a/api/exec/compose_stack_test.go b/api/exec/compose_stack_test.go index 0b5dec2a3..c61285ebd 100644 --- a/api/exec/compose_stack_test.go +++ b/api/exec/compose_stack_test.go @@ -64,21 +64,3 @@ func Test_createEnvFile(t *testing.T) { }) } } - -func Test_getStackFiles(t *testing.T) { - stack := &portainer.Stack{ - EntryPoint: "./file", // picks entry point - AdditionalFiles: []string{ - ``, // ignores empty string - `.`, // ignores . - `..`, // ignores .. - `./dir/`, // ignrores paths that end with trailing / - `/with-root-prefix`, // replaces "root" based paths with relative - `./relative`, // keeps relative paths - `../escape`, // prevents dir escape - }, - } - - filePaths := getStackFiles(stack) - assert.ElementsMatch(t, filePaths, []string{`./file`, `./with-root-prefix`, `./relative`}) -} diff --git a/api/exec/swarm_stack.go b/api/exec/swarm_stack.go index 21a34a722..6872d2555 100644 --- a/api/exec/swarm_stack.go +++ b/api/exec/swarm_stack.go @@ -191,7 +191,7 @@ func (manager *SwarmStackManager) updateDockerCLIConfiguration(configPath string func (manager *SwarmStackManager) retrieveConfigurationFromDisk(path string) (map[string]interface{}, error) { var config map[string]interface{} - raw, err := manager.fileService.GetFileContent(path) + raw, err := manager.fileService.GetFileContent(path, "") if err != nil { return make(map[string]interface{}), nil } diff --git a/api/filesystem/filesystem.go b/api/filesystem/filesystem.go index 3f68bcf53..b76d337f3 100644 --- a/api/filesystem/filesystem.go +++ b/api/filesystem/filesystem.go @@ -6,14 +6,14 @@ import ( "encoding/pem" "errors" "fmt" - "io/ioutil" + "path/filepath" + "strings" "github.com/gofrs/uuid" portainer "github.com/portainer/portainer/api" "io" "os" - "path" ) const ( @@ -69,12 +69,31 @@ type Service struct { fileStorePath string } +// JoinPaths takes a trusted root path and a list of untrusted paths and joins +// them together using directory separators while enforcing that the untrusted +// paths cannot go higher up than the trusted root +func JoinPaths(trustedRoot string, untrustedPaths ...string) string { + if trustedRoot == "" { + trustedRoot = "." + } + + p := filepath.Join(trustedRoot, filepath.Join(append([]string{"/"}, untrustedPaths...)...)) + + // avoid setting a volume name from the untrusted paths + vnp := filepath.VolumeName(p) + if filepath.VolumeName(trustedRoot) == "" && vnp != "" { + return strings.TrimPrefix(strings.TrimPrefix(p, vnp), `\`) + } + + return p +} + // NewService initializes a new service. It creates a data directory and a directory to store files // inside this directory if they don't exist. func NewService(dataStorePath, fileStorePath string) (*Service, error) { service := &Service{ dataStorePath: dataStorePath, - fileStorePath: path.Join(dataStorePath, fileStorePath), + fileStorePath: JoinPaths(dataStorePath, fileStorePath), } err := os.MkdirAll(dataStorePath, 0755) @@ -112,12 +131,12 @@ func NewService(dataStorePath, fileStorePath string) (*Service, error) { // GetBinaryFolder returns the full path to the binary store on the filesystem func (service *Service) GetBinaryFolder() string { - return path.Join(service.fileStorePath, BinaryStorePath) + return JoinPaths(service.fileStorePath, BinaryStorePath) } // GetDockerConfigPath returns the full path to the docker config store on the filesystem func (service *Service) GetDockerConfigPath() string { - return path.Join(service.fileStorePath, DockerConfigPath) + return JoinPaths(service.fileStorePath, DockerConfigPath) } // RemoveDirectory removes a directory on the filesystem. @@ -128,7 +147,7 @@ func (service *Service) RemoveDirectory(directoryPath string) error { // GetStackProjectPath returns the absolute path on the FS for a stack based // on its identifier. func (service *Service) GetStackProjectPath(stackIdentifier string) string { - return path.Join(service.fileStorePath, ComposeStorePath, stackIdentifier) + return JoinPaths(service.wrapFileStore(ComposeStorePath), stackIdentifier) } // Copy copies the file on fromFilePath to toFilePath @@ -194,13 +213,13 @@ func (service *Service) Copy(fromFilePath string, toFilePath string, deleteIfExi // StoreStackFileFromBytes creates a subfolder in the ComposeStorePath and stores a new file from bytes. // It returns the path to the folder where the file is stored. func (service *Service) StoreStackFileFromBytes(stackIdentifier, fileName string, data []byte) (string, error) { - stackStorePath := path.Join(ComposeStorePath, stackIdentifier) + stackStorePath := JoinPaths(ComposeStorePath, stackIdentifier) err := service.createDirectoryInStore(stackStorePath) if err != nil { return "", err } - composeFilePath := path.Join(stackStorePath, fileName) + composeFilePath := JoinPaths(stackStorePath, fileName) r := bytes.NewReader(data) err = service.createFileInStore(composeFilePath, r) @@ -208,25 +227,25 @@ func (service *Service) StoreStackFileFromBytes(stackIdentifier, fileName string return "", err } - return path.Join(service.fileStorePath, stackStorePath), nil + return service.wrapFileStore(stackStorePath), nil } // GetEdgeStackProjectPath returns the absolute path on the FS for a edge stack based // on its identifier. func (service *Service) GetEdgeStackProjectPath(edgeStackIdentifier string) string { - return path.Join(service.fileStorePath, EdgeStackStorePath, edgeStackIdentifier) + return JoinPaths(service.wrapFileStore(EdgeStackStorePath), edgeStackIdentifier) } // StoreEdgeStackFileFromBytes creates a subfolder in the EdgeStackStorePath and stores a new file from bytes. // It returns the path to the folder where the file is stored. func (service *Service) StoreEdgeStackFileFromBytes(edgeStackIdentifier, fileName string, data []byte) (string, error) { - stackStorePath := path.Join(EdgeStackStorePath, edgeStackIdentifier) + stackStorePath := JoinPaths(EdgeStackStorePath, edgeStackIdentifier) err := service.createDirectoryInStore(stackStorePath) if err != nil { return "", err } - composeFilePath := path.Join(stackStorePath, fileName) + composeFilePath := JoinPaths(stackStorePath, fileName) r := bytes.NewReader(data) err = service.createFileInStore(composeFilePath, r) @@ -234,20 +253,20 @@ func (service *Service) StoreEdgeStackFileFromBytes(edgeStackIdentifier, fileNam return "", err } - return path.Join(service.fileStorePath, stackStorePath), nil + return service.wrapFileStore(stackStorePath), nil } // StoreRegistryManagementFileFromBytes creates a subfolder in the // ExtensionRegistryManagementStorePath and stores a new file from bytes. // It returns the path to the folder where the file is stored. func (service *Service) StoreRegistryManagementFileFromBytes(folder, fileName string, data []byte) (string, error) { - extensionStorePath := path.Join(ExtensionRegistryManagementStorePath, folder) + extensionStorePath := JoinPaths(ExtensionRegistryManagementStorePath, folder) err := service.createDirectoryInStore(extensionStorePath) if err != nil { return "", err } - file := path.Join(extensionStorePath, fileName) + file := JoinPaths(extensionStorePath, fileName) r := bytes.NewReader(data) err = service.createFileInStore(file, r) @@ -255,13 +274,13 @@ func (service *Service) StoreRegistryManagementFileFromBytes(folder, fileName st return "", err } - return path.Join(service.fileStorePath, file), nil + return service.wrapFileStore(file), nil } // StoreTLSFileFromBytes creates a folder in the TLSStorePath and stores a new file from bytes. // It returns the path to the newly created file. func (service *Service) StoreTLSFileFromBytes(folder string, fileType portainer.TLSFileType, data []byte) (string, error) { - storePath := path.Join(TLSStorePath, folder) + storePath := JoinPaths(TLSStorePath, folder) err := service.createDirectoryInStore(storePath) if err != nil { return "", err @@ -279,13 +298,13 @@ func (service *Service) StoreTLSFileFromBytes(folder string, fileType portainer. return "", ErrUndefinedTLSFileType } - tlsFilePath := path.Join(storePath, fileName) + tlsFilePath := JoinPaths(storePath, fileName) r := bytes.NewReader(data) err = service.createFileInStore(tlsFilePath, r) if err != nil { return "", err } - return path.Join(service.fileStorePath, tlsFilePath), nil + return service.wrapFileStore(tlsFilePath), nil } // GetPathForTLSFile returns the absolute path to a specific TLS file for an environment(endpoint). @@ -301,17 +320,13 @@ func (service *Service) GetPathForTLSFile(folder string, fileType portainer.TLSF default: return "", ErrUndefinedTLSFileType } - return path.Join(service.fileStorePath, TLSStorePath, folder, fileName), nil + return JoinPaths(service.wrapFileStore(TLSStorePath), folder, fileName), nil } // DeleteTLSFiles deletes a folder in the TLS store path. func (service *Service) DeleteTLSFiles(folder string) error { - storePath := path.Join(service.fileStorePath, TLSStorePath, folder) - err := os.RemoveAll(storePath) - if err != nil { - return err - } - return nil + storePath := JoinPaths(service.wrapFileStore(TLSStorePath), folder) + return os.RemoveAll(storePath) } // DeleteTLSFile deletes a specific TLS file from a folder. @@ -328,20 +343,19 @@ func (service *Service) DeleteTLSFile(folder string, fileType portainer.TLSFileT return ErrUndefinedTLSFileType } - filePath := path.Join(service.fileStorePath, TLSStorePath, folder, fileName) + filePath := JoinPaths(service.wrapFileStore(TLSStorePath), folder, fileName) - err := os.Remove(filePath) - if err != nil { - return err - } - return nil + return os.Remove(filePath) } // GetFileContent returns the content of a file as bytes. -func (service *Service) GetFileContent(filePath string) ([]byte, error) { - content, err := ioutil.ReadFile(filePath) +func (service *Service) GetFileContent(trustedRoot, filePath string) ([]byte, error) { + content, err := os.ReadFile(JoinPaths(trustedRoot, filePath)) if err != nil { - return nil, err + if filePath == "" { + filePath = trustedRoot + } + return nil, fmt.Errorf("could not get the contents of the file '%s'", filePath) } return content, nil @@ -359,7 +373,7 @@ func (service *Service) WriteJSONToFile(path string, content interface{}) error return err } - return ioutil.WriteFile(path, jsonContent, 0644) + return os.WriteFile(path, jsonContent, 0644) } // FileExists checks for the existence of the specified file. @@ -369,23 +383,17 @@ func (service *Service) FileExists(filePath string) (bool, error) { // KeyPairFilesExist checks for the existence of the key files. func (service *Service) KeyPairFilesExist() (bool, error) { - privateKeyPath := path.Join(service.dataStorePath, PrivateKeyFile) + privateKeyPath := JoinPaths(service.dataStorePath, PrivateKeyFile) exists, err := service.FileExists(privateKeyPath) - if err != nil { + if err != nil || !exists { return false, err } - if !exists { - return false, nil - } - publicKeyPath := path.Join(service.dataStorePath, PublicKeyFile) + publicKeyPath := JoinPaths(service.dataStorePath, PublicKeyFile) exists, err = service.FileExists(publicKeyPath) - if err != nil { + if err != nil || !exists { return false, err } - if !exists { - return false, nil - } return true, nil } @@ -397,12 +405,7 @@ func (service *Service) StoreKeyPair(private, public []byte, privatePEMHeader, p return err } - err = service.createPEMFileInStore(public, publicPEMHeader, PublicKeyFile) - if err != nil { - return err - } - - return nil + return service.createPEMFileInStore(public, publicPEMHeader, PublicKeyFile) } // LoadKeyPair retrieve the content of both key files on disk. @@ -422,13 +425,13 @@ func (service *Service) LoadKeyPair() ([]byte, []byte, error) { // createDirectoryInStore creates a new directory in the file store func (service *Service) createDirectoryInStore(name string) error { - path := path.Join(service.fileStorePath, name) + path := service.wrapFileStore(name) return os.MkdirAll(path, 0700) } // createFile creates a new file in the file store with the content from r. func (service *Service) createFileInStore(filePath string, r io.Reader) error { - path := path.Join(service.fileStorePath, filePath) + path := service.wrapFileStore(filePath) out, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600) if err != nil { @@ -437,15 +440,11 @@ func (service *Service) createFileInStore(filePath string, r io.Reader) error { defer out.Close() _, err = io.Copy(out, r) - if err != nil { - return err - } - - return nil + return err } func (service *Service) createPEMFileInStore(content []byte, fileType, filePath string) error { - path := path.Join(service.fileStorePath, filePath) + path := service.wrapFileStore(filePath) block := &pem.Block{Type: fileType, Bytes: content} out, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600) @@ -454,18 +453,13 @@ func (service *Service) createPEMFileInStore(content []byte, fileType, filePath } defer out.Close() - err = pem.Encode(out, block) - if err != nil { - return err - } - - return nil + return pem.Encode(out, block) } func (service *Service) getContentFromPEMFile(filePath string) ([]byte, error) { - path := path.Join(service.fileStorePath, filePath) + path := service.wrapFileStore(filePath) - fileContent, err := ioutil.ReadFile(path) + fileContent, err := os.ReadFile(path) if err != nil { return nil, err } @@ -477,19 +471,19 @@ func (service *Service) getContentFromPEMFile(filePath string) ([]byte, error) { // GetCustomTemplateProjectPath returns the absolute path on the FS for a custom template based // on its identifier. func (service *Service) GetCustomTemplateProjectPath(identifier string) string { - return path.Join(service.fileStorePath, CustomTemplateStorePath, identifier) + return JoinPaths(service.wrapFileStore(CustomTemplateStorePath), identifier) } // StoreCustomTemplateFileFromBytes creates a subfolder in the CustomTemplateStorePath and stores a new file from bytes. // It returns the path to the folder where the file is stored. func (service *Service) StoreCustomTemplateFileFromBytes(identifier, fileName string, data []byte) (string, error) { - customTemplateStorePath := path.Join(CustomTemplateStorePath, identifier) + customTemplateStorePath := JoinPaths(CustomTemplateStorePath, identifier) err := service.createDirectoryInStore(customTemplateStorePath) if err != nil { return "", err } - templateFilePath := path.Join(customTemplateStorePath, fileName) + templateFilePath := JoinPaths(customTemplateStorePath, fileName) r := bytes.NewReader(data) err = service.createFileInStore(templateFilePath, r) @@ -497,32 +491,32 @@ func (service *Service) StoreCustomTemplateFileFromBytes(identifier, fileName st return "", err } - return path.Join(service.fileStorePath, customTemplateStorePath), nil + return service.wrapFileStore(customTemplateStorePath), nil } // GetEdgeJobFolder returns the absolute path on the filesystem for an Edge job based // on its identifier. func (service *Service) GetEdgeJobFolder(identifier string) string { - return path.Join(service.fileStorePath, EdgeJobStorePath, identifier) + return JoinPaths(service.wrapFileStore(EdgeJobStorePath), identifier) } // StoreEdgeJobFileFromBytes creates a subfolder in the EdgeJobStorePath and stores a new file from bytes. // It returns the path to the folder where the file is stored. func (service *Service) StoreEdgeJobFileFromBytes(identifier string, data []byte) (string, error) { - edgeJobStorePath := path.Join(EdgeJobStorePath, identifier) + edgeJobStorePath := JoinPaths(EdgeJobStorePath, identifier) err := service.createDirectoryInStore(edgeJobStorePath) if err != nil { return "", err } - filePath := path.Join(edgeJobStorePath, createEdgeJobFileName(identifier)) + filePath := JoinPaths(edgeJobStorePath, createEdgeJobFileName(identifier)) r := bytes.NewReader(data) err = service.createFileInStore(filePath, r) if err != nil { return "", err } - return path.Join(service.fileStorePath, filePath), nil + return service.wrapFileStore(filePath), nil } func createEdgeJobFileName(identifier string) string { @@ -532,20 +526,14 @@ func createEdgeJobFileName(identifier string) string { // ClearEdgeJobTaskLogs clears the Edge job task logs func (service *Service) ClearEdgeJobTaskLogs(edgeJobID string, taskID string) error { path := service.getEdgeJobTaskLogPath(edgeJobID, taskID) - - err := os.Remove(path) - if err != nil { - return err - } - - return nil + return os.Remove(path) } // GetEdgeJobTaskLogFileContent fetches the Edge job task logs func (service *Service) GetEdgeJobTaskLogFileContent(edgeJobID string, taskID string) (string, error) { path := service.getEdgeJobTaskLogPath(edgeJobID, taskID) - fileContent, err := ioutil.ReadFile(path) + fileContent, err := os.ReadFile(path) if err != nil { return "", err } @@ -555,20 +543,15 @@ func (service *Service) GetEdgeJobTaskLogFileContent(edgeJobID string, taskID st // StoreEdgeJobTaskLogFileFromBytes stores the log file func (service *Service) StoreEdgeJobTaskLogFileFromBytes(edgeJobID, taskID string, data []byte) error { - edgeJobStorePath := path.Join(EdgeJobStorePath, edgeJobID) + edgeJobStorePath := JoinPaths(EdgeJobStorePath, edgeJobID) err := service.createDirectoryInStore(edgeJobStorePath) if err != nil { return err } - filePath := path.Join(edgeJobStorePath, fmt.Sprintf("logs_%s", taskID)) + filePath := JoinPaths(edgeJobStorePath, fmt.Sprintf("logs_%s", taskID)) r := bytes.NewReader(data) - err = service.createFileInStore(filePath, r) - if err != nil { - return err - } - - return nil + return service.createFileInStore(filePath, r) } func (service *Service) getEdgeJobTaskLogPath(edgeJobID string, taskID string) string { @@ -582,7 +565,7 @@ func (service *Service) GetTemporaryPath() (string, error) { return "", err } - return path.Join(service.fileStorePath, TempPath, uid.String()), nil + return JoinPaths(service.wrapFileStore(TempPath), uid.String()), nil } // GetDataStorePath returns path to data folder @@ -591,12 +574,12 @@ func (service *Service) GetDatastorePath() string { } func (service *Service) wrapFileStore(filepath string) string { - return path.Join(service.fileStorePath, filepath) + return JoinPaths(service.fileStorePath, filepath) } func defaultCertPathUnderFileStore() (string, string) { - certPath := path.Join(SSLCertPath, DefaultSSLCertFilename) - keyPath := path.Join(SSLCertPath, DefaultSSLKeyFilename) + certPath := JoinPaths(SSLCertPath, DefaultSSLCertFilename) + keyPath := JoinPaths(SSLCertPath, DefaultSSLKeyFilename) return certPath, keyPath } diff --git a/api/filesystem/filesystem_linux_test.go b/api/filesystem/filesystem_linux_test.go new file mode 100644 index 000000000..dad55c951 --- /dev/null +++ b/api/filesystem/filesystem_linux_test.go @@ -0,0 +1,70 @@ +package filesystem + +import "testing" + +func TestJoinPaths(t *testing.T) { + var ts = []struct { + trusted string + untrusted string + expected string + }{ + {"", "", "."}, + {"", ".", "."}, + {"", "d/e/f", "d/e/f"}, + {"", "./d/e/f", "d/e/f"}, + {"", "../d/e/f", "d/e/f"}, + {"", "/d/e/f", "d/e/f"}, + {"", "../../../etc/shadow", "etc/shadow"}, + + {".", "", "."}, + {".", ".", "."}, + {".", "d/e/f", "d/e/f"}, + {".", "./d/e/f", "d/e/f"}, + {".", "../d/e/f", "d/e/f"}, + {".", "/d/e/f", "d/e/f"}, + {".", "../../../etc/shadow", "etc/shadow"}, + + {"./", "", "."}, + {"./", ".", "."}, + {"./", "d/e/f", "d/e/f"}, + {"./", "./d/e/f", "d/e/f"}, + {"./", "../d/e/f", "d/e/f"}, + {"./", "/d/e/f", "d/e/f"}, + {"./", "../../../etc/shadow", "etc/shadow"}, + + {"a/b/c", "", "a/b/c"}, + {"a/b/c", ".", "a/b/c"}, + {"a/b/c", "d/e/f", "a/b/c/d/e/f"}, + {"a/b/c", "./d/e/f", "a/b/c/d/e/f"}, + {"a/b/c", "../d/e/f", "a/b/c/d/e/f"}, + {"a/b/c", "../../../etc/shadow", "a/b/c/etc/shadow"}, + + {"/a/b/c", "", "/a/b/c"}, + {"/a/b/c", ".", "/a/b/c"}, + {"/a/b/c", "d/e/f", "/a/b/c/d/e/f"}, + {"/a/b/c", "./d/e/f", "/a/b/c/d/e/f"}, + {"/a/b/c", "../d/e/f", "/a/b/c/d/e/f"}, + {"/a/b/c", "../../../etc/shadow", "/a/b/c/etc/shadow"}, + + {"./a/b/c", "", "a/b/c"}, + {"./a/b/c", ".", "a/b/c"}, + {"./a/b/c", "d/e/f", "a/b/c/d/e/f"}, + {"./a/b/c", "./d/e/f", "a/b/c/d/e/f"}, + {"./a/b/c", "../d/e/f", "a/b/c/d/e/f"}, + {"./a/b/c", "../../../etc/shadow", "a/b/c/etc/shadow"}, + + {"../a/b/c", "", "../a/b/c"}, + {"../a/b/c", ".", "../a/b/c"}, + {"../a/b/c", "d/e/f", "../a/b/c/d/e/f"}, + {"../a/b/c", "./d/e/f", "../a/b/c/d/e/f"}, + {"../a/b/c", "../d/e/f", "../a/b/c/d/e/f"}, + {"../a/b/c", "../../../etc/shadow", "../a/b/c/etc/shadow"}, + } + + for _, c := range ts { + r := JoinPaths(c.trusted, c.untrusted) + if r != c.expected { + t.Fatalf("expected '%s', got '%s'. Inputs = '%s', '%s'", c.expected, r, c.trusted, c.untrusted) + } + } +} diff --git a/api/filesystem/filesystem_windows_test.go b/api/filesystem/filesystem_windows_test.go new file mode 100644 index 000000000..70187dd59 --- /dev/null +++ b/api/filesystem/filesystem_windows_test.go @@ -0,0 +1,120 @@ +package filesystem + +import "testing" + +func TestJoinPaths(t *testing.T) { + var ts = []struct { + trusted string + untrusted string + expected string + }{ + {"", "", "."}, + {"", ".", "."}, + {"", "d/e/f", `d\e\f`}, + {"", "./d/e/f", `d\e\f`}, + {"", "../d/e/f", `d\e\f`}, + {"", "/d/e/f", `d\e\f`}, + {"", "../../../windows/system.ini", `windows\system.ini`}, + {"", `C:\windows\system.ini`, `windows\system.ini`}, + {"", `..\..\..\..\C:\windows\system.ini`, `windows\system.ini`}, + {"", `\\server\a\b\c`, `server\a\b\c`}, + {"", `..\..\..\..\\server\a\b\c`, `server\a\b\c`}, + + {".", "", "."}, + {".", ".", "."}, + {".", "d/e/f", `d\e\f`}, + {".", "./d/e/f", `d\e\f`}, + {".", "../d/e/f", `d\e\f`}, + {".", "/d/e/f", `d\e\f`}, + {".", "../../../windows/system.ini", `windows\system.ini`}, + {".", `C:\windows\system.ini`, `windows\system.ini`}, + {".", `..\..\..\..\C:\windows\system.ini`, `windows\system.ini`}, + {".", `\\server\a\b\c`, `server\a\b\c`}, + {".", `..\..\..\..\\server\a\b\c`, `server\a\b\c`}, + + {"./", "", "."}, + {"./", ".", "."}, + {"./", "d/e/f", `d\e\f`}, + {"./", "./d/e/f", `d\e\f`}, + {"./", "../d/e/f", `d\e\f`}, + {"./", "/d/e/f", `d\e\f`}, + {"./", "../../../windows/system.ini", `windows\system.ini`}, + {"./", `C:\windows\system.ini`, `windows\system.ini`}, + {"./", `..\..\..\..\C:\windows\system.ini`, `windows\system.ini`}, + {"./", `\\server\a\b\c`, `server\a\b\c`}, + {"./", `..\..\..\..\\server\a\b\c`, `server\a\b\c`}, + + {"a/b/c", "", `a\b\c`}, + {"a/b/c", ".", `a\b\c`}, + {"a/b/c", "d/e/f", `a\b\c\d\e\f`}, + {"a/b/c", "./d/e/f", `a\b\c\d\e\f`}, + {"a/b/c", "../d/e/f", `a\b\c\d\e\f`}, + {"a/b/c", "../../../windows/system.ini", `a\b\c\windows\system.ini`}, + {"a/b/c", `C:\windows\system.ini`, `a\b\c\C:\windows\system.ini`}, + {"a/b/c", `..\..\..\..\C:\windows\system.ini`, `a\b\c\C:\windows\system.ini`}, + {"a/b/c", `\\server\a\b\c`, `a\b\c\server\a\b\c`}, + {"a/b/c", `..\..\..\..\\server\a\b\c`, `a\b\c\server\a\b\c`}, + + {"/a/b/c", "", `\a\b\c`}, + {"/a/b/c", ".", `\a\b\c`}, + {"/a/b/c", "d/e/f", `\a\b\c\d\e\f`}, + {"/a/b/c", "./d/e/f", `\a\b\c\d\e\f`}, + {"/a/b/c", "../d/e/f", `\a\b\c\d\e\f`}, + {"/a/b/c", "../../../windows/system.ini", `\a\b\c\windows\system.ini`}, + {"/a/b/c", `C:\windows\system.ini`, `\a\b\c\C:\windows\system.ini`}, + {"/a/b/c", `..\..\..\..\C:\windows\system.ini`, `\a\b\c\C:\windows\system.ini`}, + {"/a/b/c", `\\server\a\b\c`, `\a\b\c\server\a\b\c`}, + {"/a/b/c", `..\..\..\..\\server\a\b\c`, `\a\b\c\server\a\b\c`}, + + {"./a/b/c", "", `a\b\c`}, + {"./a/b/c", ".", `a\b\c`}, + {"./a/b/c", "d/e/f", `a\b\c\d\e\f`}, + {"./a/b/c", "./d/e/f", `a\b\c\d\e\f`}, + {"./a/b/c", "../d/e/f", `a\b\c\d\e\f`}, + {"./a/b/c", "../../../windows/system.ini", `a\b\c\windows\system.ini`}, + {"./a/b/c", `C:\windows\system.ini`, `a\b\c\C:\windows\system.ini`}, + {"./a/b/c", `..\..\..\..\C:\windows\system.ini`, `a\b\c\C:\windows\system.ini`}, + {"./a/b/c", `\\server\a\b\c`, `a\b\c\server\a\b\c`}, + {"./a/b/c", `..\..\..\..\\server\a\b\c`, `a\b\c\server\a\b\c`}, + + {"../a/b/c", "", `..\a\b\c`}, + {"../a/b/c", ".", `..\a\b\c`}, + {"../a/b/c", "d/e/f", `..\a\b\c\d\e\f`}, + {"../a/b/c", "./d/e/f", `..\a\b\c\d\e\f`}, + {"../a/b/c", "../d/e/f", `..\a\b\c\d\e\f`}, + {"../a/b/c", "../../../windows/system.ini", `..\a\b\c\windows\system.ini`}, + {"../a/b/c", `C:\windows\system.ini`, `..\a\b\c\C:\windows\system.ini`}, + {"../a/b/c", `..\..\..\..\C:\windows\system.ini`, `..\a\b\c\C:\windows\system.ini`}, + {"../a/b/c", `\\server\a\b\c`, `..\a\b\c\server\a\b\c`}, + {"../a/b/c", `..\..\..\..\\server\a\b\c`, `..\a\b\c\server\a\b\c`}, + + {"C:/a/b/c", "", `C:\a\b\c`}, + {"C:/a/b/c", ".", `C:\a\b\c`}, + {"C:/a/b/c", "d/e/f", `C:\a\b\c\d\e\f`}, + {"C:/a/b/c", "./d/e/f", `C:\a\b\c\d\e\f`}, + {"C:/a/b/c", "../d/e/f", `C:\a\b\c\d\e\f`}, + {"C:/a/b/c", "../../../windows/system.ini", `C:\a\b\c\windows\system.ini`}, + {"C:/a/b/c", `C:\windows\system.ini`, `C:\a\b\c\C:\windows\system.ini`}, + {"C:/a/b/c", `..\..\..\..\C:\windows\system.ini`, `C:\a\b\c\C:\windows\system.ini`}, + {"C:/a/b/c", `\\server\a\b\c`, `C:\a\b\c\server\a\b\c`}, + {"C:/a/b/c", `..\..\..\..\\server\a\b\c`, `C:\a\b\c\server\a\b\c`}, + + {`\\server\a\b\c`, "", `\\server\a\b\c`}, + {`\\server\a\b\c`, ".", `\\server\a\b\c`}, + {`\\server\a\b\c`, "d/e/f", `\\server\a\b\c\d\e\f`}, + {`\\server\a\b\c`, "./d/e/f", `\\server\a\b\c\d\e\f`}, + {`\\server\a\b\c`, "../d/e/f", `\\server\a\b\c\d\e\f`}, + {`\\server\a\b\c`, "../../../windows/system.ini", `\\server\a\b\c\windows\system.ini`}, + {`\\server\a\b\c`, `C:\windows\system.ini`, `\\server\a\b\c\C:\windows\system.ini`}, + {`\\server\a\b\c`, `..\..\..\C:\windows\system.ini`, `\\server\a\b\c\C:\windows\system.ini`}, + {`\\server\a\b\c`, `\\server\a\b\c`, `\\server\a\b\c\server\a\b\c`}, + {`\\server\a\b\c`, `..\..\..\\server\a\b\c`, `\\server\a\b\c\server\a\b\c`}, + } + + for _, c := range ts { + r := JoinPaths(c.trusted, c.untrusted) + if r != c.expected { + t.Fatalf("expected '%s', got '%s'. Inputs = '%s', '%s'", c.expected, r, c.trusted, c.untrusted) + } + } +} diff --git a/api/http/handler/customtemplates/customtemplate_file.go b/api/http/handler/customtemplates/customtemplate_file.go index e238dffb5..2d7d29a88 100644 --- a/api/http/handler/customtemplates/customtemplate_file.go +++ b/api/http/handler/customtemplates/customtemplate_file.go @@ -2,7 +2,6 @@ package customtemplates import ( "net/http" - "path" httperror "github.com/portainer/libhttp/error" "github.com/portainer/libhttp/request" @@ -41,7 +40,7 @@ func (handler *Handler) customTemplateFile(w http.ResponseWriter, r *http.Reques return &httperror.HandlerError{http.StatusInternalServerError, "Unable to find a custom template with the specified identifier inside the database", err} } - fileContent, err := handler.FileService.GetFileContent(path.Join(customTemplate.ProjectPath, customTemplate.EntryPoint)) + fileContent, err := handler.FileService.GetFileContent(customTemplate.ProjectPath, customTemplate.EntryPoint) if err != nil { return &httperror.HandlerError{http.StatusInternalServerError, "Unable to retrieve custom template file from disk", err} } diff --git a/api/http/handler/edgejobs/edgejob_file.go b/api/http/handler/edgejobs/edgejob_file.go index 751d56246..e38b9a448 100644 --- a/api/http/handler/edgejobs/edgejob_file.go +++ b/api/http/handler/edgejobs/edgejob_file.go @@ -39,7 +39,7 @@ func (handler *Handler) edgeJobFile(w http.ResponseWriter, r *http.Request) *htt return &httperror.HandlerError{http.StatusInternalServerError, "Unable to find an Edge job with the specified identifier inside the database", err} } - edgeJobFileContent, err := handler.FileService.GetFileContent(edgeJob.ScriptPath) + edgeJobFileContent, err := handler.FileService.GetFileContent("", edgeJob.ScriptPath) if err != nil { return &httperror.HandlerError{http.StatusInternalServerError, "Unable to retrieve Edge job script file from disk", err} } diff --git a/api/http/handler/edgestacks/edgestack_file.go b/api/http/handler/edgestacks/edgestack_file.go index 7ecafa883..4a791d9c0 100644 --- a/api/http/handler/edgestacks/edgestack_file.go +++ b/api/http/handler/edgestacks/edgestack_file.go @@ -2,7 +2,6 @@ package edgestacks import ( "net/http" - "path" httperror "github.com/portainer/libhttp/error" "github.com/portainer/libhttp/request" @@ -45,7 +44,7 @@ func (handler *Handler) edgeStackFile(w http.ResponseWriter, r *http.Request) *h fileName = stack.ManifestPath } - stackFileContent, err := handler.FileService.GetFileContent(path.Join(stack.ProjectPath, fileName)) + stackFileContent, err := handler.FileService.GetFileContent(stack.ProjectPath, fileName) if err != nil { return &httperror.HandlerError{http.StatusInternalServerError, "Unable to retrieve Compose file from disk", err} } diff --git a/api/http/handler/edgestacks/handler.go b/api/http/handler/edgestacks/handler.go index d9bf0e39e..72fb27098 100644 --- a/api/http/handler/edgestacks/handler.go +++ b/api/http/handler/edgestacks/handler.go @@ -3,7 +3,6 @@ package edgestacks import ( "fmt" "net/http" - "path" "strconv" "github.com/gorilla/mux" @@ -56,7 +55,7 @@ func (handler *Handler) convertAndStoreKubeManifestIfNeeded(edgeStack *portainer return nil } - composeConfig, err := handler.FileService.GetFileContent(path.Join(edgeStack.ProjectPath, edgeStack.EntryPoint)) + composeConfig, err := handler.FileService.GetFileContent(edgeStack.ProjectPath, edgeStack.EntryPoint) if err != nil { return fmt.Errorf("unable to retrieve Compose file from disk: %w", err) } diff --git a/api/http/handler/endpointedge/endpoint_edgestack_inspect.go b/api/http/handler/endpointedge/endpoint_edgestack_inspect.go index 7f89f128c..cc6cf4594 100644 --- a/api/http/handler/endpointedge/endpoint_edgestack_inspect.go +++ b/api/http/handler/endpointedge/endpoint_edgestack_inspect.go @@ -3,7 +3,6 @@ package endpointedge import ( "errors" "net/http" - "path" httperror "github.com/portainer/libhttp/error" "github.com/portainer/libhttp/request" @@ -75,7 +74,7 @@ func (handler *Handler) endpointEdgeStackInspect(w http.ResponseWriter, r *http. } } - stackFileContent, err := handler.FileService.GetFileContent(path.Join(edgeStack.ProjectPath, fileName)) + stackFileContent, err := handler.FileService.GetFileContent(edgeStack.ProjectPath, fileName) if err != nil { return &httperror.HandlerError{http.StatusInternalServerError, "Unable to retrieve Compose file from disk", err} } diff --git a/api/http/handler/endpoints/endpoint_status_inspect.go b/api/http/handler/endpoints/endpoint_status_inspect.go index 9aa9bc4aa..de32502d9 100644 --- a/api/http/handler/endpoints/endpoint_status_inspect.go +++ b/api/http/handler/endpoints/endpoint_status_inspect.go @@ -131,7 +131,7 @@ func (handler *Handler) endpointStatusInspect(w http.ResponseWriter, r *http.Req Version: job.Version, } - file, err := handler.FileService.GetFileContent(job.ScriptPath) + file, err := handler.FileService.GetFileContent("", job.ScriptPath) if err != nil { return &httperror.HandlerError{http.StatusInternalServerError, "Unable to retrieve Edge job script file", err} diff --git a/api/http/handler/stacks/create_compose_stack.go b/api/http/handler/stacks/create_compose_stack.go index 685fd64d8..4d8c43929 100644 --- a/api/http/handler/stacks/create_compose_stack.go +++ b/api/http/handler/stacks/create_compose_stack.go @@ -3,7 +3,6 @@ package stacks import ( "fmt" "net/http" - "path" "strconv" "time" @@ -389,10 +388,9 @@ func (handler *Handler) deployComposeStack(config *composeStackDeploymentConfig) !isAdminOrEndpointAdmin { for _, file := range append([]string{config.stack.EntryPoint}, config.stack.AdditionalFiles...) { - path := path.Join(config.stack.ProjectPath, file) - stackContent, err := handler.FileService.GetFileContent(path) + stackContent, err := handler.FileService.GetFileContent(config.stack.ProjectPath, file) if err != nil { - return errors.Wrapf(err, "failed to get stack file content `%q`", path) + return errors.Wrapf(err, "failed to get stack file content `%q`", file) } err = handler.isValidStackFile(stackContent, securitySettings) diff --git a/api/http/handler/stacks/create_swarm_stack.go b/api/http/handler/stacks/create_swarm_stack.go index 66f8e230f..6e9b8d3c0 100644 --- a/api/http/handler/stacks/create_swarm_stack.go +++ b/api/http/handler/stacks/create_swarm_stack.go @@ -3,7 +3,6 @@ package stacks import ( "fmt" "net/http" - "path" "strconv" "time" @@ -399,8 +398,7 @@ func (handler *Handler) deploySwarmStack(config *swarmStackDeploymentConfig) err if !settings.AllowBindMountsForRegularUsers && !isAdminOrEndpointAdmin { for _, file := range append([]string{config.stack.EntryPoint}, config.stack.AdditionalFiles...) { - path := path.Join(config.stack.ProjectPath, file) - stackContent, err := handler.FileService.GetFileContent(path) + stackContent, err := handler.FileService.GetFileContent(config.stack.ProjectPath, file) if err != nil { return errors.WithMessage(err, "failed to get stack file content") } diff --git a/api/http/handler/stacks/stack_delete.go b/api/http/handler/stacks/stack_delete.go index 799f6f612..a87bdf847 100644 --- a/api/http/handler/stacks/stack_delete.go +++ b/api/http/handler/stacks/stack_delete.go @@ -6,7 +6,6 @@ import ( "io/ioutil" "net/http" "os" - "path" "strconv" "github.com/pkg/errors" @@ -198,8 +197,8 @@ func (handler *Handler) deleteStack(userID portainer.UserID, stack *portainer.St defer os.RemoveAll(tmpDir) for _, fileName := range fileNames { - manifestFilePath := path.Join(tmpDir, fileName) - manifestContent, err := ioutil.ReadFile(path.Join(stack.ProjectPath, fileName)) + manifestFilePath := filesystem.JoinPaths(tmpDir, fileName) + manifestContent, err := handler.FileService.GetFileContent(stack.ProjectPath, fileName) if err != nil { return errors.Wrap(err, "failed to read manifest file") } diff --git a/api/http/handler/stacks/stack_file.go b/api/http/handler/stacks/stack_file.go index e5c5056d3..d0f313505 100644 --- a/api/http/handler/stacks/stack_file.go +++ b/api/http/handler/stacks/stack_file.go @@ -2,7 +2,6 @@ package stacks import ( "net/http" - "path" httperror "github.com/portainer/libhttp/error" "github.com/portainer/libhttp/request" @@ -82,7 +81,7 @@ func (handler *Handler) stackFile(w http.ResponseWriter, r *http.Request) *httpe } } - stackFileContent, err := handler.FileService.GetFileContent(path.Join(stack.ProjectPath, stack.EntryPoint)) + stackFileContent, err := handler.FileService.GetFileContent(stack.ProjectPath, stack.EntryPoint) if err != nil { return &httperror.HandlerError{http.StatusInternalServerError, "Unable to retrieve Compose file from disk", err} } diff --git a/api/http/handler/stacks/update_kubernetes_stack.go b/api/http/handler/stacks/update_kubernetes_stack.go index 9ce74cb6c..4eda081e4 100644 --- a/api/http/handler/stacks/update_kubernetes_stack.go +++ b/api/http/handler/stacks/update_kubernetes_stack.go @@ -5,7 +5,6 @@ import ( "io/ioutil" "net/http" "os" - "path" "strconv" "github.com/asaskevich/govalidator" @@ -108,7 +107,7 @@ func (handler *Handler) updateKubernetesStack(r *http.Request, stack *portainer. tempFileDir, _ := ioutil.TempDir("", "kub_file_content") defer os.RemoveAll(tempFileDir) - if err := filesystem.WriteToFile(path.Join(tempFileDir, stack.EntryPoint), []byte(payload.StackFileContent)); err != nil { + if err := filesystem.WriteToFile(filesystem.JoinPaths(tempFileDir, stack.EntryPoint), []byte(payload.StackFileContent)); err != nil { return &httperror.HandlerError{StatusCode: http.StatusInternalServerError, Message: "Failed to persist deployment file in a temp directory", Err: err} } diff --git a/api/http/handler/templates/template_file.go b/api/http/handler/templates/template_file.go index 844e99e16..e5b5c3d4a 100644 --- a/api/http/handler/templates/template_file.go +++ b/api/http/handler/templates/template_file.go @@ -4,7 +4,6 @@ import ( "errors" "log" "net/http" - "path" "github.com/asaskevich/govalidator" httperror "github.com/portainer/libhttp/error" @@ -68,9 +67,7 @@ func (handler *Handler) templateFile(w http.ResponseWriter, r *http.Request) *ht return &httperror.HandlerError{http.StatusInternalServerError, "Unable to clone git repository", err} } - composeFilePath := path.Join(projectPath, payload.ComposeFilePathInRepository) - - fileContent, err := handler.FileService.GetFileContent(composeFilePath) + fileContent, err := handler.FileService.GetFileContent(projectPath, payload.ComposeFilePathInRepository) if err != nil { return &httperror.HandlerError{http.StatusInternalServerError, "Failed loading file content", err} } diff --git a/api/internal/stackutils/stackutils.go b/api/internal/stackutils/stackutils.go index 4945eeaa1..0d8b92e6b 100644 --- a/api/internal/stackutils/stackutils.go +++ b/api/internal/stackutils/stackutils.go @@ -3,7 +3,6 @@ package stackutils import ( "fmt" "io/ioutil" - "path" "github.com/pkg/errors" portainer "github.com/portainer/portainer/api" @@ -20,7 +19,7 @@ func ResourceControlID(endpointID portainer.EndpointID, name string) string { func GetStackFilePaths(stack *portainer.Stack) []string { var filePaths []string for _, file := range append([]string{stack.EntryPoint}, stack.AdditionalFiles...) { - filePaths = append(filePaths, path.Join(stack.ProjectPath, file)) + filePaths = append(filePaths, filesystem.JoinPaths(stack.ProjectPath, file)) } return filePaths } @@ -37,8 +36,8 @@ func CreateTempK8SDeploymentFiles(stack *portainer.Stack, kubeDeployer portainer } for _, fileName := range fileNames { - manifestFilePath := path.Join(tmpDir, fileName) - manifestContent, err := ioutil.ReadFile(path.Join(stack.ProjectPath, fileName)) + manifestFilePath := filesystem.JoinPaths(tmpDir, fileName) + manifestContent, err := ioutil.ReadFile(filesystem.JoinPaths(stack.ProjectPath, fileName)) if err != nil { return nil, "", errors.Wrap(err, "failed to read manifest file") } diff --git a/api/portainer.go b/api/portainer.go index b2803f38b..43703bc4c 100644 --- a/api/portainer.go +++ b/api/portainer.go @@ -1212,7 +1212,7 @@ type ( // FileService represents a service for managing files FileService interface { GetDockerConfigPath() string - GetFileContent(filePath string) ([]byte, error) + GetFileContent(trustedRootPath, filePath string) ([]byte, error) Copy(fromFilePath string, toFilePath string, deleteIfExists bool) error Rename(oldPath, newPath string) error RemoveDirectory(directoryPath string) error