diff mbox series

efi_loader: Add size checks to efi_create_indexed_name()

Message ID 20201231102647.201318-1-ilias.apalodimas@linaro.org
State Accepted
Commit fe179d7fb5c10d8a4e299af06c766f47f2c8d51a
Headers show
Series efi_loader: Add size checks to efi_create_indexed_name() | expand

Commit Message

Ilias Apalodimas Dec. 31, 2020, 10:26 a.m. UTC 
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>

---
 include/efi_loader.h         |  3 ++-
 lib/efi_loader/efi_capsule.c |  7 ++++---
 lib/efi_loader/efi_string.c  | 10 ++++++++--
 test/unicode_ut.c            |  2 +-
 4 files changed, 15 insertions(+), 7 deletions(-)

-- 
2.30.0

Comments

Heinrich Schuchardt Dec. 31, 2020, 12:04 p.m. UTC | #1 
On 12/31/20 11:26 AM, Ilias Apalodimas wrote:
> 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 mbox series

Patch

diff --git a/include/efi_loader.h b/include/efi_loader.h
index 365f3d01dc74..def0ab3a7954 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -822,7 +822,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 ea22ee796843..4ef254626786 100644
--- 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 3de721f06c7f..962724228866 100644
--- 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 33fc8b0ee1e2..6130ef0b5497 100644
--- 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));