From c8fc880a672afbfdbd384dc6afa4b7fbdd666b73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20=C4=8Cern=C3=BD?= Date: Wed, 27 May 2020 10:38:56 +0200 Subject: [PATCH 1/3] Add a regression test for RHBZ#1686370 There is a non-optimal behavior of file probe. It happens when file path is specified using a variable with 2 values with `operation="equals"` and `var_check="all"`. The probe recurses into a file system tree even if it's obvious that it won't find any match. If one of values is a big tree (for example `/`) it eventually runs out of memory and crashes. The OVAL doesn't make sense because it's impossible that a single file would have 2 different paths. But despite that it's a valid OVAL document. The test is expected to fail because the bug hasn't been fixed. --- tests/probes/file/CMakeLists.txt | 1 + .../test_probes_file_multiple_file_paths.sh | 39 +++++++++++++++++ .../test_probes_file_multiple_file_paths.xml | 42 +++++++++++++++++++ 3 files changed, 82 insertions(+) create mode 100755 tests/probes/file/test_probes_file_multiple_file_paths.sh create mode 100644 tests/probes/file/test_probes_file_multiple_file_paths.xml diff --git a/tests/probes/file/CMakeLists.txt b/tests/probes/file/CMakeLists.txt index 12718603f..35b4c1169 100644 --- a/tests/probes/file/CMakeLists.txt +++ b/tests/probes/file/CMakeLists.txt @@ -1,3 +1,4 @@ if(ENABLE_PROBES_UNIX) add_oscap_test("test_probes_file.sh") + add_oscap_test("test_probes_file_multiple_file_paths.sh") endif() diff --git a/tests/probes/file/test_probes_file_multiple_file_paths.sh b/tests/probes/file/test_probes_file_multiple_file_paths.sh new file mode 100755 index 000000000..1cececbb0 --- /dev/null +++ b/tests/probes/file/test_probes_file_multiple_file_paths.sh @@ -0,0 +1,39 @@ +#!/bin/bash + +set -e -o pipefail + +. $builddir/tests/test_common.sh + +probecheck "file" || exit 255 +which strace || exit 255 + +function check_strace_output { + strace_log="$1" + grep -q "/tmp/numbers/1" $strace_log && return 1 + grep -q "/tmp/numbers/1/2" $strace_log && return 1 + grep -q "/tmp/numbers/1/2/3" $strace_log && return 1 + grep -q "/tmp/numbers/1/2/3/4" $strace_log && return 1 + grep -q "/tmp/numbers/1/2/3/4/5" $strace_log && return 1 + grep -q "/tmp/numbers/1/2/3/4/5/6" $strace_log && return 1 + grep -q "/tmp/letters/a" $strace_log && return 1 + grep -q "/tmp/letters/a/b" $strace_log && return 1 + grep -q "/tmp/letters/a/b/c" $strace_log && return 1 + grep -q "/tmp/letters/a/b/c/d" $strace_log && return 1 + grep -q "/tmp/letters/a/b/c/d/e" $strace_log && return 1 + grep -q "/tmp/letters/a/b/c/d/e/f" $strace_log && return 1 + return 0 +} + +rm -rf /tmp/numbers +mkdir -p /tmp/numbers/1/2/3/4/5/6 +rm -rf /tmp/letters +mkdir -p /tmp/letters/a/b/c/d/e/f +strace_log=$(mktemp) +strace -f -e openat -o $strace_log $OSCAP oval eval --results results.xml "$srcdir/test_probes_file_multiple_file_paths.xml" +ret=0 +check_strace_output $strace_log || ret=$? +rm -f $strace_log +rm -f results.xml +rm -rf /tmp/numbers +rm -rf /tmp/letters +exit $ret diff --git a/tests/probes/file/test_probes_file_multiple_file_paths.xml b/tests/probes/file/test_probes_file_multiple_file_paths.xml new file mode 100644 index 000000000..893a3fe97 --- /dev/null +++ b/tests/probes/file/test_probes_file_multiple_file_paths.xml @@ -0,0 +1,42 @@ + + + + 5.10 + 0001-01-01T00:00:00+00:00 + + + + + + Specify a file path using variable with two values + x + + multi_platform_all + + + + + + + + + + + + + + + + + + + + + + + + /tmp/numbers + /tmp/letters + + + From 569e0013ca83adef233ddecc78a052db9b3ccc5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20=C4=8Cern=C3=BD?= Date: Tue, 2 Jun 2020 15:11:37 +0200 Subject: [PATCH 2/3] Add strace to the list of test dependencies --- docs/developer/developer.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/developer/developer.adoc b/docs/developer/developer.adoc index 823a1504e..0f01ace74 100644 --- a/docs/developer/developer.adoc +++ b/docs/developer/developer.adoc @@ -152,7 +152,7 @@ After building the library you might want to run library self-checks. To do that you need to have these additional packages installed: ---- -wget lua which procps-ng initscripts chkconfig sendmail bzip2 rpm-build +wget lua which procps-ng initscripts chkconfig sendmail bzip2 rpm-build strace ---- On Ubuntu 18.04, also install: From a47604bf30c6574e570abde4fd01488ba120f82d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20=C4=8Cern=C3=BD?= Date: Wed, 17 Jun 2020 11:00:02 +0200 Subject: [PATCH 3/3] Terminate matching to prevent recursion Fixes: RHBZ#1686370 --- src/OVAL/probes/oval_fts.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/OVAL/probes/oval_fts.c b/src/OVAL/probes/oval_fts.c index 696997942..2b7314c38 100644 --- a/src/OVAL/probes/oval_fts.c +++ b/src/OVAL/probes/oval_fts.c @@ -1029,6 +1029,15 @@ static FTSENT *oval_fts_read_match_path(OVAL_FTS *ofts) if (ores == OVAL_RESULT_TRUE) break; + if (ofts->ofts_path_op == OVAL_OPERATION_EQUALS) { + /* At this point the comparison result isn't OVAL_RESULT_TRUE. Since + we passed the exact path (from filepath or path elements) to + fts_open() we surely know that we can't find other items that would + be equal. Therefore we can terminate the matching. This can happen + if the filepath or path element references a variable that has + multiple different values. */ + return NULL; + } } /* for (;;) */ /*