1From e824c23316ae50beb7f7488f2055ac65e8b341f2 Mon Sep 17 00:00:00 2001 2From: Chrostoper Ertl <chertl@microsoft.com> 3Date: Thu, 28 Nov 2019 16:33:59 +0000 4Subject: [PATCH] fru: Fix buffer overflow vulnerabilities 5 6Partial fix for CVE-2020-5208, see 7https://github.com/ipmitool/ipmitool/security/advisories/GHSA-g659-9qxw-p7cp 8 9The `read_fru_area_section` function only performs size validation of 10requested read size, and falsely assumes that the IPMI message will not 11respond with more than the requested amount of data; it uses the 12unvalidated response size to copy into `frubuf`. If the response is 13larger than the request, this can result in overflowing the buffer. 14 15The same issue affects the `read_fru_area` function. 16 17Upstream-Status: Backport[https://github.com/ipmitool/ipmitool/commit/e824c23316ae50beb7f7488f2055ac65e8b341f2] 18CVE: CVE-2020-5208 19 20Signed-off-by: Wenlin Kang <wenlin.kang@windriver.com> 21--- 22 lib/ipmi_fru.c | 33 +++++++++++++++++++++++++++++++-- 23 1 file changed, 31 insertions(+), 2 deletions(-) 24 25diff --git a/lib/ipmi_fru.c b/lib/ipmi_fru.c 26index c2a139d..2e323ff 100644 27--- a/lib/ipmi_fru.c 28+++ b/lib/ipmi_fru.c 29@@ -663,7 +663,10 @@ int 30 read_fru_area(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id, 31 uint32_t offset, uint32_t length, uint8_t *frubuf) 32 { 33- uint32_t off = offset, tmp, finish; 34+ uint32_t off = offset; 35+ uint32_t tmp; 36+ uint32_t finish; 37+ uint32_t size_left_in_buffer; 38 struct ipmi_rs * rsp; 39 struct ipmi_rq req; 40 uint8_t msg_data[4]; 41@@ -676,10 +679,12 @@ read_fru_area(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id, 42 43 finish = offset + length; 44 if (finish > fru->size) { 45+ memset(frubuf + fru->size, 0, length - fru->size); 46 finish = fru->size; 47 lprintf(LOG_NOTICE, "Read FRU Area length %d too large, " 48 "Adjusting to %d", 49 offset + length, finish - offset); 50+ length = finish - offset; 51 } 52 53 memset(&req, 0, sizeof(req)); 54@@ -715,6 +720,7 @@ read_fru_area(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id, 55 } 56 } 57 58+ size_left_in_buffer = length; 59 do { 60 tmp = fru->access ? off >> 1 : off; 61 msg_data[0] = id; 62@@ -756,9 +762,18 @@ read_fru_area(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id, 63 } 64 65 tmp = fru->access ? rsp->data[0] << 1 : rsp->data[0]; 66+ if(rsp->data_len < 1 67+ || tmp > rsp->data_len - 1 68+ || tmp > size_left_in_buffer) 69+ { 70+ printf(" Not enough buffer size"); 71+ return -1; 72+ } 73+ 74 memcpy(frubuf, rsp->data + 1, tmp); 75 off += tmp; 76 frubuf += tmp; 77+ size_left_in_buffer -= tmp; 78 /* sometimes the size returned in the Info command 79 * is too large. return 0 so higher level function 80 * still attempts to parse what was returned */ 81@@ -791,7 +806,9 @@ read_fru_area_section(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id, 82 uint32_t offset, uint32_t length, uint8_t *frubuf) 83 { 84 static uint32_t fru_data_rqst_size = 20; 85- uint32_t off = offset, tmp, finish; 86+ uint32_t off = offset; 87+ uint32_t tmp, finish; 88+ uint32_t size_left_in_buffer; 89 struct ipmi_rs * rsp; 90 struct ipmi_rq req; 91 uint8_t msg_data[4]; 92@@ -804,10 +821,12 @@ read_fru_area_section(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id, 93 94 finish = offset + length; 95 if (finish > fru->size) { 96+ memset(frubuf + fru->size, 0, length - fru->size); 97 finish = fru->size; 98 lprintf(LOG_NOTICE, "Read FRU Area length %d too large, " 99 "Adjusting to %d", 100 offset + length, finish - offset); 101+ length = finish - offset; 102 } 103 104 memset(&req, 0, sizeof(req)); 105@@ -822,6 +841,8 @@ read_fru_area_section(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id, 106 if (fru->access && fru_data_rqst_size > 16) 107 #endif 108 fru_data_rqst_size = 16; 109+ 110+ size_left_in_buffer = length; 111 do { 112 tmp = fru->access ? off >> 1 : off; 113 msg_data[0] = id; 114@@ -853,8 +874,16 @@ read_fru_area_section(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id, 115 } 116 117 tmp = fru->access ? rsp->data[0] << 1 : rsp->data[0]; 118+ if(rsp->data_len < 1 119+ || tmp > rsp->data_len - 1 120+ || tmp > size_left_in_buffer) 121+ { 122+ printf(" Not enough buffer size"); 123+ return -1; 124+ } 125 memcpy((frubuf + off)-offset, rsp->data + 1, tmp); 126 off += tmp; 127+ size_left_in_buffer -= tmp; 128 129 /* sometimes the size returned in the Info command 130 * is too large. return 0 so higher level function 131-- 1322.17.1 133 134