commit 88a349c517f4d719983c8740455d8b99585f72ae
author John Wang <wangzqbj@inspur.com> Mon Jan 20 14:19:49 2020 +0800
committer Deepak Kodihalli <dkodihal@in.ibm.com> Mon Feb 10 07:36:47 2020 -0600
tree d6836f7185ba32535d1d409c9d8891d6dff340f4
parent b0da7a0b05d221b53d8ec17c5d178c90d8d7e88f

libpldm: bios_table: Fix a possible overflow

pldm_bios_table_string_entry_decode_string appends \0
at the end of the string which could exceed the buffer size

Signed-off-by: John Wang <wangzqbj@inspur.com>
Change-Id: I43f67e1b7631e9f6043b75846baf123adf1f7fe1

libpldm/bios_table.c

diff --git a/libpldm/bios_table.c b/libpldm/bios_table.c
index 9d71819..5eb6125 100644
--- a/libpldm/bios_table.c
+++ b/libpldm/bios_table.c
@@ -96,7 +96,7 @@
 {
 	uint16_t length =
 	    pldm_bios_table_string_entry_decode_string_length(entry);
-	length = length < size ? length : size;
+	length = length < (size - 1) ? length : (size - 1);
 	memcpy(buffer, entry->name, length);
 	buffer[length] = 0;
 	return length;

libpldmresponder/bios.cpp

diff --git a/libpldmresponder/bios.cpp b/libpldmresponder/bios.cpp
index 3f0d4ec..b0b12d8 100644
--- a/libpldmresponder/bios.cpp
+++ b/libpldmresponder/bios.cpp
@@ -331,19 +331,20 @@
     BIOSStringTable.load(table);
     auto stringEntry = pldm_bios_table_string_find_by_handle(
         table.data(), table.size(), stringHdl);
-    std::string name;
     if (stringEntry == nullptr)
     {
         std::cerr << "Reached end of BIOS string table,did not find "
                   << "string name for handle, STRING_HANDLE=" << stringHdl
                   << "\n";
+        throw InternalFailure();
     }
     auto strLength =
         pldm_bios_table_string_entry_decode_string_length(stringEntry);
-    name.resize(strLength);
-    pldm_bios_table_string_entry_decode_string(stringEntry, name.data(),
-                                               name.size());
-    return name;
+    std::vector<char> buffer(strLength + 1);
+    pldm_bios_table_string_entry_decode_string(stringEntry, buffer.data(),
+                                               buffer.size());
+
+    return std::string(buffer.data(), buffer.data() + strLength);
 }
 
 namespace bios_type_enum

test/libpldm_bios_table_test.cpp

diff --git a/test/libpldm_bios_table_test.cpp b/test/libpldm_bios_table_test.cpp
index 49eeb31..1f1655c 100644
--- a/test/libpldm_bios_table_test.cpp
+++ b/test/libpldm_bios_table_test.cpp
@@ -928,8 +928,8 @@
         entry, buffer.data(), buffer.size());
     EXPECT_EQ(decodedLength, strLength);
     EXPECT_EQ(std::strcmp("Allowed", buffer.data()), 0);
-    decodedLength =
-        pldm_bios_table_string_entry_decode_string(entry, buffer.data(), 2);
+    decodedLength = pldm_bios_table_string_entry_decode_string(
+        entry, buffer.data(), 2 + 1 /* sizeof '\0'*/);
     EXPECT_EQ(decodedLength, 2);
     EXPECT_EQ(std::strcmp("Al", buffer.data()), 0);