Blob Blame History Raw
From 9819ec7325089d325ff13af3c3d615209f3fb2c9 Mon Sep 17 00:00:00 2001
From: Peter Jones <pjones@redhat.com>
Date: Tue, 18 Jun 2019 15:54:58 -0400
Subject: [PATCH 39/63] Fix a case clang-analyzer found where we may try to
 parse a NULL

I don't think this is something that can *actually* happen - it didn't
trigger before save_variable() was added, and the save_variable() path
that calls this calls validate_name() immediately prior to this call.
validate_name() calls exit() if it's NULL.  But that's weird as well,
because that's the same pattern all the other users of parse_name() use.

Anyway, this patch expands validate_name() and moves it into
parse_name() so we don't need to call it from everywhere when we're just
calling the two in a row anyway.

Signed-off-by: Peter Jones <pjones@redhat.com>
---
 src/efivar.c | 45 ++++++++++++++++++++++++++++++---------------
 1 file changed, 30 insertions(+), 15 deletions(-)

diff --git a/src/efivar.c b/src/efivar.c
index 885a9af864b..8b1da8888f6 100644
--- a/src/efivar.c
+++ b/src/efivar.c
@@ -95,6 +95,34 @@ show_errors(void)
 	}
 }
 
+static inline void
+validate_name(const char *name)
+{
+	if (name == NULL) {
+err:
+		warnx("Invalid variable name \"%s\"",
+		      (name == NULL) ? "(null)" : name);
+		show_errors();
+		exit(1);
+	}
+	if (name[0] == '{') {
+		const char *next = strchr(name+1, '}');
+		if (!next)
+			goto err;
+		if (next[1] != '-')
+			goto err;
+		if (next[2] == '\000')
+			goto err;
+	} else {
+		if (strlen(name) < 38)
+			goto err;
+		if (name[8] != '-' || name[13] != '-' ||
+		    name[18] != '-' || name[23] != '-' ||
+		    name[36] != '-')
+			goto err;
+	}
+}
+
 static void
 list_all_variables(void)
 {
@@ -124,6 +152,8 @@ parse_name(const char *guid_name, char **name, efi_guid_t *guid)
 
 	const char *left, *right;
 
+	validate_name(guid_name);
+
 	left = strchr(guid_name, '{');
 	right = strchr(guid_name, '}');
 	if (left && right) {
@@ -408,16 +438,6 @@ edit_variable(const char *guid_name, void *data, size_t data_size,
 	}
 }
 
-static void
-validate_name(const char *name)
-{
-	if (name == NULL) {
-		fprintf(stderr, "Invalid variable name\n");
-		show_errors();
-		exit(1);
-	}
-}
-
 static void
 prepare_data(const char *filename, uint8_t **data, size_t *data_size)
 {
@@ -588,21 +608,17 @@ int main(int argc, char *argv[])
 			list_all_variables();
 			break;
 		case ACTION_PRINT:
-			validate_name(guid_name);
 			show_variable(guid_name, SHOW_VERBOSE);
 			break;
 		case ACTION_PRINT_DEC | ACTION_PRINT:
-			validate_name(guid_name);
 			show_variable(guid_name, SHOW_DECIMAL);
 			break;
 		case ACTION_APPEND | ACTION_PRINT:
-			validate_name(guid_name);
 			prepare_data(infile, &data, &data_size);
 			edit_variable(guid_name, data, data_size, attributes,
 				      EDIT_APPEND);
 			break;
 		case ACTION_WRITE | ACTION_PRINT:
-			validate_name(guid_name);
 			prepare_data(infile, &data, &data_size);
 			edit_variable(guid_name, data, data_size, attributes,
 				      EDIT_WRITE);
@@ -653,7 +669,6 @@ int main(int argc, char *argv[])
 
 				efi_variable_free(var, false);
 			} else {
-				validate_name(guid_name);
 				save_variable(guid_name, outfile, dmpstore);
 			}
 			break;
-- 
2.26.2