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