Bug#950761: RFS: ipmitool/1.8.18-11 [RC] -- utility for IPMI control with kernel driver or LAN interface (daemon)

2021-02-19 Thread Thomas Goirand
Hi Jorg and the security team,

Please find attached debdiff for the upload in buster-security.
Salvatore, would you accept such upload?

Cheers,

Thomas Goirand (zigo)
diff -Nru ipmitool-1.8.18/debian/changelog ipmitool-1.8.18/debian/changelog
--- ipmitool-1.8.18/debian/changelog2018-08-05 12:20:50.0 +0200
+++ ipmitool-1.8.18/debian/changelog2021-02-19 11:30:06.0 +0100
@@ -1,3 +1,18 @@
+ipmitool (1.8.18-6+deb10u1) buster-security; urgency=medium
+
+  * Non-maintainer upload.
+  * CVE-2020-5208: buffer overflows and potentially to remote code execution.
+Applied upstream patches:
+- CVE-2020-5208_1_Fix_buffer_overflow_vulnerabilities.patch
+- CVE-2020-5208_2-fru-Fix-buffer-overflow-in-ipmi_spd_print_fru.patch
+- 
CVE-2020-5208_3-session-Fix-buffer-overflow-in-ipmi_get_session_info.patch
+- CVE-2020-5208_4-channel-Fix-buffer-overflow.patch
+- CVE-2020-5208_5_lanp-Fix-buffer-overflows-in-get_lan_param_select.patch
+- CVE-2020-5208_6-fru-sdr-Fix-id_string-buffer-overflows.patch
+(Closes: #950761).
+
+ -- Thomas Goirand   Fri, 19 Feb 2021 11:30:06 +0100
+
 ipmitool (1.8.18-6) unstable; urgency=medium
 
   * debian/changelog:
diff -Nru 
ipmitool-1.8.18/debian/patches/CVE-2020-5208_1_Fix_buffer_overflow_vulnerabilities.patch
 
ipmitool-1.8.18/debian/patches/CVE-2020-5208_1_Fix_buffer_overflow_vulnerabilities.patch
--- 
ipmitool-1.8.18/debian/patches/CVE-2020-5208_1_Fix_buffer_overflow_vulnerabilities.patch
1970-01-01 01:00:00.0 +0100
+++ 
ipmitool-1.8.18/debian/patches/CVE-2020-5208_1_Fix_buffer_overflow_vulnerabilities.patch
2021-02-19 11:27:50.0 +0100
@@ -0,0 +1,120 @@
+Description: 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.
+Author: Chrostoper Ertl 
+Date: Thu, 28 Nov 2019 16:33:59 +
+
+Index: ipmitool-1.8.18/lib/ipmi_fru.c
+===
+--- ipmitool-1.8.18.orig/lib/ipmi_fru.c
 ipmitool-1.8.18/lib/ipmi_fru.c
+@@ -615,7 +615,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];
+@@ -628,10 +631,12 @@ read_fru_area(struct ipmi_intf * intf, s
+ 
+   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(, 0, sizeof(req));
+@@ -667,6 +672,7 @@ read_fru_area(struct ipmi_intf * intf, s
+   }
+   }
+ 
++  size_left_in_buffer = length;
+   do {
+   tmp = fru->access ? off >> 1 : off;
+   msg_data[0] = id;
+@@ -707,9 +713,18 @@ read_fru_area(struct ipmi_intf * intf, s
+   }
+ 
+   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 */
+@@ -742,7 +757,9 @@ read_fru_area_section(struct ipmi_intf *
+   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];
+@@ -755,10 +772,12 @@ read_fru_area_section(struct ipmi_intf *
+ 
+   finish = offset + length;
+   if (finish > fru->size) {

Bug#950761: RFS: ipmitool/1.8.18-11 [RC] -- utility for IPMI control with kernel driver or LAN interface (daemon)

2021-02-19 Thread Thomas Goirand
Hi Jörg,

I had a quick look at the patches and resulting code base. Without
looking a lot, it all looks like easy to understand supplementary
checks, and I didn't see anything that would break the code without also
breaking building. I may be wrong but it feels kind of safe to upload.

Also, we're not seeing any 1.8.19 release coming from upstream, and I
don't think it's reasonable, at this point, to wait further.

As time is passing and we aren't seeing any reply from you, and as this
is an RC bug, with a CVE involved, I'm uploading the debdiff I attached
in my previous reply to the DELAYED/5 queue. If you feel it's not a good
idea, you therefore have 5 days to let me know, and I'll either cancel
my upload, or sponsor the upload of any better version you may produce.

Also, it'd be nice to work on a fix for Buster. Did you do any work for
it? If you don't have the intention to do it, please let everyone know,
so we can deal with the release team to prepare an update for the next
point release.

Cheers,

Thomas Goirand (zigo)



Bug#950761: RFS: ipmitool/1.8.18-11 [RC] -- utility for IPMI control with kernel driver or LAN interface (daemon)

2021-02-08 Thread Thomas Goirand
Hi Jorg,

I've applied the upstream patches to the current codebase of Ipmitool in
Unstable. Please find the attched debdiff.

The only place where I had to rework things a little bit was
CVE-2020-5208_4-channel-Fix-buffer-overflow.patch where the constant
MAX_CIPHER_SUITE_DATA_LEN was not present in
include/ipmitool/ipmi_channel.h, so I took it from the top of the Git
upstream.

Where is the other place that you spotted that wouldn't work with the
current codebase? I didn't take the necessary time to even read the
other patches yet. (which makes this debdiff only half of the work).

Your thoughts?
Cheers,

Thomas Goirand (zigo)
diff -Nru ipmitool-1.8.18/debian/changelog ipmitool-1.8.18/debian/changelog
--- ipmitool-1.8.18/debian/changelog2020-09-14 14:35:04.0 +0200
+++ ipmitool-1.8.18/debian/changelog2021-02-08 17:57:56.0 +0100
@@ -1,3 +1,18 @@
+ipmitool (1.8.18-10.1) unstable; urgency=high
+
+  * Non-maintainer upload.
+  * CVE-2020-5208: buffer overflows and potentially to remote code execution.
+Applied upstream patches:
+- CVE-2020-5208_1_Fix_buffer_overflow_vulnerabilities.patch
+- CVE-2020-5208_2-fru-Fix-buffer-overflow-in-ipmi_spd_print_fru.patch
+- 
CVE-2020-5208_3-session-Fix-buffer-overflow-in-ipmi_get_session_info.patch
+- CVE-2020-5208_4-channel-Fix-buffer-overflow.patch
+- CVE-2020-5208_5_lanp-Fix-buffer-overflows-in-get_lan_param_select.patch
+- CVE-2020-5208_6-fru-sdr-Fix-id_string-buffer-overflows.patch
+(Closes: #950761).
+
+ -- Thomas Goirand   Mon, 08 Feb 2021 17:57:56 +0100
+
 ipmitool (1.8.18-10) unstable; urgency=medium
 
   * Add "Restrictions: superficial" to debian/tests/control (Closes: #969834).
diff -Nru 
ipmitool-1.8.18/debian/patches/CVE-2020-5208_1_Fix_buffer_overflow_vulnerabilities.patch
 
ipmitool-1.8.18/debian/patches/CVE-2020-5208_1_Fix_buffer_overflow_vulnerabilities.patch
--- 
ipmitool-1.8.18/debian/patches/CVE-2020-5208_1_Fix_buffer_overflow_vulnerabilities.patch
1970-01-01 01:00:00.0 +0100
+++ 
ipmitool-1.8.18/debian/patches/CVE-2020-5208_1_Fix_buffer_overflow_vulnerabilities.patch
2021-02-08 17:57:56.0 +0100
@@ -0,0 +1,120 @@
+Description: 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.
+Author: Chrostoper Ertl 
+Date: Thu, 28 Nov 2019 16:33:59 +
+
+Index: ipmitool-1.8.18/lib/ipmi_fru.c
+===
+--- ipmitool-1.8.18.orig/lib/ipmi_fru.c
 ipmitool-1.8.18/lib/ipmi_fru.c
+@@ -615,7 +615,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];
+@@ -628,10 +631,12 @@ read_fru_area(struct ipmi_intf * intf, s
+ 
+   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(, 0, sizeof(req));
+@@ -667,6 +672,7 @@ read_fru_area(struct ipmi_intf * intf, s
+   }
+   }
+ 
++  size_left_in_buffer = length;
+   do {
+   tmp = fru->access ? off >> 1 : off;
+   msg_data[0] = id;
+@@ -707,9 +713,18 @@ read_fru_area(struct ipmi_intf * intf, s
+   }
+ 
+   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 */
+@@ -742,7 +757,9 @@ 

Bug#950761: RFS: ipmitool/1.8.18-11 [RC] -- utility for IPMI control with kernel driver or LAN interface (daemon)

2021-02-07 Thread Salvatore Bonaccorso
Hi Jörg,

On Sun, Jan 03, 2021 at 05:21:42PM +0100, Jörg Frings-Fürst wrote:
> tags 950761 - pending
> thanks
> 
> Hello Salvatore,
> hello @All,
> 
> 
> following a tip from Salvatore, I have added the missing commits.
> Although these can be incorporated manually, they are not reliably
> functional in at least two places due to the different code base.
> 
> I think the best thing will be to wait for the next upstream release.
> 
> I am therefore withdrawing the RFS bug again.

Did you had a chance to look into these? (Or alternatively to rebase
to the new upstream version)?

Regards,
Salvatore



Bug#950761: RFS: ipmitool/1.8.18-11 [RC] -- utility for IPMI control with kernel driver or LAN interface (daemon)

2021-01-26 Thread Thomas Goirand
Hi Jorg,

Do you need sponsoring for the upload of the last upstream version in
unstable? Can we see this moving forward?

Cheers,

Thomas Goirand (zigo)



Bug#950761: RFS: ipmitool/1.8.18-11 [RC] -- utility for IPMI control with kernel driver or LAN interface (daemon)

2021-01-05 Thread Salvatore Bonaccorso
Hi Jörg,

Thanks a lot for your work on this package!

On Sun, Jan 03, 2021 at 05:21:42PM +0100, Jörg Frings-Fürst wrote:
> tags 950761 - pending
> thanks
> 
> Hello Salvatore,
> hello @All,
> 
> 
> following a tip from Salvatore, I have added the missing commits.
> Although these can be incorporated manually, they are not reliably
> functional in at least two places due to the different code base.
> 
> I think the best thing will be to wait for the next upstream release.
> 
> I am therefore withdrawing the RFS bug again.

Would it be feasible (at this stage before the freeze for bullseye) to
just rebase to the 1.8.19 release? Or are you aiming at properly
rebase those patchset to 1.8.18?

Regards,
Salvatore



Bug#950761: RFS: ipmitool/1.8.18-11 [RC] -- utility for IPMI control with kernel driver or LAN interface (daemon)

2021-01-03 Thread Jörg Frings-Fürst
tags 950761 - pending
thanks

Hello Salvatore,
hello @All,


following a tip from Salvatore, I have added the missing commits.
Although these can be incorporated manually, they are not reliably
functional in at least two places due to the different code base.

I think the best thing will be to wait for the next upstream release.

I am therefore withdrawing the RFS bug again.


CU
Jörg
-- 
New:
GPG Fingerprint: 63E0 075F C8D4 3ABB 35AB  30EE 09F8 9F3C 8CA1 D25D
GPG key (long) : 09F89F3C8CA1D25D
GPG Key: 8CA1D25D
CAcert Key S/N : 0E:D4:56

Old pgp Key: BE581B6E (revoked since 2014-12-31).

Jörg Frings-Fürst
D-54470 Lieser


git:  https://jff.email/cgit/

Threema: SYR8SJXB
Wire: @joergfringsfuerst
Skype: joergpenguin
Ring: jff
Telegram: @joergfringsfuerst


My wish list: 
 - Please send me a picture from the nature at your home.



signature.asc
Description: This is a digitally signed message part