Fix preserve_sbat_uefi_variable() logic

preserve_sbat_uefi_variable() shoud really deal with the sbat metadata version as a numerical value that could gain more digits. It also needs to only compare the datestamp since the actual metadata can grow and shrink Signed-off-by: Jan Setje-Eilers <[email protected]>
rhboot · May 23, 2022 · 759f061 · 759f061
1 parent a78673b
commit 759f061
Show file tree

Hide file tree

Showing 2 changed files with 109 additions and 11 deletions.
diff --git a/sbat.c b/sbat.c
@@ -319,16 +319,53 @@ check_sbat_var_attributes(UINT32 attributes)
 #endif
 }
 
+static char *
+nth_sbat_field(char *str, int n)
+{
+	char *ret = str;
+	while (n > 0 && ret != NULL) {
+		if (ret[0] == ',')
+			n--;
+		ret++;
+	}
+	return ret;
+}
+
 bool
 preserve_sbat_uefi_variable(UINT8 *sbat, UINTN sbatsize, UINT32 attributes,
 		char *sbat_var)
 {
-	const char *sbatc = (const char *)sbat;
-
-	return check_sbat_var_attributes(attributes) &&
-	       sbatsize >= strlen(SBAT_VAR_ORIGINAL) &&
-	       !strcmp(sbatc, SBAT_VAR_SIG SBAT_VAR_VERSION) &&
-	       strcmp(sbatc, sbat_var) >= 0;
+	char *sbatc = (char *)sbat;
+	char *current_version, *new_version,
+	     *current_datestamp, *new_datestamp;
+	int current_version_len, new_version_len;
+
+	/* current metadata is not currupt somehow */
+	if (!check_sbat_var_attributes(attributes) ||
+               sbatsize < strlen(SBAT_VAR_ORIGINAL) ||
+	       strncmp(sbatc, SBAT_VAR_SIG, strlen(SBAT_VAR_SIG)))
+		return false;
+
+	/* current metadata version not newer */
+	current_version = nth_sbat_field(sbatc, 1);
+	new_version = nth_sbat_field(sbat_var, 1);
+	current_datestamp = nth_sbat_field(sbatc, 2);
+	new_datestamp = nth_sbat_field(sbat_var, 2);
+
+	current_version_len = current_datestamp - current_version - 1;
+	new_version_len = new_datestamp - new_version - 1;
+
+	if (current_version_len > new_version_len ||
+	    (current_version_len == new_version_len &&
+	    strncmp(current_version, new_version, new_version_len) > 0))
+		return true;
+
+	/* current datestamp is not newer or idential */
+	if (strncmp(current_datestamp, new_datestamp,
+	    strlen(SBAT_VAR_ORIGINAL_DATE)) >= 0)
+                return true;
+
+	return false;
 }
 
 EFI_STATUS

diff --git a/test-sbat.c b/test-sbat.c
@@ -970,8 +970,36 @@ test_parse_and_verify(void)
 int
 test_preserve_sbat_uefi_variable_good(void)
 {
-	char sbat[] = "sbat,1,2021030218\ncomponent,2,\n";
-	char sbatvar[] = "sbat,1,2021030218\n";
+	char sbat[] =    "sbat,1,2021030218\ncomponent,2,\n";
+	char sbatvar[] = "sbat,1,2021030218\ncomponent,2,\n";
+	size_t sbat_size = sizeof(sbat);
+	UINT32 attributes = SBAT_VAR_ATTRS;
+
+	if (preserve_sbat_uefi_variable(sbat, sbat_size, attributes, sbatvar))
+		return 0;
+	else
+		return -1;
+}
+
+int
+test_preserve_sbat_uefi_variable_version_newer(void)
+{
+	char sbat[] =    "sbat,2,2022030218\ncomponent,2,\n";
+	char sbatvar[] = "sbat,1,2021030218\ncomponent,2,\n";
+	size_t sbat_size = sizeof(sbat);
+	UINT32 attributes = SBAT_VAR_ATTRS;
+
+	if (preserve_sbat_uefi_variable(sbat, sbat_size, attributes, sbatvar))
+		return 0;
+	else
+		return -1;
+}
+
+int
+test_preserve_sbat_uefi_variable_version_newerlonger(void)
+{
+	char sbat[] =    "sbat,10,2022030218\ncomponent,2,\n";
+	char sbatvar[] = "sbat,2,2021030218\ncomponent,2,\n";
 	size_t sbat_size = sizeof(sbat);
 	UINT32 attributes = SBAT_VAR_ATTRS;
 
@@ -981,11 +1009,40 @@ test_preserve_sbat_uefi_variable_good(void)
 		return -1;
 }
 
+int
+test_preserve_sbat_uefi_variable_version_older(void)
+{
+	char sbat[] =    "sbat,1,2021030218\ncomponent,2,\n";
+	char sbatvar[] = "sbat,2,2022030218\ncomponent,2,\n";
+	size_t sbat_size = sizeof(sbat);
+	UINT32 attributes = SBAT_VAR_ATTRS;
+
+	if (preserve_sbat_uefi_variable(sbat, sbat_size, attributes, sbatvar))
+		return -1;
+	else
+		return 0;
+}
+
+int
+test_preserve_sbat_uefi_variable_version_olderlonger(void)
+{
+	char sbat[] =    "sbat,2,2021030218\ncomponent,2,\n";
+	char sbatvar[] = "sbat,10,2022030218\ncomponent,2,\n";
+	size_t sbat_size = sizeof(sbat);
+	UINT32 attributes = SBAT_VAR_ATTRS;
+
+	if (preserve_sbat_uefi_variable(sbat, sbat_size, attributes, sbatvar))
+		return -1;
+	else
+		return 0;
+}
+
+
 int
 test_preserve_sbat_uefi_variable_newer(void)
 {
-	char sbat[] = "sbat,1,2021030218\ncomponent,2,\n";
-	char sbatvar[] = "sbat,1,2025030218\ncomponent,5,\n";
+	char sbat[] =    "sbat,1,2021030218\ncomponent,2,\n";
+	char sbatvar[] = "sbat,1,2025030218\ncomponent,5,\ncomponent,3";
 	size_t sbat_size = sizeof(sbat);
 	UINT32 attributes = SBAT_VAR_ATTRS;
 
@@ -997,7 +1054,7 @@ test_preserve_sbat_uefi_variable_newer(void)
 int
 test_preserve_sbat_uefi_variable_older(void)
 {
-	char sbat[] = "sbat,1,2025030218\ncomponent,2,\n";
+	char sbat[] =    "sbat,1,2025030218\ncomponent,2,\ncomponent,3";
 	char sbatvar[] = "sbat,1,2020030218\ncomponent,1,\n";
 	size_t sbat_size = sizeof(sbat);
 	UINT32 attributes = SBAT_VAR_ATTRS;
@@ -1093,6 +1150,10 @@ main(void)
 	test(test_preserve_sbat_uefi_variable_bad_sig);
 	test(test_preserve_sbat_uefi_variable_bad_attr);
 	test(test_preserve_sbat_uefi_variable_bad_short);
+	test(test_preserve_sbat_uefi_variable_version_newer);
+	test(test_preserve_sbat_uefi_variable_version_newerlonger);
+	test(test_preserve_sbat_uefi_variable_version_older);
+	test(test_preserve_sbat_uefi_variable_version_olderlonger);
 
 	return 0;
 }