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