better script options

Author: crmullineravsec

Readme.md

@@ -270,16 +270,16 @@ The check produces an offender if the script produced output on stdout or stderr
 
 - `File` : string, the full path of the file or directory
 - `Script` : string, the full path of the script
-- `ScriptOptions` : string array, (optional) the first element allows to define a pattern containing wildcards like `?`, `*`, and `**` that is applied to filenames if present it will only check files that match the pattern, this is mostly useful when running the script on a directory. The second element allows passing arguments to the script.
+- `ScriptOptions` : string array, (optional) the first element allows to define a pattern containing wildcards like `?`, `*`, and `**` that is applied to filenames if present it will only check files that match the pattern, this is mostly useful when running the script on a directory. Arguments can be passed to the script using the second and following elements.
 - `File` : string, the full path of the file, if the path points to a directory the script is run for every file in the directory and subdirectories
 
 - `Desc` : string, (optional) is a descriptive string that will be attached to failed check
 - `InformationalOnly` : bool, (optional) the result of the check will be Informational only (default: false)
 
-If the `--` is present it indicates that the next argument is from the `ScriptOptions[1]`. The script is run with the following arguments:
+If the `--` is present it indicates that the next argument is from the `ScriptOptions[1..N]`. The script is run with the following arguments:
 
 ```
-<tmp filename> <original filename> <uid> <gid> <mode in octal> <selinux label or "-" for no label> [--] [script options argument]
+<tmp filename> <original filename> <uid> <gid> <mode in octal> <selinux label or "-" for no label> [--] [script argument 1] ... [script argument N]
 ```
 
 Example:
@@ -345,13 +345,13 @@ as arguments. If the script prints output the check will be marked as failed.
 
 - `File` : string, the full path of the file
 - `Script`: string, path to the script
-- `ScriptOptions`: string, argument passed to the script
+- `ScriptOptions`: string array, (optional) arguments passed to the script
 - `OldFilePath`: string, filename (absolute or relative) to use to store old file
 - `InformationalOnly` : bool, (optional) the result of the check will be Informational only (default: false)
 
 Script runs as:
 ```sh
-script.sh <OrigFilename> <oldFile> <newFile> -- <argument>
+script.sh <OrigFilename> <oldFile> <newFile> [--] [argument 1] .. [argument N]
 ```
 
 Example:
@@ -465,14 +465,15 @@ this statement, the example below is named LastLine.
 
 - `File` : string, the full path of the file
 - `Script` :string, the full path of the script
+- `ScriptOptions` : string array (optionl), arguments to pass to the script
 - `Name` : string, (optional) the key name
 
 - `Desc` : string, (optional) description
 
 The script is run with the following arguments:
 
 ```
-<tmp filename> <original filename> <uid> <gid> <mode in octal> <selinux label or "-" for no label>
+<tmp filename> <original filename> <uid> <gid> <mode in octal> <selinux label or "-" for no label> [--] [script argument 1] ... [script argument N]
 ```
 
 Example:

pkg/analyzer/dataextract/dataextract.go

@@ -34,12 +34,13 @@ import (
 )
 
 type dataType struct {
-	File   string
-	Script string
-	RegEx  string
-	Json   string
-	Desc   string
-	Name   string // the name can be set directly otherwise the key will be used
+	File          string
+	Script        string
+	ScriptOptions []string // options for script execution
+	RegEx         string
+	Json          string
+	Desc          string
+	Name          string // the name can be set directly otherwise the key will be used
 }
 
 type dataExtractType struct {
@@ -152,7 +153,7 @@ func (state *dataExtractType) CheckFile(fi *fsparser.FileInfo, filepath string)
 		}
 
 		if item.Script != "" {
-			out, err := runScriptOnFile(state.a, item.Script, fi, fn)
+			out, err := runScriptOnFile(state.a, item.Script, item.ScriptOptions, fi, fn)
 			if err != nil {
 				state.a.AddData(item.Name, fmt.Sprintf("DataExtract ERROR: script error: %s : %s : %s", err, item.Name, item.Desc))
 			} else {
@@ -186,14 +187,19 @@ func (state *dataExtractType) CheckFile(fi *fsparser.FileInfo, filepath string)
 	return nil
 }
 
-// runScriptOnFile runs the provided script with the following parameters: <filename> <filename in filesystem> <uid> <gid> <mode> <selinux label - can be empty>
-func runScriptOnFile(a analyzer.AnalyzerType, script string, fi *fsparser.FileInfo, fpath string) (string, error) {
+// runScriptOnFile runs the provided script with the following parameters:
+// <filename> <filename in filesystem> <uid> <gid> <mode> <selinux label - can be empty> -- scriptOptions[0] scriptOptions[1]
+func runScriptOnFile(a analyzer.AnalyzerType, script string, scriptOptions []string, fi *fsparser.FileInfo, fpath string) (string, error) {
 	fname, err := a.FileGet(fpath)
 	if err != nil {
 		return "", err
 	}
-	out, err := exec.Command(script, fname, filepath.Base(fpath),
-		fmt.Sprintf("%d", fi.Uid), fmt.Sprintf("%d", fi.Gid), fmt.Sprintf("%o", fi.Mode), fi.SELinuxLabel).Output()
+	options := []string{fname, filepath.Base(fpath), fmt.Sprintf("%d", fi.Uid), fmt.Sprintf("%d", fi.Gid), fmt.Sprintf("%o", fi.Mode), fi.SELinuxLabel}
+	if len(scriptOptions) > 0 {
+		options = append(options, "--")
+		options = append(options, scriptOptions...)
+	}
+	out, err := exec.Command(script, options...).Output()
 	_ = a.RemoveFile(fname)
 
 	return string(out), err

pkg/analyzer/filecmp/filecmp.go

@@ -35,7 +35,7 @@ type cmpType struct {
 	File              string // filename
 	OldFilePath       string
 	Script            string
-	ScriptOptions     string
+	ScriptOptions     []string
 	InformationalOnly bool   // put result into Informational (not Offenders)
 	name              string // name of this check (need to be unique)
 
@@ -163,7 +163,7 @@ func (state *fileCmpType) CheckFile(fi *fsparser.FileInfo, filepath string) erro
 		args := []string{fi.Name, oldTmp, tmpfn}
 		if len(item.ScriptOptions) > 0 {
 			args = append(args, "--")
-			args = append(args, item.ScriptOptions)
+			args = append(args, item.ScriptOptions...)
 		}
 
 		out, err := exec.Command(item.Script, args...).CombinedOutput()

pkg/analyzer/filecmp/filecmp_test.go

@@ -65,7 +65,7 @@ func TestCmp(t *testing.T) {
 [FileCmp."Test1"]
 File ="/cmp_test_1"
 Script = "diff.sh"
-ScriptOptions = ""
+ScriptOptions = [""]
 OldFilePath = "/tmp/analyzer_filecmp_1"
 `
 
@@ -157,7 +157,7 @@ func TestCmpInfo(t *testing.T) {
 [FileCmp."Test1"]
 File ="/cmp_test_1"
 Script = "diff.sh"
-ScriptOptions = ""
+ScriptOptions = [""]
 InformationalOnly = true
 OldFilePath = "/tmp/analyzer_filecmp_1"
 `
@@ -220,7 +220,7 @@ func TestCmpNoOld(t *testing.T) {
 [FileCmp."Test1"]
 File ="/cmp_test_1"
 Script = "diff.sh"
-ScriptOptions = ""
+ScriptOptions = [""]
 OldFilePath = "/tmp/analyzer_filecmp_99"
 `
 

pkg/analyzer/filecontent/filecontent.go

@@ -262,14 +262,14 @@ type callbackDataType struct {
  * The script output is used to indicate an issue, the output is saved in the offender record.
  *
  * The first element in scriptOptions (from the callback data) defines a path match string.
- * This allows to specify a pattern the filename has to match. Files with names that do not patch will
- * be analyzed by the script. This is to speed up execution time since files have to be extracted
+ * This allows to specify a pattern the filename has to match. Files with names that do not match will
+ * not be analyzed by the script. This is to speed up execution time since files have to be extracted
  * to analyze them with the external script.
  *
- * The second element in scriptOptions will be passed to the script as cmd line arguments.
+ * The following elements in scriptOptions will be passed to the script as cmd line arguments.
  *
  * The script is run with the following parameters:
- * script.sh <filename> <filename in filesystem> <uid> <gid> <mode> <selinux label - can be empty> -- <ScriptOptions[1]>
+ * script.sh <filename> <filename in filesystem> <uid> <gid> <mode> <selinux label - can be empty> -- <ScriptOptions[1]> <ScriptOptions[2]>
  */
 func checkFileScript(fi *fsparser.FileInfo, fullpath string, cbData analyzer.AllFilesCallbackData) {
 	cbd := cbData.(*callbackDataType)
@@ -293,9 +293,9 @@ func checkFileScript(fi *fsparser.FileInfo, fullpath string, cbData analyzer.All
 
 	fname, _ := cbd.state.a.FileGet(path.Join(fullpath, fi.Name))
 	args := []string{fname, fi.Name, fmt.Sprintf("%d", fi.Uid), fmt.Sprintf("%d", fi.Gid), fmt.Sprintf("%o", fi.Mode), fi.SELinuxLabel}
-	if len(cbd.scriptOptions) >= 2 && len(cbd.scriptOptions[1]) > 0 {
+	if len(cbd.scriptOptions) >= 2 {
 		args = append(args, "--")
-		args = append(args, cbd.scriptOptions[1])
+		args = append(args, cbd.scriptOptions[1:]...)
 	}
 
 	out, err := exec.Command(cbd.script, args...).CombinedOutput()

test/script_test.sh

@@ -0,0 +1,13 @@
+#!/bin/bash
+
+FILEPATH=$1
+ORIG_FILENAME=$2
+ORIG_UID=$3
+ORIG_GID=$4
+ORIG_MODE=$5
+ORIG_SELINUXLABEL=$6
+
+# this is an artificial test
+if [ "$7" = "--" ]; then
+    echo -n $9 $8
+fi

test/test.py

@@ -45,28 +45,34 @@ def test(cfgfile, e2toolspath=""):
     if data.get("data", {}).get("Version") != "1.2.3":
         SetError("Data Version")
 
+    if data.get("data", {}).get("extract_test") != "test extract":
+        SetError("extract test")
+
     if "offenders" not in data:
         SetError("offenders")
     else:
-        if "/dir2/file21" not in data["offenders"]:
+        if not "/dir2/file21" in data["offenders"]:
             SetError("dir2/file21")
 
-        if "/dir2/file22" not in data["offenders"]:
+        if not "/dir2/file22" in data["offenders"]:
             SetError("dir2/file22")
 
-        if data["offenders"]["/world"][0].find("WorldWriteable") == -1:
+        if not "File is WorldWriteable, not allowed" in data["offenders"]["/world"]:
             SetError("WorldWriteable")
 
-        if data["offenders"]["/dir2/file21"][0].find("File is SUID") == -1:
+        if not "File is SUID, not allowed" in data["offenders"]["/dir2/file21"]:
             SetError("SUID")
 
-        if data["offenders"]["/dir2/file22"][0].find("DirContent") == -1:
+        if not "DirContent: File file22 not allowed in directory /dir2" in data["offenders"]["/dir2/file22"]:
             SetError("DirContent")
 
-        if "nofile" not in data["offenders"]:
+        if not "nofile" in data["offenders"]:
             SetError("DirContent")
 
-        if data["offenders"]["/ver"][0].find("Digest") == -1:
+        if not "test script" in data["offenders"]["/file2"]:
+            SetError("file2")
+
+        if not "Digest (sha256) did not match found = 44c77e41961f354f515e4081b12619fdb15829660acaa5d7438c66fc3d326df3 should be = 8b15095ed1af38d5e383af1c4eadc5ae73cab03964142eb54cb0477ccd6a8dd4. ver needs to be specific :  " in data["offenders"]["/ver"]:
             SetError("ver digest")
 
         if "File State Check failed: group found 1002 should be 0 : this needs to be this way" in data["offenders"]["/dir2/file22"]:
@@ -78,21 +84,21 @@ def test(cfgfile, e2toolspath=""):
         if not "File State Check failed: size: 0 AllowEmpyt=false : this needs to be this way" in data["offenders"]["/file1"]:
             SetError("file1 exists but size 0")
 
-        if "elf_x8664 is not stripped" not in data["offenders"]["/bin/elf_x8664"]:
+        if not "elf_x8664 is not stripped" in data["offenders"]["/bin/elf_x8664"]:
             SetError("script failed")
 
-    if "informational" not in data:
+    if not "informational" in data:
         SetError("informational")
     else:
-        if "/file1" not in data["informational"]:
+        if not "/file1" in data["informational"]:
             SetError("/file1")
         else:
-            if "changed" not in data["informational"]["/file1"][0]:
+            if not "changed" in data["informational"]["/file1"][0]:
                 SetError("file1 not changed")
-        if "/date1" not in data["informational"]:
+        if not "/date1" in data["informational"]:
             SetError("/date1")
         else:
-            if "changed" not in data["informational"]["/date1"][0]:
+            if not "changed" in data["informational"]["/date1"][0]:
                 SetError("date1 not changed")
 
 if __name__ == "__main__":

test/test_cfg.base.toml

@@ -56,8 +56,19 @@ Script="check_file_x8664.sh"
 
 [FileContent."ensure bins are stripped"]
 File = "/"
+ScriptOptions = ["*"]
 Script="check_file_elf_stripped.sh"
 
+[FileContent."script_test"]
+File = "/file2"
+ScriptOptions = ["*", "script", "test"]
+Script="script_test.sh"
+
+[DataExtract."extract_test"]
+File = "/file2"
+ScriptOptions = ["extract", "test"]
+Script="script_test.sh"
+
 [FileContent."date1 needs to be specific"]
 File = "/date1"
 Digest="8b15095ed1af38d5e383af1c4eadc5ae73cab03964142eb54cb0477ccd6a8dd5"