Vulnerability: It's been found that multiple functions in ipmitool before 1.8.19 neglect proper checking of the data received from a remote LAN party, which may lead to buffer overflows and potentially to remote code execution on the ipmitool side. This is especially dangerous if ipmitool is run as a privileged user. This problem is fixed in version 1.8.19.
Vulnerability ID: CVE-2020-5208
CWE: CWE-120
Severity score: 6.4
Project: https://github.com/ipmitool/ipmitool
Published: 2020-02-05
File: lib/ipmi_fru.c
Commit message: fru: Fix buffer overflow vulnerabilities

Partial fix for CVE-2020-5208, see
https://github.com/ipmitool/ipmitool/security/advisories/GHSA-g659-9qxw-p7cp

The `read_fru_area_section` function only performs size validation of
requested read size, and falsely assumes that the IPMI message will not
respond with more than the requested amount of data; it uses the
unvalidated response size to copy into `frubuf`. If the response is
larger than the request, this can result in overflowing the buffer.

The same issue affects the `read_fru_area` function.
Commit author: Chrostoper Ertl

Vulnerable version (before fix):
 int read_fru_area(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id, 			uint32_t offset, uint32_t length, uint8_t *frubuf) { 	uint32_t off = offset, tmp, finish; 	struct ipmi_rs * rsp; 	struct ipmi_rq req; 	uint8_t msg_data[4]; read_fru_area(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id,  	finish = offset + length; 	if (finish > fru->size) { 		finish = fru->size; 		lprintf(LOG_NOTICE, "Read FRU Area length %d too large, " 			"Adjusting to %d", 			offset + length, finish - offset); 	}  	memset(&req, 0, sizeof(req)); read_fru_area(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id, 		} 	}  	do { 		tmp = fru->access ? off >> 1 : off; 		msg_data[0] = id; read_fru_area(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id, 		}  		tmp = fru->access ? rsp->data[0] << 1 : rsp->data[0]; 		memcpy(frubuf, rsp->data + 1, tmp); 		off += tmp; 		frubuf += tmp; 		/* sometimes the size returned in the Info command 		* is too large.  return 0 so higher level function 		* still attempts to parse what was returned */ read_fru_area_section(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id, 			uint32_t offset, uint32_t length, uint8_t *frubuf) { 	static uint32_t fru_data_rqst_size = 20; 	uint32_t off = offset, tmp, finish; 	struct ipmi_rs * rsp; 	struct ipmi_rq req; 	uint8_t msg_data[4]; read_fru_area_section(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id,  	finish = offset + length; 	if (finish > fru->size) { 		finish = fru->size; 		lprintf(LOG_NOTICE, "Read FRU Area length %d too large, " 			"Adjusting to %d", 			offset + length, finish - offset); 	}  	memset(&req, 0, sizeof(req)); read_fru_area_section(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id, 	if (fru->access && fru_data_rqst_size > 16) #endif 		fru_data_rqst_size = 16; 	do { 		tmp = fru->access ? off >> 1 : off; 		msg_data[0] = id; read_fru_area_section(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id, 		}  		tmp = fru->access ? rsp->data[0] << 1 : rsp->data[0]; 		memcpy((frubuf + off)-offset, rsp->data + 1, tmp); 		off += tmp;  		/* sometimes the size returned in the Info command 		* is too large.  return 0 so higher level function

Fixed version (after fix):
 int read_fru_area(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id, 			uint32_t offset, uint32_t length, uint8_t *frubuf) { 	uint32_t off = offset; 	uint32_t tmp; 	uint32_t finish; 	uint32_t size_left_in_buffer; 	struct ipmi_rs * rsp; 	struct ipmi_rq req; 	uint8_t msg_data[4]; read_fru_area(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id,  	finish = offset + length; 	if (finish > fru->size) { 		memset(frubuf + fru->size, 0, length - fru->size); 		finish = fru->size; 		lprintf(LOG_NOTICE, "Read FRU Area length %d too large, " 			"Adjusting to %d", 			offset + length, finish - offset); 		length = finish - offset; 	}  	memset(&req, 0, sizeof(req)); read_fru_area(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id, 		} 	}  	size_left_in_buffer = length; 	do { 		tmp = fru->access ? off >> 1 : off; 		msg_data[0] = id; read_fru_area(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id, 		}  		tmp = fru->access ? rsp->data[0] << 1 : rsp->data[0]; 		if(rsp->data_len < 1 		   || tmp > rsp->data_len - 1 		   || tmp > size_left_in_buffer) 		{ 			printf(" Not enough buffer size"); 			return -1; 		}  		memcpy(frubuf, rsp->data + 1, tmp); 		off += tmp; 		frubuf += tmp; 		size_left_in_buffer -= tmp; 		/* sometimes the size returned in the Info command 		* is too large.  return 0 so higher level function 		* still attempts to parse what was returned */ read_fru_area_section(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id, 			uint32_t offset, uint32_t length, uint8_t *frubuf) { 	static uint32_t fru_data_rqst_size = 20; 	uint32_t off = offset; 	uint32_t tmp, finish; 	uint32_t size_left_in_buffer; 	struct ipmi_rs * rsp; 	struct ipmi_rq req; 	uint8_t msg_data[4]; read_fru_area_section(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id,  	finish = offset + length; 	if (finish > fru->size) { 		memset(frubuf + fru->size, 0, length - fru->size); 		finish = fru->size; 		lprintf(LOG_NOTICE, "Read FRU Area length %d too large, " 			"Adjusting to %d", 			offset + length, finish - offset); 		length = finish - offset; 	}  	memset(&req, 0, sizeof(req)); read_fru_area_section(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id, 	if (fru->access && fru_data_rqst_size > 16) #endif 		fru_data_rqst_size = 16;  	size_left_in_buffer = length; 	do { 		tmp = fru->access ? off >> 1 : off; 		msg_data[0] = id; read_fru_area_section(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id, 		}  		tmp = fru->access ? rsp->data[0] << 1 : rsp->data[0]; 		if(rsp->data_len < 1 		   || tmp > rsp->data_len - 1 		   || tmp > size_left_in_buffer) 		{ 			printf(" Not enough buffer size"); 			return -1; 		} 		memcpy((frubuf + off)-offset, rsp->data + 1, tmp); 		off += tmp; 		size_left_in_buffer -= tmp;  		/* sometimes the size returned in the Info command 		* is too large.  return 0 so higher level function

Patch:
patch: @@ -663,7 +663,10 @@ int
 read_fru_area(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id,
 			uint32_t offset, uint32_t length, uint8_t *frubuf)
 {
-	uint32_t off = offset, tmp, finish;
+	uint32_t off = offset;
+	uint32_t tmp;
+	uint32_t finish;
+	uint32_t size_left_in_buffer;
 	struct ipmi_rs * rsp;
 	struct ipmi_rq req;
 	uint8_t msg_data[4];
@@ -676,10 +679,12 @@ read_fru_area(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id,
 
 	finish = offset + length;
 	if (finish > fru->size) {
+		memset(frubuf + fru->size, 0, length - fru->size);
 		finish = fru->size;
 		lprintf(LOG_NOTICE, "Read FRU Area length %d too large, "
 			"Adjusting to %d",
 			offset + length, finish - offset);
+		length = finish - offset;
 	}
 
 	memset(&req, 0, sizeof(req));
@@ -715,6 +720,7 @@ read_fru_area(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id,
 		}
 	}
 
+	size_left_in_buffer = length;
 	do {
 		tmp = fru->access ? off >> 1 : off;
 		msg_data[0] = id;
@@ -756,9 +762,18 @@ read_fru_area(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id,
 		}
 
 		tmp = fru->access ? rsp->data[0] << 1 : rsp->data[0];
+		if(rsp->data_len < 1
+		   || tmp > rsp->data_len - 1
+		   || tmp > size_left_in_buffer)
+		{
+			printf(" Not enough buffer size");
+			return -1;
+		}
+
 		memcpy(frubuf, rsp->data + 1, tmp);
 		off += tmp;
 		frubuf += tmp;
+		size_left_in_buffer -= tmp;
 		/* sometimes the size returned in the Info command
 		* is too large.  return 0 so higher level function
 		* still attempts to parse what was returned */
@@ -791,7 +806,9 @@ read_fru_area_section(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id,
 			uint32_t offset, uint32_t length, uint8_t *frubuf)
 {
 	static uint32_t fru_data_rqst_size = 20;
-	uint32_t off = offset, tmp, finish;
+	uint32_t off = offset;
+	uint32_t tmp, finish;
+	uint32_t size_left_in_buffer;
 	struct ipmi_rs * rsp;
 	struct ipmi_rq req;
 	uint8_t msg_data[4];
@@ -804,10 +821,12 @@ read_fru_area_section(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id,
 
 	finish = offset + length;
 	if (finish > fru->size) {
+		memset(frubuf + fru->size, 0, length - fru->size);
 		finish = fru->size;
 		lprintf(LOG_NOTICE, "Read FRU Area length %d too large, "
 			"Adjusting to %d",
 			offset + length, finish - offset);
+		length = finish - offset;
 	}
 
 	memset(&req, 0, sizeof(req));
@@ -822,6 +841,8 @@ read_fru_area_section(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id,
 	if (fru->access && fru_data_rqst_size > 16)
 #endif
 		fru_data_rqst_size = 16;
+
+	size_left_in_buffer = length;
 	do {
 		tmp = fru->access ? off >> 1 : off;
 		msg_data[0] = id;
@@ -853,8 +874,16 @@ read_fru_area_section(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id,
 		}
 
 		tmp = fru->access ? rsp->data[0] << 1 : rsp->data[0];
+		if(rsp->data_len < 1
+		   || tmp > rsp->data_len - 1
+		   || tmp > size_left_in_buffer)
+		{
+			printf(" Not enough buffer size");
+			return -1;
+		}
 		memcpy((frubuf + off)-offset, rsp->data + 1, tmp);
 		off += tmp;
+		size_left_in_buffer -= tmp;
 
 		/* sometimes the size returned in the Info command
 		* is too large.  return 0 so higher level function
path: lib/ipmi_fru.c