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