From: Jean Delvare <jdelvare@suse.de>
Subject: Prevent buffer overflow with large ID strings

When IPMI records have ID strings larger than 16 bytes, we
have to make sure we only read 16 bytes, because the structures
used only have room for that many bytes, and so does the desc
buffer.

Note: I made the changes minimal because I am not familiar with the
code. But I am really curious why the structures only reserve only
16 bytes for id_string when the IPMI specification says the maximum
valid length is 30.

Signed-off-by: Jean Delvare <jdelvare@suse.de>
Acked-by: Torsten Duwe <duwe@suse.de>
---
 lib/ipmi_fru.c |    6 +++---
 lib/ipmi_sdr.c |   26 ++++++++++++++++++--------
 2 files changed, 21 insertions(+), 11 deletions(-)

--- ipmitool-1.8.11.orig/lib/ipmi_sdr.c
+++ ipmitool-1.8.11/lib/ipmi_sdr.c
@@ -1130,7 +1130,7 @@ ipmi_sdr_print_sensor_full(struct ipmi_i
 			   struct sdr_record_full_sensor *sensor)
 {
 	char sval[16], unitstr[16], desc[17];
-	int i = 0, validread = 1, do_unit = 1;
+	int i = 0, validread = 1, do_unit = 1, desc_len;
 	double val = 0.0, creading = 0.0;
 	struct ipmi_rs *rsp;
 	uint8_t target, lun;
@@ -1142,7 +1142,8 @@ ipmi_sdr_print_sensor_full(struct ipmi_i
 	lun = sensor->keys.lun;
 
 	memset(desc, 0, sizeof (desc));
-	snprintf(desc, (sensor->id_code & 0x1f) + 1, "%s", sensor->id_string);
+	desc_len = (sensor->id_code & 0x1f) > 16 ? 16 : (sensor->id_code & 0x1f);
+	snprintf(desc, desc_len + 1, "%s", sensor->id_string);
 
 	/* get sensor reading */
 	rsp = ipmi_sdr_get_sensor_reading_ipmb(intf, sensor->keys.sensor_num,
@@ -1713,7 +1714,7 @@ ipmi_sdr_print_sensor_compact(struct ipm
 {
 	struct ipmi_rs *rsp;
 	char desc[17];
-	int validread = 1;
+	int validread = 1, desc_len;
 	uint8_t target, lun;
 
 	if (sensor == NULL)
@@ -1723,7 +1724,8 @@ ipmi_sdr_print_sensor_compact(struct ipm
 	lun = sensor->keys.lun;
 
 	memset(desc, 0, sizeof (desc));
-	snprintf(desc, (sensor->id_code & 0x1f) + 1, "%s", sensor->id_string);
+	desc_len = (sensor->id_code & 0x1f) > 16 ? 16 : (sensor->id_code & 0x1f);
+	snprintf(desc, desc_len + 1, "%s", sensor->id_string);
 
 	/* get sensor reading */
 	rsp = ipmi_sdr_get_sensor_reading_ipmb(intf, sensor->keys.sensor_num,
@@ -1903,12 +1905,14 @@ ipmi_sdr_print_sensor_eventonly(struct i
 				struct sdr_record_eventonly_sensor *sensor)
 {
 	char desc[17];
+	int desc_len;
 
 	if (sensor == NULL)
 		return -1;
 
 	memset(desc, 0, sizeof (desc));
-	snprintf(desc, (sensor->id_code & 0x1f) + 1, "%s", sensor->id_string);
+	desc_len = (sensor->id_code & 0x1f) > 16 ? 16 : (sensor->id_code & 0x1f);
+	snprintf(desc, desc_len + 1, "%s", sensor->id_string);
 
 	if (verbose) {
 		printf("Sensor ID              : %s (0x%x)\n",
@@ -1953,12 +1957,14 @@ ipmi_sdr_print_sensor_mc_locator(struct
 				 struct sdr_record_mc_locator *mc)
 {
 	char desc[17];
+	int desc_len;
 
 	if (mc == NULL)
 		return -1;
 
 	memset(desc, 0, sizeof (desc));
-	snprintf(desc, (mc->id_code & 0x1f) + 1, "%s", mc->id_string);
+	desc_len = (mc->id_code & 0x1f) > 16 ? 16 : (mc->id_code & 0x1f);
+	snprintf(desc, desc_len + 1, "%s", mc->id_string);
 
 	if (verbose == 0) {
 		if (csv_output)
@@ -2049,9 +2055,11 @@ ipmi_sdr_print_sensor_generic_locator(st
 				      struct sdr_record_generic_locator *dev)
 {
 	char desc[17];
+	int desc_len;
 
 	memset(desc, 0, sizeof (desc));
-	snprintf(desc, (dev->id_code & 0x1f) + 1, "%s", dev->id_string);
+	desc_len = (dev->id_code & 0x1f) > 16 ? 16 : (dev->id_code & 0x1f);
+	snprintf(desc, desc_len + 1, "%s", dev->id_string);
 
 	if (!verbose) {
 		if (csv_output)
@@ -2106,9 +2114,11 @@ ipmi_sdr_print_sensor_fru_locator(struct
 				  struct sdr_record_fru_locator *fru)
 {
 	char desc[17];
+	int desc_len;
 
 	memset(desc, 0, sizeof (desc));
-	snprintf(desc, (fru->id_code & 0x1f) + 1, "%s", fru->id_string);
+	desc_len = (fru->id_code & 0x1f) > 16 ? 16 : (fru->id_code & 0x1f);
+	snprintf(desc, desc_len + 1, "%s", fru->id_string);
 
 	if (!verbose) {
 		if (csv_output)
--- ipmitool-1.8.11.orig/lib/ipmi_fru.c
+++ ipmitool-1.8.11/lib/ipmi_fru.c
@@ -2611,7 +2611,7 @@ ipmi_fru_print(struct ipmi_intf * intf,
 {
 	char desc[17];
 	uint8_t save_addr;
-	int rc = 0;
+	int rc = 0, desc_len;
 
 	if (fru == NULL)
 		return __ipmi_fru_print(intf, 0);
@@ -2643,8 +2643,8 @@ ipmi_fru_print(struct ipmi_intf * intf,
 		return 0;
 
 	memset(desc, 0, sizeof(desc));
-	memcpy(desc, fru->id_string, fru->id_code & 0x01f);
-	desc[fru->id_code & 0x01f] = 0;
+	desc_len = (fru->id_code & 0x1f) > 16 ? 16 : (fru->id_code & 0x1f);
+	snprintf(desc, desc_len + 1, "%s", fru->id_string);
 	printf("FRU Device Description : %s (ID %d)\n", desc, fru->device_id);
 
 	switch (fru->dev_type_modifier) {