efi_loader: Add size checks to efi_create_indexed_name()

Although the function description states the caller must provide a
sufficient buffer, it's better to have in function checks that the
destination buffer can hold the intended value.

So let's add an extra argument with the buffer size and check that
before doing any copying.

Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

diff --git a/include/efi_loader.h b/include/efi_loader.h
index 62a6c3de5a02cb30c416bbc1c18d063d545f65e2..790d4bf64c9c4b5eec196b78c66c274286d3284e 100644 (file)
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -824,7 +824,8 @@ bool efi_image_parse(void *efi, size_t len, struct efi_image_regions **regp,
 void efi_memcpy_runtime(void *dest, const void *src, size_t n);
 
 /* commonly used helper function */
-u16 *efi_create_indexed_name(u16 *buffer, const char *name, unsigned int index);
+u16 *efi_create_indexed_name(u16 *buffer, size_t buffer_size, const char *name,
+                            unsigned int index);
 
 extern const struct efi_firmware_management_protocol efi_fmp_fit;
 extern const struct efi_firmware_management_protocol efi_fmp_raw;

diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
index ea22ee796843d5dc7d314990ee1501db6a78c696..4ef2546267866cd32a2286088c84058744ba591e 100644 (file)
--- a/lib/efi_loader/efi_capsule.c
+++ b/lib/efi_loader/efi_capsule.c
@@ -73,8 +73,8 @@ void set_capsule_result(int index, struct efi_capsule_header *capsule,
        struct efi_time time;
        efi_status_t ret;
 
-       efi_create_indexed_name(variable_name16, "Capsule", index);
-
+       efi_create_indexed_name(variable_name16, sizeof(variable_name16),
+                               "Capsule", index);
        result.variable_total_size = sizeof(result);
        result.capsule_guid = capsule->capsule_guid;
        ret = EFI_CALL((*efi_runtime_services.get_time)(&time, NULL));
@@ -896,7 +896,8 @@ efi_status_t efi_launch_capsules(void)
        free(files);
 
        /* CapsuleLast */
-       efi_create_indexed_name(variable_name16, "Capsule", index - 1);
+       efi_create_indexed_name(variable_name16, sizeof(variable_name16),
+                               "Capsule", index - 1);
        efi_set_variable_int(L"CapsuleLast", &efi_guid_capsule_report,
                             EFI_VARIABLE_READ_ONLY |
                             EFI_VARIABLE_NON_VOLATILE |

diff --git a/lib/efi_loader/efi_string.c b/lib/efi_loader/efi_string.c
index 3de721f06c7f141c94804e297e06644409fe7447..96272422886657911e384708eef06046fc64a87a 100644 (file)
--- a/lib/efi_loader/efi_string.c
+++ b/lib/efi_loader/efi_string.c
@@ -23,13 +23,19 @@
  * Return: A pointer to the next position after the created string
  *        in @buffer, or NULL otherwise
  */
-u16 *efi_create_indexed_name(u16 *buffer, const char *name, unsigned int index)
+u16 *efi_create_indexed_name(u16 *buffer, size_t buffer_size, const char *name,
+                            unsigned int index)
 {
        u16 *p = buffer;
        char index_buf[5];
+       size_t size;
 
+       size = (utf8_utf16_strlen(name) * sizeof(u16) +
+               sizeof(index_buf) * sizeof(u16));
+       if (buffer_size < size)
+               return NULL;
        utf8_utf16_strcpy(&p, name);
-       sprintf(index_buf, "%04X", index);
+       snprintf(index_buf, sizeof(index_buf), "%04X", index);
        utf8_utf16_strcpy(&p, index_buf);
 
        return p;

diff --git a/test/unicode_ut.c b/test/unicode_ut.c
index 33fc8b0ee1e257f3a52c77dd6c3c558fcca5431d..6130ef0b54975cf4440b05de559df452a3bb16d0 100644 (file)
--- a/test/unicode_ut.c
+++ b/test/unicode_ut.c
@@ -603,7 +603,7 @@ static int unicode_test_efi_create_indexed_name(struct unit_test_state *uts)
        u16 *pos;
 
        memset(buf, 0xeb, sizeof(buf));
-       pos = efi_create_indexed_name(buf, "Capsule", 0x0af9);
+       pos = efi_create_indexed_name(buf, sizeof(buf), "Capsule", 0x0af9);
 
        ut_asserteq_mem(expected, buf, sizeof(expected));
        ut_asserteq(pos - buf, u16_strnlen(buf, SIZE_MAX));