Define safe variant of decode_get_fru_record_table_resp
The decode_get_fru_record_table_resp API is difficult to use safely
because it doesn't perform bounds-checking on the size of the FRU
record table before copying into the caller-allocated buffer.
This commit defines the decode_get_fru_record_table_resp_safe function
which adds an extra parameter to facilitate bounds-checking.
Signed-off-by: Zach Clark <zach@ibm.com>
Change-Id: Ief93d93df8627f5c15862206b33a901fa7bd92a6
Signed-off-by: Zach Clark <zach@ibm.com>
diff --git a/libpldm/fru.c b/libpldm/fru.c
index 7633e50..fa79f2b 100644
--- a/libpldm/fru.c
+++ b/libpldm/fru.c
@@ -436,10 +436,11 @@
return PLDM_SUCCESS;
}
-int decode_get_fru_record_table_resp(
+int decode_get_fru_record_table_resp_safe(
const struct pldm_msg *msg, size_t payload_length, uint8_t *completion_code,
uint32_t *next_data_transfer_handle, uint8_t *transfer_flag,
- uint8_t *fru_record_table_data, size_t *fru_record_table_length)
+ uint8_t *fru_record_table_data, size_t *fru_record_table_length,
+ size_t max_fru_record_table_length)
{
if (msg == NULL || completion_code == NULL ||
next_data_transfer_handle == NULL || transfer_flag == NULL ||
@@ -460,14 +461,31 @@
*next_data_transfer_handle = le32toh(resp->next_data_transfer_handle);
*transfer_flag = resp->transfer_flag;
- memcpy(fru_record_table_data, resp->fru_record_table_data,
- payload_length - PLDM_GET_FRU_RECORD_TABLE_MIN_RESP_BYTES);
+
*fru_record_table_length =
payload_length - PLDM_GET_FRU_RECORD_TABLE_MIN_RESP_BYTES;
+ if (*fru_record_table_length > max_fru_record_table_length) {
+ return PLDM_ERROR_INVALID_LENGTH;
+ }
+
+ memcpy(fru_record_table_data, resp->fru_record_table_data,
+ *fru_record_table_length);
+
return PLDM_SUCCESS;
}
+int decode_get_fru_record_table_resp(
+ const struct pldm_msg *msg, size_t payload_length, uint8_t *completion_code,
+ uint32_t *next_data_transfer_handle, uint8_t *transfer_flag,
+ uint8_t *fru_record_table_data, size_t *fru_record_table_length)
+{
+ return decode_get_fru_record_table_resp_safe(
+ msg, payload_length, completion_code, next_data_transfer_handle,
+ transfer_flag, fru_record_table_data, fru_record_table_length,
+ (size_t)-1);
+}
+
int decode_set_fru_record_table_req(const struct pldm_msg *msg,
size_t payload_length,
uint32_t *data_transfer_handle,
diff --git a/libpldm/fru.h b/libpldm/fru.h
index 456c051..bb2f571 100644
--- a/libpldm/fru.h
+++ b/libpldm/fru.h
@@ -364,6 +364,31 @@
uint32_t *next_data_transfer_handle, uint8_t *transfer_flag,
uint8_t *fru_record_table_data, size_t *fru_record_table_length);
+/** @brief Decode GetFruRecordTable response data, ensuring that the fru
+ * record table section is small enough to fit in the provided buffer.
+ *
+ * @param[in] msg - Response message
+ * @param[in] payload_length - Length of response message payload
+ * @param[out] completion_code - Pointer to response msg's PLDM completion code
+ * @param[out] next_data_transfer_handle - A handle used to identify the next
+ * portion of the transfer
+ * @param[out] transfer_flag - The transfer flag that indicates what part of
+ * the transfer this response represents
+ * @param[out] fru_record_table_data - This data is a portion of the overall
+ * FRU Record Table
+ * @param[out] fru_record_table_length - Length of the FRU record table data
+ * @param[in] max_fru_record_table_length - Maximum length of the FRU record
+ * table data. If the response contains more data than this,
+ * return PLDM_ERROR_INVALID_LENGTH.
+ * @return pldm_completion_codes
+ */
+
+int decode_get_fru_record_table_resp_safe(
+ const struct pldm_msg *msg, size_t payload_length, uint8_t *completion_code,
+ uint32_t *next_data_transfer_handle, uint8_t *transfer_flag,
+ uint8_t *fru_record_table_data, size_t *fru_record_table_length,
+ size_t max_fru_record_table_length);
+
/** @brief Encode the FRU record in the FRU table
*
* @param[in/out] fru_table - Pointer to the FRU table