Skip to contentSkip to conversation

efi_selftest: check for buffer overflow in efi_get_variable

PATCH Superseded Ivan Gorinov 2 participants 2 messages May 25, 2018

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 efi_selftest: check for buffer overflow in efi_get_variable

Check if GetVariable writes past the end of the output data buffer.

---
 lib/efi_selftest/efi_selftest_variables.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
lib/efi_selftest/efi_selftest_variables.c +16 -0
index f5e8eb6..8a02dbc 100644
@@ -72,6 +72,22 @@ static int execute(void)
 		efi_st_error("SetVariable failed\n");
 		return EFI_ST_FAILURE;
 	}
+	data[3] = 0xff;
+	len = 3;
+	ret = runtime->get_variable(L"efi_st_var0", &guid_vendor0,
+				    &attr, &len, data);
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("GetVariable failed\n");
+		return EFI_ST_FAILURE;
+	}
+	if (efi_st_memcmp(data, v + 4, 3)) {
+		efi_st_error("GetVariable returned wrong value\n");
+		return EFI_ST_FAILURE;
+	}
+	if (data[3] != 0xff) {
+		efi_st_error("GetVariable wrote past the end of the buffer\n");
+		return EFI_ST_FAILURE;
+	}
 	/* Set variable 1 */
 	ret = runtime->set_variable(L"efi_st_var1", &guid_vendor1,
 				    EFI_VARIABLE_BOOTSERVICE_ACCESS,
Signature · 1 line
2.7.4
#1 lore raw

PATCH 1/1 efi_selftest: check for buffer overflow in efi_get_variable

Quoted 3 lines · Ivan Gorinov

Check if GetVariable writes past the end of the output data buffer.

The patch produces build warnings:

lib/efi_selftest/efi_selftest_variables.c: In function ‘execute’:
lib/efi_selftest/efi_selftest_variables.c:75:10: warning: assignment makes pointer from integer without a cast [-Wint-conversion]

  data[3] = 0xff;
          ^

lib/efi_selftest/efi_selftest_variables.c:87:14: warning: comparison between pointer and integer
if (data[3] != 0xff) {

This is due to an inappropriate definition in my prior patch. Could you, please, add this change:

-u8 *data[EFI_ST_MAX_DATA_SIZE];
+u8 data[EFI_ST_MAX_DATA_SIZE];

Otherwise your patch is fine.

Best regards

Heinrich

Quoted 32 lines
---
 lib/efi_selftest/efi_selftest_variables.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
lib/efi_selftest/efi_selftest_variables.c +16 -0
index f5e8eb6..8a02dbc 100644
@@ -72,6 +72,22 @@ static int execute(void)
 		efi_st_error("SetVariable failed\n");
 		return EFI_ST_FAILURE;
 	}
+	data[3] = 0xff;
+	len = 3;
+	ret = runtime->get_variable(L"efi_st_var0", &guid_vendor0,
+				    &attr, &len, data);
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("GetVariable failed\n");
+		return EFI_ST_FAILURE;
+	}
+	if (efi_st_memcmp(data, v + 4, 3)) {
+		efi_st_error("GetVariable returned wrong value\n");
+		return EFI_ST_FAILURE;
+	}
+	if (data[3] != 0xff) {
+		efi_st_error("GetVariable wrote past the end of the buffer\n");
+		return EFI_ST_FAILURE;
+	}
 	/* Set variable 1 */
 	ret = runtime->set_variable(L"efi_st_var1", &guid_vendor1,
 				    EFI_VARIABLE_BOOTSERVICE_ACCESS,