1From d615cb6c39d401a569941be2a615176191afa7ac 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 17[Retrieve from 18https://github.com/ipmitool/ipmitool/commit/e824c23316ae50beb7f7488f2055ac65e8b341f2] 19Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com> 20--- 21 lib/ipmi_fru.c | 33 +++++++++++++++++++++++++++++++-- 22 1 file changed, 31 insertions(+), 2 deletions(-) 23 24diff --git a/lib/ipmi_fru.c b/lib/ipmi_fru.c 25index cf00eff..af99aa9 100644 26--- a/lib/ipmi_fru.c 27+++ b/lib/ipmi_fru.c 28@@ -615,7 +615,10 @@ int 29 read_fru_area(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id, 30 uint32_t offset, uint32_t length, uint8_t *frubuf) 31 { 32- uint32_t off = offset, tmp, finish; 33+ uint32_t off = offset; 34+ uint32_t tmp; 35+ uint32_t finish; 36+ uint32_t size_left_in_buffer; 37 struct ipmi_rs * rsp; 38 struct ipmi_rq req; 39 uint8_t msg_data[4]; 40@@ -628,10 +631,12 @@ read_fru_area(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id, 41 42 finish = offset + length; 43 if (finish > fru->size) { 44+ memset(frubuf + fru->size, 0, length - fru->size); 45 finish = fru->size; 46 lprintf(LOG_NOTICE, "Read FRU Area length %d too large, " 47 "Adjusting to %d", 48 offset + length, finish - offset); 49+ length = finish - offset; 50 } 51 52 memset(&req, 0, sizeof(req)); 53@@ -667,6 +672,7 @@ read_fru_area(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id, 54 } 55 } 56 57+ size_left_in_buffer = length; 58 do { 59 tmp = fru->access ? off >> 1 : off; 60 msg_data[0] = id; 61@@ -707,9 +713,18 @@ read_fru_area(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id, 62 } 63 64 tmp = fru->access ? rsp->data[0] << 1 : rsp->data[0]; 65+ if(rsp->data_len < 1 66+ || tmp > rsp->data_len - 1 67+ || tmp > size_left_in_buffer) 68+ { 69+ printf(" Not enough buffer size"); 70+ return -1; 71+ } 72+ 73 memcpy(frubuf, rsp->data + 1, tmp); 74 off += tmp; 75 frubuf += tmp; 76+ size_left_in_buffer -= tmp; 77 /* sometimes the size returned in the Info command 78 * is too large. return 0 so higher level function 79 * still attempts to parse what was returned */ 80@@ -742,7 +757,9 @@ read_fru_area_section(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id, 81 uint32_t offset, uint32_t length, uint8_t *frubuf) 82 { 83 static uint32_t fru_data_rqst_size = 20; 84- uint32_t off = offset, tmp, finish; 85+ uint32_t off = offset; 86+ uint32_t tmp, finish; 87+ uint32_t size_left_in_buffer; 88 struct ipmi_rs * rsp; 89 struct ipmi_rq req; 90 uint8_t msg_data[4]; 91@@ -755,10 +772,12 @@ read_fru_area_section(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id, 92 93 finish = offset + length; 94 if (finish > fru->size) { 95+ memset(frubuf + fru->size, 0, length - fru->size); 96 finish = fru->size; 97 lprintf(LOG_NOTICE, "Read FRU Area length %d too large, " 98 "Adjusting to %d", 99 offset + length, finish - offset); 100+ length = finish - offset; 101 } 102 103 memset(&req, 0, sizeof(req)); 104@@ -773,6 +792,8 @@ read_fru_area_section(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id, 105 if (fru->access && fru_data_rqst_size > 16) 106 #endif 107 fru_data_rqst_size = 16; 108+ 109+ size_left_in_buffer = length; 110 do { 111 tmp = fru->access ? off >> 1 : off; 112 msg_data[0] = id; 113@@ -804,8 +825,16 @@ read_fru_area_section(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id, 114 } 115 116 tmp = fru->access ? rsp->data[0] << 1 : rsp->data[0]; 117+ if(rsp->data_len < 1 118+ || tmp > rsp->data_len - 1 119+ || tmp > size_left_in_buffer) 120+ { 121+ printf(" Not enough buffer size"); 122+ return -1; 123+ } 124 memcpy((frubuf + off)-offset, rsp->data + 1, tmp); 125 off += tmp; 126+ size_left_in_buffer -= tmp; 127 128 /* sometimes the size returned in the Info command 129 * is too large. return 0 so higher level function 130-- 1312.20.1 132 133