[PATCH 0/3] efi_loader: Fix buffer underflow in FatToStr

[PATCH 0/3] efi_loader: Fix buffer underflow in FatToStr

[Heinrich Schuchardt]

Fix a buffer underflow in the FatToStr implementation.

Add a unit test to detect incorrect conversion of characters >= 0x80.

v2:
	Reformat commit message
	Add unit test

Heinrich Schuchardt (2):
  efi_selftest: unsigned char parameter for efi_st_strcmp_16_8()
  efi_selftest: Improve the FatToStr() unit test

Mikhail Ilin (1):
  efi_loader: Fix buffer underflow

 include/efi_selftest.h                            |  2 +-
 lib/efi_loader/efi_unicode_collation.c            |  2 +-
 lib/efi_selftest/efi_selftest_unicode_collation.c | 13 +++++++++++++
 lib/efi_selftest/efi_selftest_util.c              |  2 +-
 4 files changed, 16 insertions(+), 3 deletions(-)

-- 
2.37.2

[PATCH v2 1/3] efi_loader: Fix buffer underflow

[Heinrich Schuchardt]

From: Mikhail Ilin <ilin.mikhail.ol@gmail.com>

If the array index 'i' < 128, the 'codepage' array is accessed using
[-128...-1] in efi_unicode_collation.c:262. This can lead to a buffer
overflow.

    Negative index in efi_unicode_collation.c:262.

The index of the 'codepage' array should be c - 0x80 instead of i - 0x80.

Fixes: 0bc4b0da7b59 ("efi_loader: EFI_UNICODE_COLLATION_PROTOCOL")
Signed-off-by: Mikhail Ilin <ilin.mikhail.ol@gmail.com>
Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
v2:
	reformat commit message
---
 lib/efi_loader/efi_unicode_collation.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_unicode_collation.c b/lib/efi_loader/efi_unicode_collation.c
index 36be798f64..c4c7572063 100644
--- a/lib/efi_loader/efi_unicode_collation.c
+++ b/lib/efi_loader/efi_unicode_collation.c
@@ -257,7 +257,7 @@ static void EFIAPI efi_fat_to_str(struct efi_unicode_collation_protocol *this,
 	for (i = 0; i < fat_size; ++i) {
 		c = (unsigned char)fat[i];
 		if (c > 0x80)
-			c = codepage[i - 0x80];
+			c = codepage[c - 0x80];
 		string[i] = c;
 		if (!c)
 			break;
-- 
2.37.2

[PATCH v2 2/3] efi_selftest: unsigned char parameter for efi_st_strcmp_16_8()

[Heinrich Schuchardt]

Use unsigned char for the parameter of efi_st_strcmp_16_8. This allows
comparing characters 0x80 - 0xff.

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
v2:
	new patch
---
 include/efi_selftest.h               | 2 +-
 lib/efi_selftest/efi_selftest_util.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/efi_selftest.h b/include/efi_selftest.h
index e900cb85a9..7c69c3f376 100644
--- a/include/efi_selftest.h
+++ b/include/efi_selftest.h
@@ -131,7 +131,7 @@ u16 *efi_st_translate_code(u16 code);
  * @buf2:	char string
  * Return:	0 if both buffers contain equivalent strings
  */
-int efi_st_strcmp_16_8(const u16 *buf1, const char *buf2);
+int efi_st_strcmp_16_8(const u16 *buf1, const unsigned char *buf2);
 
 /**
  * efi_st_get_config_table() - get configuration table
diff --git a/lib/efi_selftest/efi_selftest_util.c b/lib/efi_selftest/efi_selftest_util.c
index 7e03e0c939..3681fa6431 100644
--- a/lib/efi_selftest/efi_selftest_util.c
+++ b/lib/efi_selftest/efi_selftest_util.c
@@ -102,7 +102,7 @@ u16 *efi_st_translate_code(u16 code)
 	return efi_st_unknown;
 }
 
-int efi_st_strcmp_16_8(const u16 *buf1, const char *buf2)
+int efi_st_strcmp_16_8(const u16 *buf1, const unsigned char *buf2)
 {
 	for (; *buf1 || *buf2; ++buf1, ++buf2) {
 		if (*buf1 != *buf2)
-- 
2.37.2

[PATCH v2 3/3] efi_selftest: Improve the FatToStr() unit test

[Heinrich Schuchardt]

Add a test with a character >= 0x80.

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
v2:
	new patch
---
 lib/efi_selftest/efi_selftest_unicode_collation.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/lib/efi_selftest/efi_selftest_unicode_collation.c b/lib/efi_selftest/efi_selftest_unicode_collation.c
index c63a1b51e4..1ac55b34cd 100644
--- a/lib/efi_selftest/efi_selftest_unicode_collation.c
+++ b/lib/efi_selftest/efi_selftest_unicode_collation.c
@@ -184,6 +184,18 @@ static int test_fat_to_str(void)
 		return EFI_ST_FAILURE;
 	}
 
+	boottime->set_mem(str, sizeof(str), 0);
+	unicode_collation_protocol->fat_to_str(unicode_collation_protocol, 13,
+					       "Kafb\240tur\000xyz", str);
+	if (str[10]) {
+		efi_st_error("fat_to_str returned to many characters\n");
+		return EFI_ST_FAILURE;
+	}
+	if (efi_st_strcmp_16_8(str, "Kafb\341tur")) {
+		efi_st_error("fat_to_str returned \"%ps\"\n", str);
+		return EFI_ST_FAILURE;
+	}
+
 	return EFI_ST_SUCCESS;
 }
 
-- 
2.37.2