[PATCH 0/3] efi_loader: Fix buffer underflow in FatToStr
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(-)
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;
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)
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; }
participants (1)
-
Heinrich Schuchardt