From: Alberto Leiva Popper <ydahhrk@gmail.com>
Date: Sat, 24 Feb 2024 01:18:55 +0000 (-0600)
Subject: Fix bad usage of strtol()
X-Git-Tag: 1.6.2~43
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=6d7985b72fa3462d311aa9421e1342fcaa2deef6;p=thirdparty%2FFORT-validator.git

Fix bad usage of strtol()

Someone didn't RTFM.

1. Error code wasn't being catched.
2. Second argument was NULL, so the caller had no way of knowing
   whether the string was fully consumed.
2. The code wasn't making sure the input was a clean hex string.
3. The input string wasn't NULL-terminated.

In the end, I ditched strtol(). It's too dangerous and cluttered for
this purpose.
---

diff --git a/src/rrdp.c b/src/rrdp.c
index 7a522e9b..a8251b6e 100644
--- a/src/rrdp.c
+++ b/src/rrdp.c
@@ -318,44 +318,74 @@ parse_string(xmlTextReaderPtr reader, char const *attr)
 	return result;
 }
 
+static unsigned int
+hexchar2uint(xmlChar xmlchar)
+{
+	if ('0' <= xmlchar && xmlchar <= '9')
+		return xmlchar - '0';
+	if ('a' <= xmlchar && xmlchar <= 'f')
+		return xmlchar - 'a' + 10;
+	if ('A' <= xmlchar && xmlchar <= 'F')
+		return xmlchar - 'A' + 10;
+	return 32;
+}
+
+static int
+hexstr2sha256(xmlChar *hexstr, unsigned char **result)
+{
+	unsigned char *hash;
+	unsigned int digit;
+	size_t i;
+
+	if (xmlStrlen(hexstr) != 2 * SHA256_DIGEST_LENGTH)
+		return EINVAL;
+
+	hash = pmalloc(SHA256_DIGEST_LENGTH);
+
+	for (i = 0; i < SHA256_DIGEST_LENGTH; i++) {
+		digit = hexchar2uint(hexstr[2 * i]);
+		if (digit > 15)
+			goto fail;
+		hash[i] = digit << 4;
+
+		digit = hexchar2uint(hexstr[2 * i + 1]);
+		if (digit > 15)
+			goto fail;
+		hash[i] |= digit;
+	}
+
+	*result = hash;
+	return 0;
+
+fail:
+	free(hash);
+	return EINVAL;
+}
+
 static int
-parse_hex_string(xmlTextReaderPtr reader, hash_requirement hr, char const *attr,
+parse_hash(xmlTextReaderPtr reader, hash_requirement hr, char const *attr,
     unsigned char **result, size_t *result_len)
 {
-	xmlChar *xml_value;
-	unsigned char *tmp, *ptr;
-	char *xml_cur;
-	char buf[2];
-	size_t tmp_len;
-
-	xml_value = xmlTextReaderGetAttribute(reader, BAD_CAST attr);
-	if (xml_value == NULL)
+	xmlChar *xmlattr;
+	int error;
+
+	if (hr == HR_IGNORE)
+		return 0;
+
+	xmlattr = xmlTextReaderGetAttribute(reader, BAD_CAST attr);
+	if (xmlattr == NULL)
 		return (hr == HR_MANDATORY)
-		    ? pr_val_err("RRDP file: Couldn't find xml attribute '%s'", attr)
+		    ? pr_val_err("Tag is missing the '%s' attribute.", attr)
 		    : 0;
 
-	/* The rest of the checks are done at the schema */
-	if (xmlStrlen(xml_value) % 2 != 0) {
-		xmlFree(xml_value);
-		return pr_val_err("RRDP file: Attribute %s isn't a valid hex string",
-		    attr);
-	}
+	error = hexstr2sha256(xmlattr, result);
 
-	tmp_len = xmlStrlen(xml_value) / 2;
-	tmp = pzalloc(tmp_len);
+	xmlFree(xmlattr);
 
-	ptr = tmp;
-	xml_cur = (char *) xml_value;
-	while (ptr - tmp < tmp_len) {
-		memcpy(buf, xml_cur, 2);
-		*ptr = strtol(buf, NULL, 16);
-		xml_cur+=2;
-		ptr++;
-	}
-	xmlFree(xml_value);
-
-	*result = tmp;
-	(*result_len) = tmp_len;
+	if (error)
+		return pr_val_err("The '%s' xml attribute does not appear to be a SHA-256 hash.",
+		    attr);
+	*result_len = SHA256_DIGEST_LENGTH;
 	return 0;
 }
 
@@ -487,7 +517,7 @@ parse_file_metadata(xmlTextReaderPtr reader, struct rpki_uri *notif,
 	if (hr == HR_IGNORE)
 		return 0;
 
-	error = parse_hex_string(reader, hr, RRDP_ATTR_HASH, &meta->hash,
+	error = parse_hash(reader, hr, RRDP_ATTR_HASH, &meta->hash,
 	    &meta->hash_len);
 	if (error) {
 		uri_refput(meta->uri);
diff --git a/test/rrdp_test.c b/test/rrdp_test.c
index 9debbacc..18ef8368 100644
--- a/test/rrdp_test.c
+++ b/test/rrdp_test.c
@@ -39,6 +39,46 @@ MOCK_ABORT_PTR(validation_tal, tal, struct validation *state)
 
 /* Mocks end */
 
+START_TEST(test_hexstr2sha256)
+{
+	char *hex;
+	unsigned char *sha = NULL;
+	unsigned int i;
+
+	hex = "01";
+	ck_assert_int_eq(EINVAL, hexstr2sha256((xmlChar *) hex, &sha));
+	ck_assert_ptr_eq(NULL, sha);
+
+	hex = "000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f";
+	ck_assert_int_eq(0, hexstr2sha256((xmlChar *) hex, &sha));
+	ck_assert_ptr_ne(NULL, sha);
+	for (i = 0; i < 32; i++)
+		ck_assert_uint_eq(i, sha[i]);
+	free(sha);
+	sha = NULL;
+
+	hex = "0x0102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f";
+	ck_assert_int_eq(EINVAL, hexstr2sha256((xmlChar *) hex, &sha));
+	ck_assert_ptr_eq(NULL, sha);
+
+	hex = " 00102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f";
+	ck_assert_int_eq(EINVAL, hexstr2sha256((xmlChar *) hex, &sha));
+	ck_assert_ptr_eq(NULL, sha);
+
+	hex = "0001020g0405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f";
+	ck_assert_int_eq(EINVAL, hexstr2sha256((xmlChar *) hex, &sha));
+	ck_assert_ptr_eq(NULL, sha);
+
+	hex = "0001020g0405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1";
+	ck_assert_int_eq(EINVAL, hexstr2sha256((xmlChar *) hex, &sha));
+	ck_assert_ptr_eq(NULL, sha);
+
+	hex = "0001020g0405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f2";
+	ck_assert_int_eq(EINVAL, hexstr2sha256((xmlChar *) hex, &sha));
+	ck_assert_ptr_eq(NULL, sha);
+}
+END_TEST
+
 #define END 0xFFFF
 
 static int
@@ -144,13 +184,14 @@ END_TEST
 static Suite *xml_load_suite(void)
 {
 	Suite *suite;
-	TCase *validate;
+	TCase *misc;
 
-	validate = tcase_create("Validate");
-	tcase_add_test(validate, test_sort_deltas);
+	misc = tcase_create("misc");
+	tcase_add_test(misc, test_hexstr2sha256);
+	tcase_add_test(misc, test_sort_deltas);
 
-	suite = suite_create("xml_test()");
-	suite_add_tcase(suite, validate);
+	suite = suite_create("RRDP");
+	suite_add_tcase(suite, misc);
 
 	return suite;
 }