fru: bound remote record table parsing

parseFruRecordTable() checked only the minimum FRU table size. It
then walked remote record headers and TLVs using attacker-controlled
field counts and TLV lengths without checking the remaining buffer.

A malicious PLDM terminus can answer the BMC's GetFRURecordTable
request with a truncated final TLV. The parser copies bytes past the
response allocation into the parsed FRU value, which may publish
adjacent heap data on D-Bus or crash pldmd when the read crosses an
unmapped boundary.

Validate every record header, TLV header and TLV body before access.
Reject malformed tables without returning partially parsed records.

Change-Id: I748b0db88f6e13b3f7b8820e877618c2f477e063
Signed-off-by: Manjeet Singh <itsmanjeet1998@gmail.com>

libpldmresponder/pdr_utils.cpp

diff --git a/libpldmresponder/pdr_utils.cpp b/libpldmresponder/pdr_utils.cpp
index c3d5458..86952a1 100644
--- a/libpldmresponder/pdr_utils.cpp
+++ b/libpldmresponder/pdr_utils.cpp
@@ -218,7 +218,7 @@
     // 7: uint16_t(FRU Record Set Identifier), uint8_t(FRU Record Type),
     // uint8_t(Number of FRU fields), uint8_t(Encoding Type for FRU fields),
     // uint8_t(FRU Field Type), uint8_t(FRU Field Length)
-    if (fruLen < fruRecordDataFormatLength)
+    if (fruData == nullptr || fruLen < fruRecordDataFormatLength)
     {
         error("Invalid FRU length '{LENGTH}' while parsing FRU record table",
               "LENGTH", fruLen);
@@ -230,6 +230,15 @@
     size_t index = 0;
     while (index < fruLen)
     {
+        constexpr size_t recordHeaderLength =
+            offsetof(pldm_fru_record_data_format, tlvs);
+        if (recordHeaderLength > fruLen - index)
+        {
+            error("Truncated FRU record header at offset '{OFFSET}'", "OFFSET",
+                  index);
+            return {};
+        }
+
         FruRecordDataFormat fru;
 
         auto record = reinterpret_cast<const pldm_fru_record_data_format*>(
@@ -239,28 +248,37 @@
         fru.fruNum = record->num_fru_fields;
         fru.fruEncodeType = record->encoding_type;
 
-        index += 5;
+        index += recordHeaderLength;
 
-        std::ranges::for_each(
-            std::views::iota(0, (int)record->num_fru_fields),
-            [fruData, &fru, &index](int) {
-                auto tlv = reinterpret_cast<const pldm_fru_record_tlv*>(
-                    fruData + index);
-                FruTLV frutlv;
-                frutlv.fruFieldType = tlv->type;
-                frutlv.fruFieldLen = tlv->length;
-                frutlv.fruFieldValue.resize(tlv->length);
-                for (const auto& i : std::views::iota(0, (int)tlv->length))
-                {
-                    memcpy(frutlv.fruFieldValue.data() + i, tlv->value + i, 1);
-                }
-                fru.fruTLV.push_back(frutlv);
+        for (uint8_t field = 0; field < record->num_fru_fields; ++field)
+        {
+            if (fruFieldTypeLength > fruLen - index)
+            {
+                error("Truncated FRU field header at offset '{OFFSET}'",
+                      "OFFSET", index);
+                return {};
+            }
 
-                // 2: 1byte FRU Field Type, 1byte FRU Field Length
-                index += fruFieldTypeLength + (unsigned)tlv->length;
-            });
+            auto tlv =
+                reinterpret_cast<const pldm_fru_record_tlv*>(fruData + index);
+            index += fruFieldTypeLength;
+            if (tlv->length > fruLen - index)
+            {
+                error("FRU field at offset '{OFFSET}' exceeds table length",
+                      "OFFSET", index - fruFieldTypeLength);
+                return {};
+            }
 
-        frus.push_back(fru);
+            FruTLV frutlv;
+            frutlv.fruFieldType = tlv->type;
+            frutlv.fruFieldLen = tlv->length;
+            frutlv.fruFieldValue.assign(fruData + index,
+                                        fruData + index + tlv->length);
+            fru.fruTLV.push_back(std::move(frutlv));
+            index += tlv->length;
+        }
+
+        frus.push_back(std::move(fru));
     }
 
     return frus;

libpldmresponder/test/libpldmresponder_platform_test.cpp

diff --git a/libpldmresponder/test/libpldmresponder_platform_test.cpp b/libpldmresponder/test/libpldmresponder_platform_test.cpp
index 294ae81..bb0b8ac 100644
--- a/libpldmresponder/test/libpldmresponder_platform_test.cpp
+++ b/libpldmresponder/test/libpldmresponder_platform_test.cpp
@@ -23,6 +23,55 @@
 using namespace pldm::responder::pdr;
 using namespace pldm::responder::pdr_utils;
 
+TEST(ParseFruRecordTable, RejectsFieldPastEndOfBuffer)
+{
+    const std::vector<uint8_t> table{
+        1,   0, /* record set id */
+        1,      /* record type */
+        1,      /* number of fields */
+        1,      /* encoding */
+        2,      /* field type */
+        4,      /* field length */
+        'x',    /* truncated field value */
+    };
+
+    EXPECT_TRUE(parseFruRecordTable(table.data(), table.size()).empty());
+}
+
+TEST(ParseFruRecordTable, RejectsTruncatedNextRecord)
+{
+    const std::vector<uint8_t> table{
+        1,   0, /* record set id */
+        1,      /* record type */
+        1,      /* number of fields */
+        1,      /* encoding */
+        2,      /* field type */
+        1,      /* field length */
+        'x',    /* field value */
+        0,      /* truncated next record */
+    };
+
+    EXPECT_TRUE(parseFruRecordTable(table.data(), table.size()).empty());
+}
+
+TEST(ParseFruRecordTable, ParsesBoundedField)
+{
+    const std::vector<uint8_t> table{
+        1,   0, /* record set id */
+        1,      /* record type */
+        1,      /* number of fields */
+        1,      /* encoding */
+        2,      /* field type */
+        1,      /* field length */
+        'x',    /* field value */
+    };
+
+    const auto records = parseFruRecordTable(table.data(), table.size());
+    ASSERT_EQ(records.size(), 1);
+    ASSERT_EQ(records[0].fruTLV.size(), 1);
+    EXPECT_EQ(records[0].fruTLV[0].fruFieldValue, std::vector<uint8_t>({'x'}));
+}
+
 using ::testing::_;
 using ::testing::Return;
 using ::testing::StrEq;