Re: [U-Boot] [PATCH] efi_selftest: check for buffer overflow in efi_get_variable

[Heinrich Schuchardt]

On 05/25/2018 02:09 AM, Ivan Gorinov wrote:
> Check if GetVariable writes past the end of the output data buffer.
> 
> Signed-off-by: Ivan Gorinov 

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

> ---
>  lib/efi_selftest/efi_selftest_variables.c | 16 
>  1 file changed, 16 insertions(+)
> 
> diff --git a/lib/efi_selftest/efi_selftest_variables.c 
> b/lib/efi_selftest/efi_selftest_variables.c
> index f5e8eb6..8a02dbc 100644
> --- a/lib/efi_selftest/efi_selftest_variables.c
> +++ b/lib/efi_selftest/efi_selftest_variables.c
> @@ -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", _vendor0,
> + , , 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", _vendor1,
>   EFI_VARIABLE_BOOTSERVICE_ACCESS,
> 

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH] efi_selftest: check for buffer overflow in efi_get_variable

[Ivan Gorinov]

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

Signed-off-by: Ivan Gorinov 
---
 lib/efi_selftest/efi_selftest_variables.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/lib/efi_selftest/efi_selftest_variables.c 
b/lib/efi_selftest/efi_selftest_variables.c
index f5e8eb6..8a02dbc 100644
--- a/lib/efi_selftest/efi_selftest_variables.c
+++ b/lib/efi_selftest/efi_selftest_variables.c
@@ -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", _vendor0,
+   , , 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", _vendor1,
EFI_VARIABLE_BOOTSERVICE_ACCESS,
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot