adamwill / rpms / openscap

Forked from rpms/openscap 3 years ago
Clone
Blob Blame History Raw
From 47a2662bccb8e6f2f192acf46c26d862fe3bbcfb Mon Sep 17 00:00:00 2001
From: Evgeny Kolesnikov <ekolesni@redhat.com>
Date: Fri, 17 Jan 2020 10:24:07 +0100
Subject: [PATCH 1/2] Covscan fixes

Error: FORWARD_NULL (CWE-476): [#def17]
xccdf_policy_remediate.c:383: var_compare_op: Comparing "rr" to null implies that "rr" might be null.
xccdf_policy_remediate.c:384: var_deref_model: Passing null pointer "rr" to "_rule_add_info_message", which dereferences it.

Error: FORWARD_NULL (CWE-476): [#def18]
test_fsdev_is_local_fs.c:35: assign_zero: Assigning: "ment.mnt_fsname" = "NULL".
test_fsdev_is_local_fs.c:37: var_deref_model: Passing "&ment" to "is_local_fs", which dereferences null "ment.mnt_fsname".
---
 src/OVAL/probes/fsdev.c                   |  4 ++++
 src/XCCDF_POLICY/xccdf_policy_remediate.c | 12 ++++++++++--
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/src/OVAL/probes/fsdev.c b/src/OVAL/probes/fsdev.c
index bd8e52fbf..a6b36f5e0 100644
--- a/src/OVAL/probes/fsdev.c
+++ b/src/OVAL/probes/fsdev.c
@@ -97,6 +97,10 @@ static int is_local_fs(struct mntent *ment)
 		return 0;
 	}
 
+	if (ment->mnt_fsname == NULL) {
+		return 0;
+	}
+
 	s = ment->mnt_fsname;
 	/* If the fsname begins with "//", it is probably CIFS. */
 	if (s[0] == '/' && s[1] == '/')
diff --git a/src/XCCDF_POLICY/xccdf_policy_remediate.c b/src/XCCDF_POLICY/xccdf_policy_remediate.c
index 389a7d1bd..f59737727 100644
--- a/src/XCCDF_POLICY/xccdf_policy_remediate.c
+++ b/src/XCCDF_POLICY/xccdf_policy_remediate.c
@@ -380,7 +380,11 @@ static inline int _xccdf_fix_decode_xml(struct xccdf_fix *fix, char **result)
 #if defined(unix) || defined(__unix__) || defined(__unix)
 static inline int _xccdf_fix_execute(struct xccdf_rule_result *rr, struct xccdf_fix *fix)
 {
-	if (fix == NULL || rr == NULL || oscap_streq(xccdf_fix_get_content(fix), NULL)) {
+	if (rr == NULL) {
+		return 1;
+	}
+
+	if (fix == NULL || oscap_streq(xccdf_fix_get_content(fix), NULL)) {
 		_rule_add_info_message(rr, "No fix available.");
 		return 1;
 	}
@@ -481,7 +485,11 @@ static inline int _xccdf_fix_execute(struct xccdf_rule_result *rr, struct xccdf_
 #else
 static inline int _xccdf_fix_execute(struct xccdf_rule_result *rr, struct xccdf_fix *fix)
 {
-	if (fix == NULL || rr == NULL || oscap_streq(xccdf_fix_get_content(fix), NULL)) {
+	if (rr == NULL) {
+		return 1;
+	}
+
+	if (fix == NULL || oscap_streq(xccdf_fix_get_content(fix), NULL)) {
 		_rule_add_info_message(rr, "No fix available.");
 		return 1;
 	} else {

From 7bccc09eabd30e0581cf0fdf4f20fa481db12e91 Mon Sep 17 00:00:00 2001
From: Evgeny Kolesnikov <ekolesni@redhat.com>
Date: Fri, 17 Jan 2020 11:04:13 +0100
Subject: [PATCH 2/2] Covscan fixes (SHELLCHECK), small refactoring in Shell
 wrappers

Error: SHELLCHECK_WARNING:
warning: die references arguments, but none are ever passed. [SC2120]

Error: SHELLCHECK_WARNING:
warning: Use 'cd ... || exit' or 'cd ... || return' in case cd fails. [SC2164]

Error: SHELLCHECK_WARNING:
warning: Declare and assign separately to avoid masking return values. [SC2155]
---
 utils/oscap-chroot | 20 ++++++++++++--------
 utils/oscap-podman | 42 +++++++++++++++++++++---------------------
 utils/oscap-ssh    | 39 ++++++++++++++++++++++-----------------
 utils/oscap-vm     | 19 +++++++++++--------
 4 files changed, 66 insertions(+), 54 deletions(-)

diff --git a/utils/oscap-chroot b/utils/oscap-chroot
index 6518d7a2c..318f55a91 100755
--- a/utils/oscap-chroot
+++ b/utils/oscap-chroot
@@ -25,6 +25,13 @@ function die()
     exit 1
 }
 
+function invalid()
+{
+    echo -e "$*\n" >&2
+    usage
+    exit 1
+}
+
 function usage()
 {
     echo "oscap-chroot -- Tool for offline SCAP evaluation of filesystems mounted in arbitrary paths."
@@ -74,26 +81,23 @@ function usage()
 }
 
 if [ $# -lt 1 ]; then
-    echo "No arguments provided."
-    usage
-    die
+    invalid "No arguments provided."
 elif [ "$1" == "-h" ] || [ "$1" == "--help" ]; then
     usage
-    die
+    exit 0
 elif [ "$#" -gt 1 ]; then
     true
 else
-    echo "Invalid arguments provided."
-    usage
-    die
+    invalid "Invalid arguments provided."
 fi
 
 # Learn more at https://www.redhat.com/archives/open-scap-list/2013-July/msg00000.html
 export OSCAP_PROBE_ROOT
-OSCAP_PROBE_ROOT="$(cd "$1"; pwd)"
+OSCAP_PROBE_ROOT="$(cd "$1" && pwd)" || die "Invalid CHROOT_PATH argument."
 export OSCAP_EVALUATION_TARGET="chroot://$OSCAP_PROBE_ROOT"
 shift 1
 
 oscap "$@"
 EXIT_CODE=$?
+
 exit $EXIT_CODE
diff --git a/utils/oscap-podman b/utils/oscap-podman
index 32ec0cfcb..6b9f4a3de 100755
--- a/utils/oscap-podman
+++ b/utils/oscap-podman
@@ -16,13 +16,19 @@
 # License along with this library; if not, write to the Free Software
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
 
-
 function die()
 {
     echo "$*" >&2
     exit 1
 }
 
+function invalid()
+{
+    echo -e "$*\n" >&2
+    usage
+    exit 1
+}
+
 function usage()
 {
     echo "oscap-podman -- Tool for SCAP evaluation of Podman images and containers."
@@ -39,30 +45,24 @@ function usage()
 OSCAP_BINARY=oscap
 
 if [ $# -lt 1 ]; then
-    echo "No arguments provided."
-    usage
-    die
+    invalid "No arguments provided."
 elif [ "$1" == "-h" ] || [ "$1" == "--help" ]; then
     usage
-    die
+    exit 0
 elif [[ "$1" == --oscap=* ]] && [ $# -gt 2 ]; then
     OSCAP_BINARY=${1#"--oscap="}
     shift
 elif [ "$#" -gt 1 ]; then
     true
 else
-    echo "Invalid arguments provided."
-    usage
-    die
+    invalid "Invalid arguments provided."
 fi
 
 if [ $(id -u) -ne 0 ]; then
-    echo "This script cannot run in rootless mode." >&2
-    die
+    die "This script cannot run in rootless mode."
 fi
 if grep -q "\-\-remediate" <<< "$@"; then
-    echo "This script does not support '--remediate' option." >&2
-    die
+    die "This script does not support '--remediate' option."
 fi
 
 IMAGE_NAME=$(podman image exists "$1" \
@@ -72,14 +72,13 @@ CONTAINER_NAME=$(podman container exists "$1" \
 
 if [ -n "$IMAGE_NAME" ] && [ -n "$CONTAINER_NAME" ]; then
     echo "Ambiguous target, container image and container with the same name detected: '$1'." >&2
-    echo "Please rather use an unique ID to specify the target of the scan." >&2
-    die
+    die  "Please rather use an unique ID to specify the target of the scan."
 fi
 
 # Check if the target of scan is image or container.
 CLEANUP=0
 if [ -n "$IMAGE_NAME" ]; then
-    ID=$(podman create $1) || die
+    ID=$(podman create $1) || die "Unable to create a container."
     TARGET="podman-image://$IMAGE_NAME"
     CLEANUP=1
 elif [ -n "$CONTAINER_NAME" ]; then
@@ -87,14 +86,13 @@ elif [ -n "$CONTAINER_NAME" ]; then
     ID=$1
     TARGET="podman-container://$CONTAINER_NAME"
 else
-    echo "Target of the scan not found: '$1'." >&2
-    die
+    die "Target of the scan not found: '$1'."
 fi
 
 # podman init creates required files such as: /run/.containerenv - we don't care about output and exit code
 podman init $ID &> /dev/null || true
 
-DIR=$(podman mount $ID) || die
+DIR=$(podman mount $ID) || die "Failed to mount."
 
 if [ ! -f "$DIR/run/.containerenv" ]; then
     # ubi8-init image does not create .containerenv when running podman init, but we need to make sure that the file is there
@@ -105,14 +103,16 @@ for VAR in `podman inspect $ID --format '{{join .Config.Env " "}}'`; do
     eval "export OSCAP_OFFLINE_$VAR"
 done
 
-export OSCAP_PROBE_ROOT="$(cd "$DIR"; pwd)"
+export OSCAP_PROBE_ROOT
+OSCAP_PROBE_ROOT="$(cd "$DIR" && pwd)" || die "Unable to change current directory to OSCAP_PROBE_ROOT (DIR)."
 export OSCAP_EVALUATION_TARGET="$TARGET"
 shift 1
 
 $OSCAP_BINARY "$@"
 EXIT_CODE=$?
-podman umount $ID > /dev/null || die
+
+podman umount $ID > /dev/null || die "Failed to unmount."
 if [ $CLEANUP -eq 1 ]; then
-    podman rm $ID > /dev/null || die
+    podman rm $ID > /dev/null || die "Failed to clean up."
 fi
 exit $EXIT_CODE
diff --git a/utils/oscap-ssh b/utils/oscap-ssh
index 08c8bcd2b..cd3600180 100755
--- a/utils/oscap-ssh
+++ b/utils/oscap-ssh
@@ -22,9 +22,12 @@ function die()
     exit 1
 }
 
-hash ssh 2> /dev/null || die "Cannot find ssh, please install the OpenSSH client."
-hash scp 2> /dev/null || die "Cannot find scp, please install the OpenSSH client."
-hash mktemp 2> /dev/null || die "Cannot find mktemp, please install coreutils."
+function invalid()
+{
+    echo -e "$*\n" >&2
+    usage
+    exit 1
+}
 
 function usage()
 {
@@ -87,10 +90,6 @@ function usage()
     echo "See \`man oscap\` to learn more about semantics of these options."
 }
 
-OSCAP_SUDO=""
-# SSH_ADDITIONAL_OPTIONS may be defined in the calling shell
-SSH_TTY_ALLOCATION_OPTION=""
-
 # $1, $2, ... SSH options (pass them as separate arguments)
 function ssh_execute_with_options {
     ssh -o ControlPath="$MASTER_SOCKET" $SSH_ADDITIONAL_OPTIONS "$@" -p "$SSH_PORT" "$SSH_HOST"
@@ -118,22 +117,20 @@ function scp_retreive_from_temp_dir {
 # Returns: String, where individual command components are double-quoted, so they are not interpreted by the shell.
 #  For example, an array ('-p' '(all)') will be transformed to "\"-p\" \"(all)\"", so after the shell expansion, it will end up as "-p" "(all)".
 function command_array_to_string {
-	eval "printf '\"%s\" ' \"\${$1[@]}\""
+    eval "printf '\"%s\" ' \"\${$1[@]}\""
 }
 
 function first_argument_is_sudo {
-	[ "$1" == "sudo" ] || [ "$1" == "--sudo" ]
-	return $?
+    [ "$1" == "sudo" ] || [ "$1" == "--sudo" ]
+    return $?
 }
 
 function sanity_check_arguments {
     if [ $# -lt 1 ]; then
-        echo "No arguments provided."
-        usage
-        die
+        invalid "No arguments provided."
     elif [ "$1" == "-h" ] || [ "$1" == "--help" ]; then
         usage
-        die
+        exit 0
     elif first_argument_is_sudo "$@"; then
         OSCAP_SUDO="sudo"
         # force pseudo-tty allocation so that users can type their password if necessary
@@ -141,9 +138,7 @@ function sanity_check_arguments {
         shift
     fi
     if [ $# -lt 2 ]; then
-        echo "Missing ssh host and ssh port."
-        usage
-        die
+        invalid "Missing ssh host and ssh port."
     fi
 }
 
@@ -165,6 +160,16 @@ function check_oscap_arguments {
     fi
 }
 
+
+hash ssh 2> /dev/null || die "Cannot find ssh, please install the OpenSSH client."
+hash scp 2> /dev/null || die "Cannot find scp, please install the OpenSSH client."
+hash mktemp 2> /dev/null || die "Cannot find mktemp, please install coreutils."
+
+
+OSCAP_SUDO=""
+# SSH_ADDITIONAL_OPTIONS may be defined in the calling shell
+SSH_TTY_ALLOCATION_OPTION=""
+
 sanity_check_arguments "$@"
 first_argument_is_sudo "$@" && shift
 
diff --git a/utils/oscap-vm b/utils/oscap-vm
index 02f8c6396..6557eb3a7 100755
--- a/utils/oscap-vm
+++ b/utils/oscap-vm
@@ -22,6 +22,13 @@ function die()
     exit 1
 }
 
+function invalid()
+{
+    echo -e "$*\n" >&2
+    usage
+    exit 1
+}
+
 function usage()
 {
     echo "oscap-vm -- Tool for offline SCAP evaluation of virtual machines."
@@ -76,12 +83,10 @@ function usage()
 OSCAP_BINARY=oscap
 
 if [ $# -lt 1 ]; then
-    echo "No arguments provided."
-    usage
-    die
+    invalid "No arguments provided."
 elif [ "$1" == "-h" ] || [ "$1" == "--help" ]; then
     usage
-    die
+    exit 0
 elif [[ "$1" == --oscap=* ]] && [ $# -gt 3 ]; then
     OSCAP_BINARY=${1#"--oscap="}
     shift
@@ -90,9 +95,7 @@ elif [ "$1" == "image" ] && [ $# -gt 2 ]; then
 elif [ "$1" == "domain" ] && [ $# -gt 2 ]; then
     true
 else
-    echo "Invalid arguments provided."
-    usage
-    die
+    invalid "Invalid arguments provided."
 fi
 
 hash guestmount 2> /dev/null || die "Cannot find guestmount, please install libguestfs utilities."
@@ -128,7 +131,7 @@ fi
 
 # Learn more at https://www.redhat.com/archives/open-scap-list/2013-July/msg00000.html
 export OSCAP_PROBE_ROOT
-OSCAP_PROBE_ROOT="$(cd "$MOUNTPOINT"; pwd)"
+OSCAP_PROBE_ROOT="$(cd "$MOUNTPOINT" && pwd)" || die "Unable to change current directory to OSCAP_PROBE_ROOT (MOUNTPOINT)."
 export OSCAP_EVALUATION_TARGET="oscap-vm $1 $2"
 shift 2