1*4882a593SmuzhiyunFrom d615cb6c39d401a569941be2a615176191afa7ac Mon Sep 17 00:00:00 2001
2*4882a593SmuzhiyunFrom: Chrostoper Ertl <chertl@microsoft.com>
3*4882a593SmuzhiyunDate: Thu, 28 Nov 2019 16:33:59 +0000
4*4882a593SmuzhiyunSubject: [PATCH] fru: Fix buffer overflow vulnerabilities
5*4882a593Smuzhiyun
6*4882a593SmuzhiyunPartial fix for CVE-2020-5208, see
7*4882a593Smuzhiyunhttps://github.com/ipmitool/ipmitool/security/advisories/GHSA-g659-9qxw-p7cp
8*4882a593Smuzhiyun
9*4882a593SmuzhiyunThe `read_fru_area_section` function only performs size validation of
10*4882a593Smuzhiyunrequested read size, and falsely assumes that the IPMI message will not
11*4882a593Smuzhiyunrespond with more than the requested amount of data; it uses the
12*4882a593Smuzhiyununvalidated response size to copy into `frubuf`. If the response is
13*4882a593Smuzhiyunlarger than the request, this can result in overflowing the buffer.
14*4882a593Smuzhiyun
15*4882a593SmuzhiyunThe same issue affects the `read_fru_area` function.
16*4882a593Smuzhiyun
17*4882a593Smuzhiyun[Retrieve from
18*4882a593Smuzhiyunhttps://github.com/ipmitool/ipmitool/commit/e824c23316ae50beb7f7488f2055ac65e8b341f2]
19*4882a593SmuzhiyunSigned-off-by: Heiko Thiery <heiko.thiery@gmail.com>
20*4882a593Smuzhiyun---
21*4882a593Smuzhiyun lib/ipmi_fru.c | 33 +++++++++++++++++++++++++++++++--
22*4882a593Smuzhiyun 1 file changed, 31 insertions(+), 2 deletions(-)
23*4882a593Smuzhiyun
24*4882a593Smuzhiyundiff --git a/lib/ipmi_fru.c b/lib/ipmi_fru.c
25*4882a593Smuzhiyunindex cf00eff..af99aa9 100644
26*4882a593Smuzhiyun--- a/lib/ipmi_fru.c
27*4882a593Smuzhiyun+++ b/lib/ipmi_fru.c
28*4882a593Smuzhiyun@@ -615,7 +615,10 @@ int
29*4882a593Smuzhiyun read_fru_area(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id,
30*4882a593Smuzhiyun 			uint32_t offset, uint32_t length, uint8_t *frubuf)
31*4882a593Smuzhiyun {
32*4882a593Smuzhiyun-	uint32_t off = offset, tmp, finish;
33*4882a593Smuzhiyun+	uint32_t off = offset;
34*4882a593Smuzhiyun+	uint32_t tmp;
35*4882a593Smuzhiyun+	uint32_t finish;
36*4882a593Smuzhiyun+	uint32_t size_left_in_buffer;
37*4882a593Smuzhiyun 	struct ipmi_rs * rsp;
38*4882a593Smuzhiyun 	struct ipmi_rq req;
39*4882a593Smuzhiyun 	uint8_t msg_data[4];
40*4882a593Smuzhiyun@@ -628,10 +631,12 @@ read_fru_area(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id,
41*4882a593Smuzhiyun
42*4882a593Smuzhiyun 	finish = offset + length;
43*4882a593Smuzhiyun 	if (finish > fru->size) {
44*4882a593Smuzhiyun+		memset(frubuf + fru->size, 0, length - fru->size);
45*4882a593Smuzhiyun 		finish = fru->size;
46*4882a593Smuzhiyun 		lprintf(LOG_NOTICE, "Read FRU Area length %d too large, "
47*4882a593Smuzhiyun 			"Adjusting to %d",
48*4882a593Smuzhiyun 			offset + length, finish - offset);
49*4882a593Smuzhiyun+		length = finish - offset;
50*4882a593Smuzhiyun 	}
51*4882a593Smuzhiyun
52*4882a593Smuzhiyun 	memset(&req, 0, sizeof(req));
53*4882a593Smuzhiyun@@ -667,6 +672,7 @@ read_fru_area(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id,
54*4882a593Smuzhiyun 		}
55*4882a593Smuzhiyun 	}
56*4882a593Smuzhiyun
57*4882a593Smuzhiyun+	size_left_in_buffer = length;
58*4882a593Smuzhiyun 	do {
59*4882a593Smuzhiyun 		tmp = fru->access ? off >> 1 : off;
60*4882a593Smuzhiyun 		msg_data[0] = id;
61*4882a593Smuzhiyun@@ -707,9 +713,18 @@ read_fru_area(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id,
62*4882a593Smuzhiyun 		}
63*4882a593Smuzhiyun
64*4882a593Smuzhiyun 		tmp = fru->access ? rsp->data[0] << 1 : rsp->data[0];
65*4882a593Smuzhiyun+		if(rsp->data_len < 1
66*4882a593Smuzhiyun+		   || tmp > rsp->data_len - 1
67*4882a593Smuzhiyun+		   || tmp > size_left_in_buffer)
68*4882a593Smuzhiyun+		{
69*4882a593Smuzhiyun+			printf(" Not enough buffer size");
70*4882a593Smuzhiyun+			return -1;
71*4882a593Smuzhiyun+		}
72*4882a593Smuzhiyun+
73*4882a593Smuzhiyun 		memcpy(frubuf, rsp->data + 1, tmp);
74*4882a593Smuzhiyun 		off += tmp;
75*4882a593Smuzhiyun 		frubuf += tmp;
76*4882a593Smuzhiyun+		size_left_in_buffer -= tmp;
77*4882a593Smuzhiyun 		/* sometimes the size returned in the Info command
78*4882a593Smuzhiyun 		* is too large.  return 0 so higher level function
79*4882a593Smuzhiyun 		* still attempts to parse what was returned */
80*4882a593Smuzhiyun@@ -742,7 +757,9 @@ read_fru_area_section(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id,
81*4882a593Smuzhiyun 			uint32_t offset, uint32_t length, uint8_t *frubuf)
82*4882a593Smuzhiyun {
83*4882a593Smuzhiyun 	static uint32_t fru_data_rqst_size = 20;
84*4882a593Smuzhiyun-	uint32_t off = offset, tmp, finish;
85*4882a593Smuzhiyun+	uint32_t off = offset;
86*4882a593Smuzhiyun+	uint32_t tmp, finish;
87*4882a593Smuzhiyun+	uint32_t size_left_in_buffer;
88*4882a593Smuzhiyun 	struct ipmi_rs * rsp;
89*4882a593Smuzhiyun 	struct ipmi_rq req;
90*4882a593Smuzhiyun 	uint8_t msg_data[4];
91*4882a593Smuzhiyun@@ -755,10 +772,12 @@ read_fru_area_section(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id,
92*4882a593Smuzhiyun
93*4882a593Smuzhiyun 	finish = offset + length;
94*4882a593Smuzhiyun 	if (finish > fru->size) {
95*4882a593Smuzhiyun+		memset(frubuf + fru->size, 0, length - fru->size);
96*4882a593Smuzhiyun 		finish = fru->size;
97*4882a593Smuzhiyun 		lprintf(LOG_NOTICE, "Read FRU Area length %d too large, "
98*4882a593Smuzhiyun 			"Adjusting to %d",
99*4882a593Smuzhiyun 			offset + length, finish - offset);
100*4882a593Smuzhiyun+		length = finish - offset;
101*4882a593Smuzhiyun 	}
102*4882a593Smuzhiyun
103*4882a593Smuzhiyun 	memset(&req, 0, sizeof(req));
104*4882a593Smuzhiyun@@ -773,6 +792,8 @@ read_fru_area_section(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id,
105*4882a593Smuzhiyun 	if (fru->access && fru_data_rqst_size > 16)
106*4882a593Smuzhiyun #endif
107*4882a593Smuzhiyun 		fru_data_rqst_size = 16;
108*4882a593Smuzhiyun+
109*4882a593Smuzhiyun+	size_left_in_buffer = length;
110*4882a593Smuzhiyun 	do {
111*4882a593Smuzhiyun 		tmp = fru->access ? off >> 1 : off;
112*4882a593Smuzhiyun 		msg_data[0] = id;
113*4882a593Smuzhiyun@@ -804,8 +825,16 @@ read_fru_area_section(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id,
114*4882a593Smuzhiyun 		}
115*4882a593Smuzhiyun
116*4882a593Smuzhiyun 		tmp = fru->access ? rsp->data[0] << 1 : rsp->data[0];
117*4882a593Smuzhiyun+		if(rsp->data_len < 1
118*4882a593Smuzhiyun+		   || tmp > rsp->data_len - 1
119*4882a593Smuzhiyun+		   || tmp > size_left_in_buffer)
120*4882a593Smuzhiyun+		{
121*4882a593Smuzhiyun+			printf(" Not enough buffer size");
122*4882a593Smuzhiyun+			return -1;
123*4882a593Smuzhiyun+		}
124*4882a593Smuzhiyun 		memcpy((frubuf + off)-offset, rsp->data + 1, tmp);
125*4882a593Smuzhiyun 		off += tmp;
126*4882a593Smuzhiyun+		size_left_in_buffer -= tmp;
127*4882a593Smuzhiyun
128*4882a593Smuzhiyun 		/* sometimes the size returned in the Info command
129*4882a593Smuzhiyun 		* is too large.  return 0 so higher level function
130*4882a593Smuzhiyun--
131*4882a593Smuzhiyun2.20.1
132*4882a593Smuzhiyun
133