XCCDF/OVAL/CPE/DS: fix NULL-pointer dereferences on malformed input

src/DS/ds_rds_session.c

@@ -106,6 +106,11 @@ int ds_rds_session_set_target_dir(struct ds_rds_session *session, const char *ta
 
 int ds_rds_session_register_component_source(struct ds_rds_session *session, const char *content_id, struct oscap_source *component)
 {
+	if (content_id == NULL) {
+		// A report/asset with no id cannot be used as a hash-table key.
+		oscap_seterr(OSCAP_EFAMILY_OSCAP, "Cannot register a Result DataStream component without an id.");
+		return -1;
+	}
 	if (!oscap_htable_add(session->component_sources, content_id, component)) {
 		oscap_seterr(OSCAP_EFAMILY_OSCAP, "Content '%s' has already been register with Result DataStream session: %s",
 				content_id, oscap_source_readable_origin(session->source));

src/DS/rds.c

@@ -245,7 +245,7 @@ static xmlNodePtr ds_rds_add_ai_from_xccdf_results(xmlDocPtr doc, xmlNodePtr ass
 		char* id_candidate = oscap_sprintf("asset%i", suffix);
 		xmlChar* id = xmlGetProp(child_asset, BAD_CAST "id");
 
-		if (strcmp(id_candidate, (const char*)id) == 0)
+		if (oscap_strcmp(id_candidate, (const char*)id) == 0)
 		{
 			suffix++;
 		}
@@ -392,8 +392,8 @@ static int ds_rds_report_inject_ai_target_id_ref(xmlDocPtr doc, xmlNodePtr test_
 				xmlChar* system_attr = xmlGetProp(duplicate_candidate, BAD_CAST "system");
 				xmlChar* name_attr = xmlGetProp(duplicate_candidate, BAD_CAST "name");
 
-				if (strcmp((const char*)system_attr, ai_ns_uri) == 0 &&
-					strcmp((const char*)name_attr, asset_id) == 0) {
+				if (oscap_strcmp((const char*)system_attr, ai_ns_uri) == 0 &&
+					oscap_strcmp((const char*)name_attr, asset_id) == 0) {
 
 					xmlFree(system_attr);
 					xmlFree(name_attr);

src/DS/sds_index.c

@@ -246,7 +246,7 @@
 	while (ds_stream_index_iterator_has_more(streams))
 	{
 		struct ds_stream_index* stream = ds_stream_index_iterator_next(streams);
-		if (strcmp(ds_stream_index_get_id(stream), stream_id) == 0)
+		if (oscap_strcmp(ds_stream_index_get_id(stream), stream_id) == 0)
 		{
 			ret = stream;
 			break;
@@ -419,20 +419,23 @@
 
 	int ret = 1;
 
+	if (s == NULL)
+		return ret;
+
 	struct ds_stream_index_iterator* streams_it = ds_sds_index_get_streams(s);
 	while (ds_stream_index_iterator_has_more(streams_it))
 	{
 		struct ds_stream_index* stream_idx = ds_stream_index_iterator_next(streams_it);
 		const char* stream_id = ds_stream_index_get_id(stream_idx);
 
-		if (!*datastream_id || strcmp(stream_id, *datastream_id) == 0)
+		if (!*datastream_id || oscap_strcmp(stream_id, *datastream_id) == 0)
 		{
 			struct oscap_string_iterator* checklists_it = ds_stream_index_get_checklists(stream_idx);
 			while (oscap_string_iterator_has_more(checklists_it))
 			{
 				const char* checklist_id = oscap_string_iterator_next(checklists_it);
 
-				if (!*component_id || strcmp(checklist_id, *component_id) == 0)
+				if (!*component_id || oscap_strcmp(checklist_id, *component_id) == 0)
 				{
 					*component_id = checklist_id;
 					*datastream_id = ds_stream_index_get_id(stream_idx);
@@ -451,6 +454,9 @@
 int ds_sds_index_select_checklist_by_benchmark_id(struct ds_sds_index* s,
 		const char *benchmark_id, const char **datastream_id, const char **component_ref_id)
 {
+	if (s == NULL)
+		return 1;
+
 	const char *mapped_component_id = (const char*)oscap_htable_get(s->benchmark_id_to_component_id, benchmark_id);
 	if (!mapped_component_id) {
 		oscap_seterr(OSCAP_EFAMILY_XML, "Can't map benchmark ID '%s' to any component ID.", benchmark_id);
@@ -465,7 +471,7 @@
 		struct ds_stream_index *stream_idx = ds_stream_index_iterator_next(streams_it);
 		const char *stream_id = ds_stream_index_get_id(stream_idx);
 
-		if (!*datastream_id || strcmp(stream_id, *datastream_id) == 0)
+		if (!*datastream_id || oscap_strcmp(stream_id, *datastream_id) == 0)
 		{
 			const char *candidate_component_ref_id = (const char*)oscap_htable_get(stream_idx->component_id_to_component_ref_id, mapped_component_id);
 			if (candidate_component_ref_id) {

src/OVAL/oval_definition.c

@@ -438,7 +438,9 @@ int oval_definition_parse_tag(xmlTextReaderPtr reader, struct oval_parser_contex
 	id = NULL;
 
 	char *version = (char *)xmlTextReaderGetAttribute(reader, BAD_CAST "version");
-	oval_definition_set_version(definition, atoi(version));
+	if (version != NULL) {
+		oval_definition_set_version(definition, atoi(version));
+	}
 	free(version);
 	version = NULL;
 

src/OVAL/oval_directives.c

@@ -364,7 +364,7 @@ int oval_result_directives_parse_tag(xmlTextReaderPtr reader, struct oval_parser
 	if ( (int) type != OVAL_ENUMERATION_INVALID) {
 		/*reported */
 		xmlChar *boolstr = xmlTextReaderGetAttribute(reader, BAD_CAST "reported");
-		bool reported = (strcmp((const char *)boolstr, "1") == 0) || (strcmp((const char *)boolstr, "true") == 0);
+		bool reported = oscap_streq((const char *)boolstr, "1") || oscap_streq((const char *)boolstr, "true");
 		free(boolstr);
 		oval_result_directives_set_reported(directives, type, reported);
 

src/OVAL/oval_object.c

@@ -366,7 +366,9 @@ int oval_object_parse_tag(xmlTextReaderPtr reader, struct oval_parser_context *c
 	oval_object_set_deprecated(object, deprecated);
 
 	version = (char *)xmlTextReaderGetAttribute(reader, BAD_CAST "version");
-	oval_object_set_version(object, atoi(version));
+	if (version != NULL) {
+		oval_object_set_version(object, atoi(version));
+	}
 
 	ret = oval_parser_parse_tag(reader, context, &_oval_object_parse_tag, object);
 

src/OVAL/oval_state.c

@@ -315,7 +315,9 @@ int oval_state_parse_tag(xmlTextReaderPtr reader, struct oval_parser_context *co
 	int deprecated = oval_parser_boolean_attribute(reader, "deprecated", 0);
 	oval_state_set_deprecated(state, deprecated);
 	char *version = (char *)xmlTextReaderGetAttribute(reader, BAD_CAST "version");
-	oval_state_set_version(state, atoi(version));
+	if (version != NULL) {
+		oval_state_set_version(state, atoi(version));
+	}
 	free(version);
 	oval_operator_t operator = oval_operator_parse(reader, "operator", OVAL_OPERATOR_AND);
 	oval_state_set_operator(state, operator);

src/OVAL/oval_varModel.c

@@ -194,8 +194,12 @@ static int _oval_variable_model_parse_variable_values
 
 		return_code = xmlTextReaderRead(reader);
 		char *value = (char *)xmlTextReaderValue(reader);
-		ov = oval_value_new(frame->datatype, value);
-		oval_collection_add(frame->values, ov);
+		// frame is NULL when this variable's id duplicated an earlier one
+		// (see _oval_variable_model_parse_variable); ignore its values.
+		if (frame != NULL) {
+			ov = oval_value_new(frame->datatype, value);
+			oval_collection_add(frame->values, ov);
+		}
 		free(value);
 	} else {
 		dW("Unprocessed tag: <%s:%s>.", namespace, tagname);

src/OVAL/oval_variable.c

@@ -1129,7 +1129,9 @@ int oval_variable_parse_tag(xmlTextReaderPtr reader, struct oval_parser_context
 	oval_variable_set_deprecated(variable, deprecated);
 
 	version = (char *)xmlTextReaderGetAttribute(reader, BAD_CAST "version");
-	oval_variable_set_version(variable, atoi(version));
+	if (version != NULL) {
+		oval_variable_set_version(variable, atoi(version));
+	}
 
 	oval_datatype_t datatype = oval_datatype_parse(reader, "datatype", OVAL_DATATYPE_UNKNOWN);
 	oval_variable_set_datatype(variable, datatype);

src/OVAL/results/oval_resultDefinition.c

@@ -257,7 +257,7 @@ int oval_result_definition_parse_tag(xmlTextReaderPtr reader, struct oval_parser
 	struct oval_result_definition *definition;
 	xmlChar *definition_id = xmlTextReaderGetAttribute(reader, BAD_CAST "definition_id");
 	xmlChar *definition_version = xmlTextReaderGetAttribute(reader, BAD_CAST "version");
-	int resvsn = atoi((char *)definition_version);
+	int resvsn = (definition_version != NULL) ? atoi((char *)definition_version) : 0;
 
 	oval_result_t result = oval_result_parse(reader, "result", OVAL_ENUMERATION_INVALID);
 

src/XCCDF/benchmark.c

@@ -451,6 +451,10 @@ XCCDF_ITEM_ADDER_REG(benchmark, profile, profiles)
 
 bool xccdf_benchmark_add_result(struct xccdf_benchmark *benchmark, struct xccdf_result *item)
 {
+	if (item == NULL) {
+		// xccdf_result_new_parse() returns NULL on a malformed TestResult.
+		return false;
+	}
 	const char *id = xccdf_result_get_id(item);
 	if (id != NULL) {
 		// Resolve possible conflicts of the IDs in the list of TestResults.

src/XCCDF/item.c

@@ -760,6 +760,11 @@ bool xccdf_item_process_attributes(struct xccdf_item *item, xmlTextReaderPtr rea
 void xccdf_item_add_applicable_platform(struct xccdf_item *item, xmlTextReaderPtr reader)
 {
 	char *platform_idref = xccdf_attribute_copy(reader, XCCDFA_IDREF);
+	if (platform_idref == NULL) {
+		// A <platform> with no @idref has nothing to reference; ignore it
+		// rather than passing NULL to the regex/strlen below.
+		return;
+	}
 
 	/* Official Windows 7 CPE according to National Vulnerability Database
 	 * CPE Dictionary as of 2018-08-29 is 'cpe:/o:microsoft:windows_7'.

src/XCCDF/result.c

@@ -1453,7 +1453,13 @@ static struct xccdf_score *xccdf_score_new_parse(xmlTextReaderPtr reader)
 	if (xccdf_attribute_has(reader, XCCDFA_MAXIMUM))
 		score->maximum = xccdf_attribute_get_float(reader, XCCDFA_MAXIMUM);
 	else score->maximum = XCCDF_SCORE_MAX_DAFAULT;
-	score->score = atof(oscap_element_string_get(reader));
+	const char *score_str = oscap_element_string_get(reader);
+	if (score_str == NULL) {
+		dW("Empty <score> element is invalid, rejecting.");
+		xccdf_score_free(score);
+		return NULL;
+	}
+	score->score = atof(score_str);
 	return score;
 }
 

src/XCCDF/tailoring.c

@@ -81,6 +81,10 @@ void xccdf_tailoring_free(struct xccdf_tailoring *tailoring)
 
 bool xccdf_tailoring_add_profile(struct xccdf_tailoring *tailoring, struct xccdf_profile *profile)
 {
+	if (profile == NULL) {
+		// xccdf_profile_parse() returns NULL on a malformed <Profile>.
+		return false;
+	}
 	xccdf_profile_set_tailoring(profile, true);
 	return oscap_list_add(tailoring->profiles, XITEM(profile));
 }

src/XCCDF/value.c

@@ -143,14 +143,26 @@
 			break;
 		case XCCDFE_LOWER_BOUND:
 			if (type == XCCDF_TYPE_NUMBER) {
+				const char *lb = oscap_element_string_get(reader);
+				if (lb == NULL) {
+					dW("Empty <lower-bound> element is invalid, rejecting <Value>.");
+					xccdf_value_free(value);
+					return NULL;
+				}
 				val = _xccdf_value_find_or_create_instance(XVALUE(value), selector, type);
-				val->lower_bound = atof(oscap_element_string_get(reader));
+				val->lower_bound = atof(lb);
 			}
 			break;
 		case XCCDFE_UPPER_BOUND:
 			if (type == XCCDF_TYPE_NUMBER) {
+				const char *ub = oscap_element_string_get(reader);
+				if (ub == NULL) {
+					dW("Empty <upper-bound> element is invalid, rejecting <Value>.");
+					xccdf_value_free(value);
+					return NULL;
+				}
 				val = _xccdf_value_find_or_create_instance(XVALUE(value), selector, type);
-				val->upper_bound = atof(oscap_element_string_get(reader));
+				val->upper_bound = atof(ub);
 			}
 			break;
 		case XCCDFE_CHOICES:

src/common/util.h

@@ -343,13 +343,17 @@ static inline bool oscap_streq(const char *s1, const char *s2) {
 	return oscap_strcmp(s1, s2) == 0;
 }
 
-/// Check whether str starts with "prefix"
+/// Check whether str starts with "prefix" (a NULL str never matches)
 static inline bool oscap_str_startswith(const char *str, const char *prefix) {
+	if (str == NULL || prefix == NULL)
+		return false;
 	return strncmp(str, prefix, strlen(prefix)) == 0;
 }
 
-/// Check whether str ends with "suffix"
+/// Check whether str ends with "suffix" (a NULL str never matches)
 static inline bool oscap_str_endswith(const char *str, const char *suffix) {
+	if (str == NULL || suffix == NULL)
+		return false;
 	const size_t str_len = strlen(str);
 	const size_t suffix_len = strlen(suffix);
 	if (suffix_len > str_len)

tests/API/OVAL/unittests/CMakeLists.txt

@@ -1,4 +1,5 @@
 add_oscap_test("test_anyxml.sh")
+add_oscap_test("test_null_ptr_regression.sh")
 add_oscap_test("test_applicability_check.sh")
 add_oscap_test("test_cim_datetime.sh")
 add_oscap_test("test_circular_extend_def.sh")

tests/API/OVAL/unittests/test_null_ptr_regression.oval.xml

@@ -0,0 +1 @@
+<oval_definitions xmlns="http://oval.mitre.org/XMLSchema/oval-definitions-5"><states><variable_state></variable_state></states></oval_definitions>

tests/API/OVAL/unittests/test_null_ptr_regression.sh

@@ -0,0 +1,17 @@
+#!/usr/bin/env bash
+# Regression test for NULL-pointer dereferences on malformed OVAL input.
+# Each case used to crash (SIGSEGV) before the NULL guards were added.
+
+. $builddir/tests/test_common.sh
+
+result=0
+
+# Missing version attribute on a state caused atoi(NULL) in oval_state_parse.
+assert_no_crash "oval_state version=NULL" \
+    info "$srcdir/test_null_ptr_regression.oval.xml" || result=1
+
+# Duplicate variable ID yielded a NULL varModel frame, then dereferenced it.
+assert_no_crash "oval_varModel duplicate ID" \
+    info "$srcdir/test_null_ptr_regression_varmodel.oval_variables.xml" || result=1
+
+exit $result

tests/API/OVAL/unittests/test_null_ptr_regression_varmodel.oval_variables.xml

@@ -0,0 +1 @@
+<oval_variables xmlns="http://oval.mitre.org/XMLSchema/oval-variables-5"><variables><variable id="oval:x:var:3"></variable> <variable id="oval:x:var:3"><value></value></variable></variables></oval_variables>

tests/API/XCCDF/unittests/CMakeLists.txt

@@ -120,3 +120,4 @@ add_oscap_test("test_single_line_tailoring.sh")
 add_oscap_test("test_reference.sh")
 add_oscap_test("test_remediation_bootc.sh")
 add_oscap_test("openscap_2289_regression.sh")
+add_oscap_test("test_null_ptr_regression.sh")

tests/API/XCCDF/unittests/test_null_ptr_regression.sh

@@ -0,0 +1,21 @@
+#!/usr/bin/env bash
+# Regression test for NULL-pointer dereferences on malformed XCCDF input.
+# Each case used to crash (SIGSEGV) before the NULL guards were added.
+
+. $builddir/tests/test_common.sh
+
+result=0
+
+# Empty <TestResult> element caused NULL item dereference in xccdf_benchmark_add_result.
+assert_no_crash "XCCDF empty TestResult" \
+    info "$srcdir/test_null_ptr_xccdf_empty_testresult.xml" || result=1
+
+# <platform> with no idref attribute caused strlen(NULL) in xccdf_item_add_applicable_platform.
+assert_no_crash "XCCDF platform missing idref" \
+    info "$srcdir/test_null_ptr_xccdf_platform_no_idref.xml" || result=1
+
+# Empty <Profile> in tailoring caused NULL dereference in xccdf_tailoring_add_profile.
+assert_no_crash "XCCDF tailoring empty profile" \
+    info "$srcdir/test_null_ptr_xccdf_tailoring_empty_profile.xml" || result=1
+
+exit $result

tests/API/XCCDF/unittests/test_null_ptr_xccdf_empty_testresult.xml

@@ -0,0 +1 @@
+<Benchmark xmlns="http://checklists.nist.gov/xccdf" id=""><TestResult></TestResult></Benchmark>

tests/API/XCCDF/unittests/test_null_ptr_xccdf_platform_no_idref.xml

@@ -0,0 +1 @@
+<Benchmark xmlns="http://checklists.nist.gov/xccdf" id=""><platform/></Benchmark>

tests/API/XCCDF/unittests/test_null_ptr_xccdf_tailoring_empty_profile.xml

@@ -0,0 +1 @@
+<Tailoring xmlns="http://checklists.nist.gov/xccdf"><Profile></Profile></Tailoring>

tests/DS/CMakeLists.txt

@@ -1,4 +1,5 @@
 add_oscap_test("test_ds_misc.sh")
+add_oscap_test("test_null_ptr_regression.sh")
 add_oscap_test("test_rds.sh")
 add_oscap_test("test_sds_eval.sh")
 add_oscap_test("test_sds_fix_from_results.sh")

tests/DS/test_null_ptr_rds_missing_rel_type.xml

@@ -0,0 +1 @@
+<arf:asset-report-collection xmlns:arf="." xmlns:core="c"><core:relationships><core:relationship></core:relationship></core:relationships></arf:asset-report-collection>

tests/DS/test_null_ptr_regression.sh

@@ -0,0 +1,29 @@
+#!/usr/bin/env bash
+# Regression test for NULL-pointer dereferences on malformed DS/ARF/SDS input.
+# Each case used to crash (SIGSEGV) before the NULL guards were added.
+
+. $builddir/tests/test_common.sh
+
+result=0
+
+# Relationship element with no type attribute caused strcmp(NULL, ...) in rds.c /
+# strncmp(NULL, ...) via oscap_str_startswith in util.h.
+assert_no_crash "RDS relationship missing type" \
+    info "$srcdir/test_null_ptr_rds_missing_rel_type.xml" || result=1
+
+# Empty data-stream-collection causes ds_sds_index_new_from_source to return NULL
+# (no streams found), and xccdf_session_load_xccdf then calls
+# ds_sds_index_select_checklist(NULL, ...) which dereferences the NULL index.
+# --skip-valid bypasses the early schema-validation return in xccdf_session_load_xccdf
+# so that ds_sds_session_select_checklist is actually reached.
+assert_no_crash "SDS empty collection" \
+    xccdf eval --skip-valid "$srcdir/test_null_ptr_sds_empty_collection.xml" || result=1
+
+# data-stream without id caused strcmp(NULL, datastream_id) in sds_index.c when
+# a non-NULL datastream-id is requested. --skip-valid bypasses early validation
+# return so that ds_sds_session_select_checklist is actually reached.
+assert_no_crash "SDS stream missing id" \
+    xccdf eval --skip-valid --datastream-id "regression-test-id" \
+    "$srcdir/test_null_ptr_sds_stream_no_id.xml" || result=1
+
+exit $result

tests/DS/test_null_ptr_sds_empty_collection.xml

@@ -0,0 +1 @@
+<data-stream-collection/>

tests/DS/test_null_ptr_sds_stream_no_id.xml

@@ -0,0 +1,6 @@
+<data-stream-collection xmlns="http://scap.nist.gov/schema/scap/source/1.2" xmlns:xlink="http://www.w3.org/1999/xlink" id="c">
+<data-stream scap-version="1.3" use-case="OTHER">
+<checklists><component-ref id="r" xlink:href="#a"/></checklists>
+</data-stream>
+<component id="a"><Benchmark xmlns="http://checklists.nist.gov/xccdf/1.2" id="b"/></component>
+</data-stream-collection>

tests/test_common.sh.in

@@ -413,5 +413,23 @@ clean_dbus_mock() {
 
 export -f assert_exists
 
+# Run $OSCAP with the given args and fail if the process crashed.
+# Handles both ASan builds (exit 1 + "AddressSanitizer" / "runtime error:" in
+# stderr) and plain builds (exit >= 128 from SIGSEGV/SIGABRT).
+assert_no_crash() {
+	local desc="$1"
+	local stderr_file
+	stderr_file=$(mktemp)
+	trap 'rm -f "$stderr_file"' RETURN
+	local ret=0
+	$OSCAP "${@:2}" > /dev/null 2>"$stderr_file" || ret=$?
+	if [ "$ret" -ge 128 ] || grep -q "AddressSanitizer:DEADLYSIGNAL\|ERROR: AddressSanitizer:\|runtime error:" "$stderr_file"; then
+		echo "CRASH detected in: $desc (exit $ret)"
+		return 1
+	fi
+}
+
+export -f assert_no_crash
+
 export OPENSCAP_ENABLE_MD5="@OPENSCAP_ENABLE_MD5@"
 export OPENSCAP_ENABLE_SHA1="@OPENSCAP_ENABLE_SHA1@"