Fix path traversal vulnerabilities (#817)

Properly validate expanded paths to ensure they are not attempting path
traversal attacks

cmd/wsh/cmd/wshcmd-term.go

@@ -4,6 +4,7 @@
 package cmd
 
 import (
+	"log"
 	"os"
 	"path/filepath"
 
@@ -33,7 +34,12 @@ func termRun(cmd *cobra.Command, args []string) {
 	var cwd string
 	if len(args) > 0 {
 		cwd = args[0]
-		cwd = wavebase.ExpandHomeDir(cwd)
+		cwdExpanded, err := wavebase.ExpandHomeDir(cwd)
+		if err != nil {
+			log.Fatal(err)
+			return
+		}
+		cwd = cwdExpanded
 	} else {
 		var err error
 		cwd, err = os.Getwd()

pkg/blockcontroller/blockcontroller.go

@@ -224,7 +224,11 @@ func (bc *BlockController) DoRunShellCommand(rc *RunShellOpts, blockMeta waveobj
 		cmdOpts.Login = true
 		cmdOpts.Cwd = blockMeta.GetString(waveobj.MetaKey_CmdCwd, "")
 		if cmdOpts.Cwd != "" {
-			cmdOpts.Cwd = wavebase.ExpandHomeDir(cmdOpts.Cwd)
+			cwdPath, err := wavebase.ExpandHomeDir(cmdOpts.Cwd)
+			if err != nil {
+				return err
+			}
+			cmdOpts.Cwd = cwdPath
 		}
 	} else if bc.ControllerType == BlockController_Cmd {
 		cmdStr = blockMeta.GetString(waveobj.MetaKey_Cmd, "")
@@ -233,7 +237,11 @@ func (bc *BlockController) DoRunShellCommand(rc *RunShellOpts, blockMeta waveobj
 		}
 		cmdOpts.Cwd = blockMeta.GetString(waveobj.MetaKey_CmdCwd, "")
 		if cmdOpts.Cwd != "" {
-			cmdOpts.Cwd = wavebase.ExpandHomeDir(cmdOpts.Cwd)
+			cwdPath, err := wavebase.ExpandHomeDir(cmdOpts.Cwd)
+			if err != nil {
+				return err
+			}
+			cmdOpts.Cwd = cwdPath
 		}
 		cmdOpts.Interactive = blockMeta.GetBool(waveobj.MetaKey_CmdInteractive, false)
 		cmdOpts.Login = blockMeta.GetBool(waveobj.MetaKey_CmdLogin, false)

pkg/remote/sshclient.go

@@ -75,7 +75,11 @@ func createPublicKeyCallback(connCtx context.Context, sshKeywords *SshKeywords,
 	// checking the file early prevents us from needing to send a
 	// dummy signer if there's a problem with the signer
 	for _, identityFile := range sshKeywords.IdentityFile {
-		privateKey, err := os.ReadFile(wavebase.ExpandHomeDir(identityFile))
+		filePath, err := wavebase.ExpandHomeDir(identityFile)
+		if err != nil {
+			continue
+		}
+		privateKey, err := os.ReadFile(filePath)
 		if err != nil {
 			// skip this key and try with the next
 			continue
@@ -352,7 +356,11 @@ func createHostKeyCallback(opts *SSHOpts) (ssh.HostKeyCallback, HostKeyAlgorithm
 
 	var knownHostsFiles []string
 	for _, filename := range unexpandedKnownHostsFiles {
-		knownHostsFiles = append(knownHostsFiles, wavebase.ExpandHomeDir(filename))
+		filePath, err := wavebase.ExpandHomeDir(filename)
+		if err != nil {
+			continue
+		}
+		knownHostsFiles = append(knownHostsFiles, filePath)
 	}
 
 	// there are no good known hosts files
@@ -715,12 +723,20 @@ func findSshConfigKeywords(hostPattern string) (*SshKeywords, error) {
 		authSockCommand := exec.Command(shellPath, "-c", "echo ${SSH_AUTH_SOCK}")
 		sshAuthSock, err := authSockCommand.Output()
 		if err == nil {
-			sshKeywords.IdentityAgent = wavebase.ExpandHomeDir(trimquotes.TryTrimQuotes(strings.TrimSpace(string(sshAuthSock))))
+			agentPath, err := wavebase.ExpandHomeDir(trimquotes.TryTrimQuotes(strings.TrimSpace(string(sshAuthSock))))
+			if err != nil {
+				return nil, err
+			}
+			sshKeywords.IdentityAgent = agentPath
 		} else {
 			log.Printf("unable to find SSH_AUTH_SOCK: %v\n", err)
 		}
 	} else {
-		sshKeywords.IdentityAgent = wavebase.ExpandHomeDir(trimquotes.TryTrimQuotes(identityAgentRaw))
+		agentPath, err := wavebase.ExpandHomeDir(trimquotes.TryTrimQuotes(identityAgentRaw))
+		if err != nil {
+			return nil, err
+		}
+		sshKeywords.IdentityAgent = agentPath
 	}
 
 	return sshKeywords, nil

pkg/wavebase/wavebase.go

@@ -51,15 +51,25 @@ func GetHomeDir() string {
 	return homeVar
 }
 
-func ExpandHomeDir(pathStr string) string {
+func ExpandHomeDir(pathStr string) (string, error) {
 	if pathStr != "~" && !strings.HasPrefix(pathStr, "~/") {
-		return pathStr
+		return pathStr, nil
 	}
 	homeDir := GetHomeDir()
 	if pathStr == "~" {
-		return homeDir
+		return homeDir, nil
 	}
-	return filepath.Clean(filepath.Join(homeDir, pathStr[2:]))
+	expandedPath := filepath.Join(homeDir, pathStr[2:])
+	absPath, err := filepath.Abs(filepath.Join(homeDir, expandedPath))
+	if err != nil || !strings.HasPrefix(absPath, homeDir) {
+		return "", fmt.Errorf("Potential path traversal detected for path %s", pathStr)
+	}
+	return expandedPath, nil
+}
+
+func ExpandHomeDirSafe(pathStr string) string {
+	path, _ := ExpandHomeDir(pathStr)
+	return path
 }
 
 func ReplaceHomeDir(pathStr string) string {
@@ -80,12 +90,12 @@ func GetDomainSocketName() string {
 func GetWaveHomeDir() string {
 	homeVar := os.Getenv(WaveHomeVarName)
 	if homeVar != "" {
-		return ExpandHomeDir(homeVar)
+		return ExpandHomeDirSafe(homeVar)
 	}
 	if IsDevMode() {
-		return ExpandHomeDir(DevWaveHome)
+		return ExpandHomeDirSafe(DevWaveHome)
 	}
-	return ExpandHomeDir(DefaultWaveHome)
+	return ExpandHomeDirSafe(DefaultWaveHome)
 }
 
 func EnsureWaveHomeDir() error {

pkg/web/web.go

@@ -231,7 +231,10 @@ func handleLocalStreamFile(w http.ResponseWriter, r *http.Request, fileName stri
 			serveTransparentGIF(w)
 		}
 	} else {
-		fileName = wavebase.ExpandHomeDir(fileName)
+		fileName, err := wavebase.ExpandHomeDir(fileName)
+		if err != nil {
+			http.Error(w, err.Error(), http.StatusBadRequest)
+		}
 		http.ServeFile(w, r, fileName)
 	}
 }

pkg/wshrpc/wshremote/wshremote.go

@@ -163,8 +163,10 @@ func (impl *ServerImpl) remoteStreamFileInternal(ctx context.Context, data wshrp
 	if err != nil {
 		return err
 	}
-	path := data.Path
+	path, err := wavebase.ExpandHomeDir(data.Path)
-	path = wavebase.ExpandHomeDir(path)
+	if err != nil {
+		return err
+	}
 	finfo, err := impl.fileInfoInternal(path, true)
 	if err != nil {
 		return fmt.Errorf("cannot stat file %q: %w", path, err)
@@ -246,7 +248,7 @@ func checkIsReadOnly(path string, fileInfo fs.FileInfo, exists bool) bool {
 }
 
 func computeDirPart(path string, isDir bool) string {
-	path = filepath.Clean(wavebase.ExpandHomeDir(path))
+	path = filepath.Clean(wavebase.ExpandHomeDirSafe(path))
 	path = filepath.ToSlash(path)
 	if path == "/" {
 		return "/"
@@ -259,7 +261,7 @@ func computeDirPart(path string, isDir bool) string {
 }
 
 func (*ServerImpl) fileInfoInternal(path string, extended bool) (*wshrpc.FileInfo, error) {
-	cleanedPath := filepath.Clean(wavebase.ExpandHomeDir(path))
+	cleanedPath := filepath.Clean(wavebase.ExpandHomeDirSafe(path))
 	finfo, err := os.Stat(cleanedPath)
 	if os.IsNotExist(err) {
 		return &wshrpc.FileInfo{
@@ -281,11 +283,11 @@ func (*ServerImpl) fileInfoInternal(path string, extended bool) (*wshrpc.FileInf
 
 func resolvePaths(paths []string) string {
 	if len(paths) == 0 {
-		return wavebase.ExpandHomeDir("~")
+		return wavebase.ExpandHomeDirSafe("~")
 	}
-	var rtnPath = wavebase.ExpandHomeDir(paths[0])
+	rtnPath := wavebase.ExpandHomeDirSafe(paths[0])
 	for _, path := range paths[1:] {
-		path = wavebase.ExpandHomeDir(path)
+		path = wavebase.ExpandHomeDirSafe(path)
 		if filepath.IsAbs(path) {
 			rtnPath = path
 			continue
@@ -305,7 +307,10 @@ func (impl *ServerImpl) RemoteFileInfoCommand(ctx context.Context, path string)
 }
 
 func (*ServerImpl) RemoteWriteFileCommand(ctx context.Context, data wshrpc.CommandRemoteWriteFileData) error {
-	path := wavebase.ExpandHomeDir(data.Path)
+	path, err := wavebase.ExpandHomeDir(data.Path)
+	if err != nil {
+		return err
+	}
 	createMode := data.CreateMode
 	if createMode == 0 {
 		createMode = 0644
@@ -324,8 +329,12 @@ func (*ServerImpl) RemoteWriteFileCommand(ctx context.Context, data wshrpc.Comma
 }
 
 func (*ServerImpl) RemoteFileDeleteCommand(ctx context.Context, path string) error {
-	cleanedPath := filepath.Clean(wavebase.ExpandHomeDir(path))
+	expandedPath, err := wavebase.ExpandHomeDir(path)
-	err := os.Remove(cleanedPath)
+	if err != nil {
+		return fmt.Errorf("cannot delete file %q: %w", path, err)
+	}
+	cleanedPath := filepath.Clean(expandedPath)
+	err = os.Remove(cleanedPath)
 	if err != nil {
 		return fmt.Errorf("cannot delete file %q: %w", path, err)
 	}

pkg/wstore/wstore_dboldmigration.go

@@ -17,7 +17,7 @@ import (
 const OldDBName = "~/.waveterm/waveterm.db"
 
 func GetOldDBName() string {
-	return wavebase.ExpandHomeDir(OldDBName)
+	return wavebase.ExpandHomeDirSafe(OldDBName)
 }
 
 func MakeOldDB(ctx context.Context) (*sqlx.DB, error) {

yarn.lock

@@ -9737,6 +9737,27 @@ __metadata:
   languageName: node
   linkType: hard
 
+"send@npm:0.18.0":
+  version: 0.18.0
+  resolution: "send@npm:0.18.0"
+  dependencies:
+    debug: "npm:2.6.9"
+    depd: "npm:2.0.0"
+    destroy: "npm:1.2.0"
+    encodeurl: "npm:~1.0.2"
+    escape-html: "npm:~1.0.3"
+    etag: "npm:~1.8.1"
+    fresh: "npm:0.5.2"
+    http-errors: "npm:2.0.0"
+    mime: "npm:1.6.0"
+    ms: "npm:2.1.3"
+    on-finished: "npm:2.4.1"
+    range-parser: "npm:~1.2.1"
+    statuses: "npm:2.0.1"
+  checksum: 10c0/0eb134d6a51fc13bbcb976a1f4214ea1e33f242fae046efc311e80aff66c7a43603e26a79d9d06670283a13000e51be6e0a2cb80ff0942eaf9f1cd30b7ae736a
+  languageName: node
+  linkType: hard
+
 "send@npm:0.19.0":
   version: 0.19.0
   resolution: "send@npm:0.19.0"
@@ -9774,7 +9795,7 @@ __metadata:
     encodeurl: "npm:~1.0.2"
     escape-html: "npm:~1.0.3"
     parseurl: "npm:~1.3.3"
-    send: "npm:0.19.0"
+    send: "npm:0.18.0"
   checksum: 10c0/d7a5beca08cc55f92998d8b87c111dd842d642404231c90c11f504f9650935da4599c13256747b0a988442a59851343271fe8e1946e03e92cd79c447b5f3ae01
   languageName: node
   linkType: hard