powershell: external commands quouting improvements (#361)

Co-authored-by: Ahmet Oeztuerk <Ahmet.Oeztuerk@consol.de>

Author: inqrphl

packaging/snclient.ini

@@ -349,7 +349,7 @@ allow arguments = false
 ; we will allow clients to specify nasty (as defined in nasty characters) characters in arguments.
 allow nasty characters = false
 
-; Script root folder - Root path where all scripts are contained (You can not upload/download scripts outside this folder).
+; Script root folder - Root path where all scripts are contained. Used in external script wrappers.
 script root = ${scripts}
 
 ; Load all scripts in a given folder - Load all (${script path}/*) scripts in a given directory and use them as commands.

pkg/snclient/snclient.go

@@ -1096,8 +1096,8 @@ func (snc *Agent) restartWatcherCb(restartCb func()) {
 }
 
 func fixReturnCodes(output, stderr *string, exitCode *int64, timeout int64, cmd *exec.Cmd, procState *os.ProcessState, err error) {
-	log.Tracef("stdout: %s", *output)
-	log.Tracef("stderr: %s", *stderr)
+	log.Tracef("stdout: \n%s", *output)
+	log.Tracef("stderr: \n%s", *stderr)
 	log.Tracef("exitCode: %d", *exitCode)
 	log.Tracef("timeout: %d", timeout)
 	log.Tracef("error: %#v", err)
@@ -1367,9 +1367,9 @@ func (snc *Agent) MakeCmd(ctx context.Context, command string) (*exec.Cmd, error
 	case err != nil:
 		return nil, err
 	case cmd.Args != nil:
-		log.Tracef("command object:\n path: %s\n args: %v\n dir: %s\n workingDirectory: %s\n SysProcAttr: %v\n", cmd.Path, cmd.Args, cmd.Dir, workingDirectory, cmd.SysProcAttr)
+		log.Tracef("command object:\n path: %s\n args: %v\n dir: %s\n workingDirectory: %s\n SysProcAttr: %#v\n", cmd.Path, cmd.Args, cmd.Dir, workingDirectory, cmd.SysProcAttr)
 	default:
-		log.Tracef("command object:\n path: %s\n args: (none)\n dir: %s\n workingDirectory: %s\n SysProcAttr: %v\n", cmd.Path, cmd.Dir, workingDirectory, cmd.SysProcAttr)
+		log.Tracef("command object:\n path: %s\n args: (none)\n dir: %s\n workingDirectory: %s\n SysProcAttr: %#v\n", cmd.Path, cmd.Dir, workingDirectory, cmd.SysProcAttr)
 	}
 
 	return cmd, err

pkg/snclient/snclient_windows.go

@@ -262,6 +262,8 @@ func (snc *Agent) makeCmd(ctx context.Context, command string) (*exec.Cmd, error
 		cmd := execCommandContext(ctx, "powershell", env)
 		cmd.SysProcAttr.CmdLine = fmt.Sprintf(`%s -command %s; exit($LASTEXITCODE)`, POWERSHELL, command)
 
+		log.Tracef("cmd.SysProcAttr.CmdLine: %s", cmd.SysProcAttr.CmdLine)
+
 		return cmd, nil
 
 	// command does not exist
@@ -283,17 +285,34 @@ func (snc *Agent) makeCmd(ctx context.Context, command string) (*exec.Cmd, error
 			strings.Join(cmdArgs, " "),
 		)
 
+		log.Tracef("cmd.SysProcAttr.CmdLine: %s", cmd.SysProcAttr.CmdLine)
+
 		return cmd, nil
 
 	// powershell files
 	case isPsFile(cmdName):
-		for i, ca := range cmdArgs {
-			if strings.ContainsAny(ca, " \t") {
-				cmdArgs[i] = `'` + ca + `'`
-			}
+		// parse the command one more time, this time adding the shelltoken.KeepQuoutes option
+		cmdName, cmdArgs, _, err = snc.shellParse(command, shelltoken.SplitKeepQuotes)
+		if err != nil {
+			return nil, err
+		}
+
+		cmdArgsModified := make([]string, 0, len(cmdArgs))
+		for _, cmdArg := range cmdArgs {
+			// parsed arguments might include double quoutes.
+			// cmd.SysProcAttr.CmdLine is set so that the arguments are found within double quoutes inside the -Command parameter
+			// to include a double quoute here, you have to add three double quoutes.
+			// windows has very confusing, runtime-dependent command line argument parsing
+			// This is done with the assumption that its using GetCommandLineW, and it works for now
+			cmdArg = strings.ReplaceAll(cmdArg, `"`, `"""`)
+
+			cmdArgsModified = append(cmdArgsModified, cmdArg)
 		}
+
 		cmd := execCommandContext(ctx, "powershell", env)
-		cmd.SysProcAttr.CmdLine = fmt.Sprintf(`%s -Command ". '%s' %s; exit($LASTEXITCODE)"`, POWERSHELL, cmdName, strings.Join(cmdArgs, " "))
+		cmd.SysProcAttr.CmdLine = fmt.Sprintf(`%s -Command ". '%s' %s ; exit($LASTEXITCODE)"`, POWERSHELL, cmdName, strings.Join(cmdArgsModified, " "))
+
+		log.Tracef("cmd.SysProcAttr.CmdLine: %s", cmd.SysProcAttr.CmdLine)
 
 		return cmd, nil
 
@@ -312,12 +331,29 @@ func (snc *Agent) makeCmd(ctx context.Context, command string) (*exec.Cmd, error
 			shell,
 			strings.Replace(command, cmdName, syscall.EscapeArg(cmdName), 1))
 
+		log.Tracef("cmd.SysProcAttr.CmdLine: %s", cmd.SysProcAttr.CmdLine)
+
 		return cmd, nil
 	}
 }
 
-func (snc *Agent) shellParse(command string) (cmdName string, args []string, hasShellCode bool, err error) {
-	args, err = shelltoken.SplitQuotes(command, shelltoken.Whitespace, shelltoken.SplitKeepBackslashes|shelltoken.SplitContinueOnShellCharacters)
+func (snc *Agent) shellParse(command string, additionalOptions ...shelltoken.SplitOption) (cmdName string, args []string, hasShellCode bool, err error) {
+	options := shelltoken.SplitKeepBackslashes | shelltoken.SplitContinueOnShellCharacters
+
+	// shelltoken.SplitKeepQuotes may be passed as an additional Option
+	// when invoking a script, the script might use $ARG1$ macro
+	// arg1 here might be a single string composed of many arguments, e.g:
+	// powershell_detail_arg1 "-option1 option1 -option2 `'option2`' -option3 `"option3`" -option4 `'foo,bar`' -option5 `"baz,xyz`" "
+	// if sheltoken.SplitKeepQuoutes option is not set, it strips the quotation marks from each option value
+	// this leads to arguments like foo,bar not being quouted and being left as is
+	// powershell then thinks its an array due to comma
+	// if they were quouted, it would think that they are a string that includes comma character
+
+	for _, opt := range additionalOptions {
+		options |= opt
+	}
+
+	args, err = shelltoken.SplitQuotes(command, shelltoken.Whitespace, options)
 	if err != nil {
 		tst := &shelltoken.ShellCharactersFoundError{}
 		if errors.As(err, &tst) {