openbmc / libpldm

Apache License 2.0
9 stars 12 forks source link

Harden get_fru_record_by_option() #10

Closed fbarrat closed 1 year ago

fbarrat commented 1 year ago

get_fru_record_by_option() walks the table of records and makes 2 assumptions to detect the end of the table:

If any of those 2 conditions is not met, it may try reading data outside of the caller-provided buffer, which never ends well.

Suggested quick fix:

diff --git a/pldm/libpldm/fru.c b/pldm/libpldm/fru.c
index be9d4114..48252414 100644
--- a/pldm/libpldm/fru.c
+++ b/pldm/libpldm/fru.c
@@ -201,7 +201,7 @@ int encode_fru_record(uint8_t *fru_table, size_t total_size, size_t *curr_size,
 static bool is_table_end(const struct pldm_fru_record_data_format *p,
                         const void *table, size_t table_size)
 {
-       return p ==
+       return p >=
               (const struct pldm_fru_record_data_format *)((uint8_t *)table +
                                                            table_size);
 }
amboar commented 1 year ago

Agh, this slipped through the cracks, but I've pushed a patch now: https://gerrit.openbmc.org/c/openbmc/libpldm/+/64973