From c59ade5fc40ab24415af1fcf1c253311700eb08f Mon Sep 17 00:00:00 2001 From: Edgar Aguilar Date: Thu, 20 Mar 2025 17:35:56 -0600 Subject: [PATCH 1/3] Fix textfilecontent54_probe behaviour According to OVAL docs the 'instance' tag should be able to take negative numbers. And these negative numbers should count from the last element matched. This is was not the case in oscap implementation Signed-off-by: Edgar Aguilar --- .../independent/textfilecontent54_probe.c | 75 +++++++++++-------- 1 file changed, 45 insertions(+), 30 deletions(-) diff --git a/src/OVAL/probes/independent/textfilecontent54_probe.c b/src/OVAL/probes/independent/textfilecontent54_probe.c index 34199853fa..d92a2ab7c7 100644 --- a/src/OVAL/probes/independent/textfilecontent54_probe.c +++ b/src/OVAL/probes/independent/textfilecontent54_probe.c @@ -123,12 +123,15 @@ struct pfdata { static int process_file(const char *prefix, const char *path, const char *file, struct pfdata *pfd, oval_schema_version_t over, struct oscap_list *blocked_paths) { int ret = 0, path_len, file_len, cur_inst = 0, fd = -1, substr_cnt, - buf_size = 0, buf_used = 0, ofs = 0, buf_inc = 4096; + buf_size = 0, buf_used = 0, ofs = 0, buf_inc = 4096, instance_count = 0, + want_instance = 1, negative_instance_value = 0; char **substrs = NULL; char *whole_path = NULL, *whole_path_with_prefix = NULL, *buf = NULL; - SEXP_t *next_inst = NULL; + SEXP_t *next_inst = NULL, *items = SEXP_list_new(NULL), *instance_value_list = NULL, + *instance_value = NULL; struct stat st; + if (file == NULL) goto cleanup; @@ -210,16 +213,6 @@ static int process_file(const char *prefix, const char *path, const char *file, buf[buf_used++] = '\0'; do { - int want_instance; - - next_inst = SEXP_number_newi_32(cur_inst + 1); - - if (probe_entobj_cmp(pfd->instance_ent, next_inst) == OVAL_RESULT_TRUE) - want_instance = 1; - else - want_instance = 0; - - SEXP_free(next_inst); substr_cnt = oscap_pcre_get_substrings(buf, &ofs, pfd->compiled_regex, want_instance, &substrs); if (substr_cnt < 0) { @@ -235,27 +228,49 @@ static int process_file(const char *prefix, const char *path, const char *file, } if (substr_cnt > 0) { - ++cur_inst; - - if (want_instance) { - int k; - SEXP_t *item; - - item = create_item(path, file, pfd->pattern, - cur_inst, substrs, substr_cnt, over); - - for (k = 0; k < substr_cnt; ++k) - free(substrs[k]); - free(substrs); - int pic_ret = probe_item_collect(pfd->ctx, item); - if (pic_ret == 2 || pic_ret == -1) { - ret = -4; - break; - } - } + int k; + instance_count++; + + SEXP_list_add(items, create_item(path, file, pfd->pattern, + instance_count, substrs, substr_cnt, over)); + + for (k = 0; k < substr_cnt; ++k) + free(substrs[k]); + free(substrs); } } while (substr_cnt > 0 && ofs < buf_used); + probe_ent_getvals(pfd->instance_ent, &instance_value_list); + instance_value = SEXP_list_first(instance_value_list); + negative_instance_value = SEXP_number_geti_64(instance_value) < 0; + SEXP_free(instance_value_list); + SEXP_free(instance_value); + + for(cur_inst = 0; cur_inst < instance_count; cur_inst++){ + if(negative_instance_value) + next_inst = SEXP_number_newi_32(cur_inst - instance_count); + + else + next_inst = SEXP_number_newi_32(cur_inst + 1); + + if (probe_entobj_cmp(pfd->instance_ent, next_inst) == OVAL_RESULT_TRUE) + want_instance = 1; + else + want_instance = 0; + + SEXP_free(next_inst); + + if (want_instance){ + int pic_ret = probe_item_collect(pfd->ctx, SEXP_list_nth(items, cur_inst + 1)); + if (pic_ret == 2 || pic_ret == -1) { + ret = -4; + break; + } + } + else + SEXP_free(SEXP_list_nth(items, cur_inst + 1)); + } + cleanup: if (fd != -1) close(fd); From b9140df91c812e63ffdf3d001cdd28ec6a9ebbcf Mon Sep 17 00:00:00 2001 From: Edgar Aguilar Date: Mon, 24 Mar 2025 18:12:49 -0600 Subject: [PATCH 2/3] Add test for textfilecontent54 Validate behaviour with negative instance Signed-off-by: Edgar Aguilar --- tests/probes/textfilecontent54/CMakeLists.txt | 1 + .../test_negative_instance.sh | 37 ++++ .../test_negative_instance.xml | 180 ++++++++++++++++++ 3 files changed, 218 insertions(+) create mode 100755 tests/probes/textfilecontent54/test_negative_instance.sh create mode 100644 tests/probes/textfilecontent54/test_negative_instance.xml diff --git a/tests/probes/textfilecontent54/CMakeLists.txt b/tests/probes/textfilecontent54/CMakeLists.txt index 9577016ba1..3f95ae8a20 100644 --- a/tests/probes/textfilecontent54/CMakeLists.txt +++ b/tests/probes/textfilecontent54/CMakeLists.txt @@ -6,4 +6,5 @@ if(ENABLE_PROBES_INDEPENDENT) add_oscap_test("test_recursion_limit.sh") add_oscap_test("test_symlinks.sh") add_oscap_test("test_validation_of_various_oval_versions.sh") + add_oscap_test("test_negative_instance.sh") endif() diff --git a/tests/probes/textfilecontent54/test_negative_instance.sh b/tests/probes/textfilecontent54/test_negative_instance.sh new file mode 100755 index 0000000000..960664fb15 --- /dev/null +++ b/tests/probes/textfilecontent54/test_negative_instance.sh @@ -0,0 +1,37 @@ +#!/usr/bin/env bash + +. $builddir/tests/test_common.sh + +function test_negative_instance { + + probecheck "textfilecontent54" || return 255 + + local ret_val=0; + local DF="${srcdir}/test_negative_instance.xml" + local RF="results.xml" + + [ -f $RF ] && rm -f $RF + + local FILE_A="/tmp/test_negative_instance.tmp_file" + + touch "$FILE_A" + + echo "valid_key = valid_value" > "$FILE_A" + echo "valid_key = valid_value" >> "$FILE_A" + echo "valid_key = valid_value" >> "$FILE_A" + + $OSCAP oval eval --results $RF $DF + + if [ -f $RF ]; then + verify_results "tst" $DF $RF 13 && verify_results "def" $DF $RF 1 + ret_val=$? + else + ret_val=1 + fi + + rm -f $FILE_A $FILE_B $FILE_C + + return $ret_val +} + +test_negative_instance diff --git a/tests/probes/textfilecontent54/test_negative_instance.xml b/tests/probes/textfilecontent54/test_negative_instance.xml new file mode 100644 index 0000000000..bb227e7175 --- /dev/null +++ b/tests/probes/textfilecontent54/test_negative_instance.xml @@ -0,0 +1,180 @@ + + + + + 5.11.1 + 2025-03-24T00:00:00-00:00 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + /tmp + test_negative_instance.tmp_file + (\s)*valid_key(\s)*=(\s)*valid_value(\s)* + 1 + + + + /tmp + test_negative_instance.tmp_file + (\s)*valid_key(\s)*=(\s)*valid_value(\s)* + 2 + + + + /tmp + test_negative_instance.tmp_file + (\s)*valid_key(\s)*=(\s)*valid_value(\s)* + 3 + + + + /tmp + test_negative_instance.tmp_file + (\s)*valid_key(\s)*=(\s)*valid_value(\s)* + -1 + + + + /tmp + test_negative_instance.tmp_file + (\s)*valid_key(\s)*=(\s)*valid_value(\s)* + -2 + + + + /tmp + test_negative_instance.tmp_file + (\s)*valid_key(\s)*=(\s)*valid_value(\s)* + -3 + + + + /tmp + test_negative_instance.tmp_file + (\s)*valid_key(\s)*=(\s)*valid_value(\s)* + -1 + + + + /tmp + test_negative_instance.tmp_file + (\s)*valid_key(\s)*=(\s)*valid_value(\s)* + -2 + + + + /tmp + test_negative_instance.tmp_file + (\s)*valid_key(\s)*=(\s)*valid_value(\s)* + -3 + + + + /tmp + test_negative_instance.tmp_file + (\s)*valid_key(\s)*=(\s)*valid_value(\s)* + -1 + + + + /tmp + test_negative_instance.tmp_file + (\s)*valid_key(\s)*=(\s)*valid_value(\s)* + -2 + + + + /tmp + test_negative_instance.tmp_file + (\s)*valid_key(\s)*=(\s)*valid_value(\s)* + -3 + + + + + + From bf55247f32abe89b538eaac0c681ff138bc7ca44 Mon Sep 17 00:00:00 2001 From: Edgar Aguilar Date: Tue, 25 Mar 2025 10:18:32 -0600 Subject: [PATCH 3/3] Apply suggestions from code review Co-authored-by: Evgeny Kolesnikov --- src/OVAL/probes/independent/textfilecontent54_probe.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/OVAL/probes/independent/textfilecontent54_probe.c b/src/OVAL/probes/independent/textfilecontent54_probe.c index d92a2ab7c7..5fcc00f005 100644 --- a/src/OVAL/probes/independent/textfilecontent54_probe.c +++ b/src/OVAL/probes/independent/textfilecontent54_probe.c @@ -247,7 +247,7 @@ static int process_file(const char *prefix, const char *path, const char *file, SEXP_free(instance_value); for(cur_inst = 0; cur_inst < instance_count; cur_inst++){ - if(negative_instance_value) + if (negative_instance_value) next_inst = SEXP_number_newi_32(cur_inst - instance_count); else @@ -260,7 +260,7 @@ static int process_file(const char *prefix, const char *path, const char *file, SEXP_free(next_inst); - if (want_instance){ + if (want_instance) { int pic_ret = probe_item_collect(pfd->ctx, SEXP_list_nth(items, cur_inst + 1)); if (pic_ret == 2 || pic_ret == -1) { ret = -4;