diff --git a/src/OVAL/results/oval_resultCriteriaNode.c b/src/OVAL/results/oval_resultCriteriaNode.c index 5ddd20adf3..8072832062 100644 --- a/src/OVAL/results/oval_resultCriteriaNode.c +++ b/src/OVAL/results/oval_resultCriteriaNode.c @@ -234,8 +234,10 @@ void oval_result_criteria_node_free(struct oval_result_criteria_node *node) free(node); } -struct oval_result_criteria_node *make_result_criteria_node_from_oval_criteria_node - (struct oval_result_system *sys, struct oval_criteria_node *oval_node, int variable_instance) { +struct oval_result_criteria_node *make_result_criteria_node_from_oval_criteria_node( + struct oval_result_system *sys, struct oval_criteria_node *oval_node, + struct oscap_list *visited_definitions, int variable_instance) +{ struct oval_result_criteria_node *rslt_node = NULL; if (oval_node) { oval_criteria_node_type_t type = oval_criteria_node_get_type(oval_node); @@ -255,7 +257,9 @@ struct oval_result_criteria_node *make_result_criteria_node_from_oval_criteria_n struct oval_criteria_node *oval_subnode = oval_criteria_node_iterator_next(oval_subnodes); struct oval_result_criteria_node *rslt_subnode - = make_result_criteria_node_from_oval_criteria_node(sys, oval_subnode, variable_instance); + = make_result_criteria_node_from_oval_criteria_node(sys, oval_subnode, visited_definitions, variable_instance); + if (rslt_subnode == NULL) + return NULL; oval_result_criteria_node_add_subnode(rslt_node, rslt_subnode); } oval_criteria_node_iterator_free(oval_subnodes); @@ -272,7 +276,9 @@ struct oval_result_criteria_node *make_result_criteria_node_from_oval_criteria_n case OVAL_NODETYPE_EXTENDDEF:{ struct oval_definition *oval_definition = oval_criteria_node_get_definition(oval_node); struct oval_result_definition *rslt_definition - = oval_result_system_get_new_definition(sys, oval_definition, variable_instance); + = oval_result_system_get_new_definition_with_check(sys, oval_definition, visited_definitions, variable_instance); + if (rslt_definition == NULL) + return NULL; rslt_node = oval_result_criteria_node_new(sys, type, negate, diff --git a/src/OVAL/results/oval_resultDefinition.c b/src/OVAL/results/oval_resultDefinition.c index 9ea0c69257..a5db1a5acb 100644 --- a/src/OVAL/results/oval_resultDefinition.c +++ b/src/OVAL/results/oval_resultDefinition.c @@ -113,14 +113,21 @@ void oval_result_definition_free(struct oval_result_definition *definition) free(definition); } -struct oval_result_definition *make_result_definition_from_oval_definition - (struct oval_result_system *sys, struct oval_definition *oval_definition, int variable_instance) { +struct oval_result_definition *make_result_definition_from_oval_definition(struct oval_result_system *sys, struct oval_definition *oval_definition, struct oscap_list *visited_definitions, int variable_instance) +{ char *defid = oval_definition_get_id(oval_definition); + if (visited_definitions != NULL) { + if (oscap_list_contains(visited_definitions, defid, (oscap_cmp_func) oscap_streq)) { + dE("Circular dependency in OVAL definition '%s'.", defid); + return NULL; + } + oscap_list_add(visited_definitions, strdup(defid)); + } struct oval_result_definition *rslt_definition = oval_result_definition_new(sys, defid); oval_result_definition_set_instance(rslt_definition, variable_instance); struct oval_criteria_node *oval_criteria = oval_definition_get_criteria(oval_definition); struct oval_result_criteria_node *rslt_criteria = - make_result_criteria_node_from_oval_criteria_node(sys, oval_criteria, variable_instance); + make_result_criteria_node_from_oval_criteria_node(sys, oval_criteria, visited_definitions, variable_instance); if (rslt_criteria) oval_result_definition_set_criteria(rslt_definition, rslt_criteria); return rslt_definition; diff --git a/src/OVAL/results/oval_resultSystem.c b/src/OVAL/results/oval_resultSystem.c index 6c12dc5603..e957ed4512 100644 --- a/src/OVAL/results/oval_resultSystem.c +++ b/src/OVAL/results/oval_resultSystem.c @@ -50,6 +50,7 @@ #include "common/debug_priv.h" #include "common/_error.h" #include "common/util.h" +#include "common/list.h" typedef struct oval_result_system { struct oval_results_model *model; @@ -195,6 +196,14 @@ struct oval_result_test *oval_result_system_get_test(struct oval_result_system * struct oval_result_definition *oval_result_system_get_new_definition (struct oval_result_system *sys, struct oval_definition *oval_definition, int variable_instance) { + + return oval_result_system_get_new_definition_with_check(sys, oval_definition, NULL, variable_instance); + +} + +struct oval_result_definition *oval_result_system_get_new_definition_with_check + (struct oval_result_system *sys, struct oval_definition *oval_definition, struct oscap_list *visited_definitions, int variable_instance) +{ // This function is used from multiple different places which might not be sustainable. // variable_instance=0 means that caller has no special preference for variable_instance // and the very last definition shall be returned. Additionally, we should create @@ -205,14 +214,18 @@ struct oval_result_definition *oval_result_system_get_new_definition char *id = oval_definition_get_id(oval_definition); rslt_definition = oval_result_system_get_definition(sys, id); if (rslt_definition == NULL) { - rslt_definition = make_result_definition_from_oval_definition(sys, oval_definition, + rslt_definition = make_result_definition_from_oval_definition(sys, oval_definition, visited_definitions, variable_instance ? variable_instance : 1); + if (rslt_definition == NULL) + return NULL; oval_result_system_add_definition(sys, rslt_definition); } else if (oval_result_definition_get_variable_instance_hint(rslt_definition) != oval_result_definition_get_instance(rslt_definition)) { int hint = oval_result_definition_get_variable_instance_hint(rslt_definition); dI("Creating another result-definition for id=%s based on variable_instance: %d", id, hint); - rslt_definition = make_result_definition_from_oval_definition(sys, oval_definition, hint); + rslt_definition = make_result_definition_from_oval_definition(sys, oval_definition, visited_definitions, hint); + if (rslt_definition == NULL) + return NULL; oval_result_system_add_definition(sys, rslt_definition); } } @@ -376,17 +389,24 @@ struct oval_result_definition *oval_result_system_prepare_definition(struct oval return NULL; } + struct oscap_list *visited_definitions = oscap_list_new(); rslt_definition = oval_result_system_get_definition(sys, id); if (rslt_definition == NULL) { - rslt_definition = make_result_definition_from_oval_definition(sys, oval_definition, 1); + rslt_definition = make_result_definition_from_oval_definition(sys, oval_definition, visited_definitions, 1); + if (rslt_definition == NULL) + goto cleanup; oval_result_system_add_definition(sys, rslt_definition); } else if (oval_result_definition_get_variable_instance_hint(rslt_definition) != oval_result_definition_get_instance(rslt_definition)) { int hint = oval_result_definition_get_variable_instance_hint(rslt_definition); dI("Creating another result-definition for id=%s based on variable_instance: %d", id, hint); - rslt_definition = make_result_definition_from_oval_definition(sys, oval_definition, hint); + rslt_definition = make_result_definition_from_oval_definition(sys, oval_definition, visited_definitions, hint); + if (rslt_definition == NULL) + goto cleanup; oval_result_system_add_definition(sys, rslt_definition); } +cleanup: + oscap_list_free(visited_definitions, free); return rslt_definition; } diff --git a/src/OVAL/results/oval_results_impl.h b/src/OVAL/results/oval_results_impl.h index a6c0747613..cd9f728d2b 100644 --- a/src/OVAL/results/oval_results_impl.h +++ b/src/OVAL/results/oval_results_impl.h @@ -37,6 +37,7 @@ #include "OVAL/oval_system_characteristics_impl.h" #include "OVAL/adt/oval_smc_impl.h" +#include "common/list.h" #include "common/util.h" #include "source/oscap_source_priv.h" @@ -47,9 +48,9 @@ xmlNode *oval_result_system_to_dom(struct oval_result_system *, struct oval_resu struct oval_result_test *oval_result_system_get_new_test(struct oval_result_system *, struct oval_test *, int variable_instance); int oval_result_definition_parse_tag(xmlTextReaderPtr, struct oval_parser_context *, void *); -struct oval_result_definition *make_result_definition_from_oval_definition(struct oval_result_system *, - struct oval_definition *, - int variable_instance); +struct oval_result_definition *make_result_definition_from_oval_definition( + struct oval_result_system *, struct oval_definition *, + struct oscap_list *, int variable_instance); xmlNode *oval_result_definition_to_dom(struct oval_result_definition *, oval_result_directive_content_t, xmlDocPtr, xmlNode *); int oval_result_definition_get_variable_instance_hint(const struct oval_result_definition *definition); void oval_result_definition_set_variable_instance_hint(struct oval_result_definition *definition, int new_hint_value); @@ -63,7 +64,7 @@ xmlNode *oval_result_test_to_dom(struct oval_result_test *, xmlDocPtr, xmlNode * int oval_result_item_parse_tag(xmlTextReaderPtr, struct oval_parser_context *, struct oval_result_system *, oscap_consumer_func, void *); xmlNode *oval_result_item_to_dom(struct oval_result_item *, xmlDocPtr, xmlNode *); -struct oval_result_criteria_node *make_result_criteria_node_from_oval_criteria_node(struct oval_result_system *, struct oval_criteria_node *, int variable_instance); +struct oval_result_criteria_node *make_result_criteria_node_from_oval_criteria_node(struct oval_result_system *, struct oval_criteria_node *, struct oscap_list *, int variable_instance); int oval_result_criteria_node_parse(xmlTextReaderPtr, struct oval_parser_context *, struct oval_result_system *, oscap_consumer_func, void *); oval_result_t oval_result_criteria_node_negate(struct oval_result_criteria_node *node, oval_result_t result); @@ -74,6 +75,8 @@ oval_result_t oval_result_parse(xmlTextReaderPtr, char *, oval_result_t); struct oval_result_definition *oval_result_system_get_new_definition(struct oval_result_system *, struct oval_definition *, int variable_instance); +struct oval_result_definition *oval_result_system_get_new_definition_with_check(struct oval_result_system *, struct oval_definition *, struct oscap_list *, int); + struct oval_result_test *oval_result_system_get_test(struct oval_result_system *, char *); struct oresults { diff --git a/tests/API/OVAL/unittests/CMakeLists.txt b/tests/API/OVAL/unittests/CMakeLists.txt index 5b1e26aa92..9ec019f764 100644 --- a/tests/API/OVAL/unittests/CMakeLists.txt +++ b/tests/API/OVAL/unittests/CMakeLists.txt @@ -1,6 +1,7 @@ add_oscap_test("test_anyxml.sh") add_oscap_test("test_applicability_check.sh") add_oscap_test("test_cim_datetime.sh") +add_oscap_test("test_circular_extend_def.sh") add_oscap_test("test_comment.sh") add_oscap_test("test_count_function.sh") add_oscap_test("test_deprecated_def.sh") @@ -24,6 +25,7 @@ add_oscap_test("test_item_not_exist.sh") add_oscap_test("test_object_component_type.sh") add_oscap_test("test_oval_empty_variable_evaluation.sh") add_oscap_test("test_platform_version.sh") +add_oscap_test("test_recursive_extend_def.sh") add_oscap_test("test_skip_valid.sh") add_oscap_test("test_state_check_existence.sh") add_oscap_test("test_without_syschars.sh") diff --git a/tests/API/OVAL/unittests/test_circular_extend_def.sh b/tests/API/OVAL/unittests/test_circular_extend_def.sh new file mode 100755 index 0000000000..32b263658b --- /dev/null +++ b/tests/API/OVAL/unittests/test_circular_extend_def.sh @@ -0,0 +1,39 @@ +#!/usr/bin/env bash + +. $builddir/tests/test_common.sh +set -e +set -o pipefail + +stdout="$(mktemp)" +stderr="$(mktemp)" + +$OSCAP oval eval --id "oval:x:def:1" $srcdir/test_circular_extend_def.xml > "$stdout" 2> "$stderr" + +grep -q "Definition oval:x:def:1: not evaluated" "$stdout" +grep -q "Circular dependency in OVAL definition 'oval:x:def:1'\." "$stderr" + +rm -f "$stdout" +rm -f "$stderr" + +stdout="$(mktemp)" +stderr="$(mktemp)" + +$OSCAP oval eval --id "oval:x:def:2" $srcdir/test_circular_extend_def.xml > "$stdout" 2> "$stderr" + +grep -q "Definition oval:x:def:2: not evaluated" "$stdout" +grep -q "Circular dependency in OVAL definition 'oval:x:def:2'\." "$stderr" + +rm -f "$stdout" +rm -f "$stderr" + +stdout="$(mktemp)" +stderr="$(mktemp)" + +$OSCAP oval eval $srcdir/test_circular_extend_def.xml > "$stdout" 2> "$stderr" + +grep -q "Definition oval:x:def:1: not evaluated" "$stdout" +grep -q "Definition oval:x:def:2: not evaluated" "$stdout" +grep -q "Circular dependency in OVAL definition .*" "$stderr" + +rm -f "$stdout" +rm -f "$stderr" diff --git a/tests/API/OVAL/unittests/test_circular_extend_def.xml b/tests/API/OVAL/unittests/test_circular_extend_def.xml new file mode 100644 index 0000000000..c745a0bd89 --- /dev/null +++ b/tests/API/OVAL/unittests/test_circular_extend_def.xml @@ -0,0 +1,40 @@ + + + + combine_ovals.py from SCAP Security Guide + ssg: [0, 1, 49], python: 3.7.6 + 5.11 + 2020-03-11T10:34:56 + + + + + Definition 1 + Definition 1 + + + + + + + + Definition 2 + Definition 2 + + + + + + + + + + + + + + + /tmp/xxx + + + diff --git a/tests/API/OVAL/unittests/test_recursive_extend_def.sh b/tests/API/OVAL/unittests/test_recursive_extend_def.sh new file mode 100755 index 0000000000..b7e3fc2469 --- /dev/null +++ b/tests/API/OVAL/unittests/test_recursive_extend_def.sh @@ -0,0 +1,16 @@ +#!/usr/bin/env bash + +. $builddir/tests/test_common.sh +set -e +set -o pipefail + +stdout="$(mktemp)" +stderr="$(mktemp)" + +$OSCAP oval eval $srcdir/test_recursive_extend_def.xml > "$stdout" 2> "$stderr" + +grep -q "Definition oval:x:def:1: not evaluated" "$stdout" +grep -q "Circular dependency in OVAL definition 'oval:x:def:1'\." "$stderr" + +rm -f "$stdout" +rm -f "$stderr" diff --git a/tests/API/OVAL/unittests/test_recursive_extend_def.xml b/tests/API/OVAL/unittests/test_recursive_extend_def.xml new file mode 100644 index 0000000000..99d8613c2b --- /dev/null +++ b/tests/API/OVAL/unittests/test_recursive_extend_def.xml @@ -0,0 +1,40 @@ + + + + combine_ovals.py from SCAP Security Guide + ssg: [0, 1, 49], python: 3.7.6 + 5.11 + 2020-03-11T10:34:56 + + + + + Definition 1 + Definition 1 + + + + + + + + + Definition 2 + Definition 2 + + + + + + + + + + + + + + /tmp/xxx + + +