admin/ipmitool/patches/0011-CVE-2020-5208-fru-sdr-Fix-id_string-buffer-overflows.patch

   1 From 98b47424cf548f58c4d295fa8235210406ea85ca Mon Sep 17 00:00:00 2001
   2 From: Chrostoper Ertl <chertl@microsoft.com>
   3 Date: Thu, 28 Nov 2019 17:13:45 +0000
   4 Subject: [PATCH 11/11] fru, sdr: Fix id_string buffer overflows
   5
   6 Final part of the fixes for CVE-2020-5208, see
   7 https://github.com/ipmitool/ipmitool/security/advisories/GHSA-g659-9qxw-p7cp
   8
   9 9 variants of stack buffer overflow when parsing `id_string` field of
  10 SDR records returned from `CMD_GET_SDR` command.
  11
  12 SDR record structs have an `id_code` field, and an `id_string` `char`
  13 array.
  14
  15 The length of `id_string` is calculated as `(id_code & 0x1f) + 1`,
  16 which can be larger than expected 16 characters (if `id_code = 0xff`,
  17 then length will be `(0xff & 0x1f) + 1 = 32`).
  18
  19 In numerous places, this can cause stack buffer overflow when copying
  20 into fixed buffer of size `17` bytes from this calculated length.
  21 ---
  22  lib/ipmi_fru.c |  2 +-
  23  lib/ipmi_sdr.c | 40 ++++++++++++++++++++++++----------------
  24  2 files changed, 25 insertions(+), 17 deletions(-)
  25
  26 diff --git a/lib/ipmi_fru.c b/lib/ipmi_fru.c
  27 index af99aa99444c..98bc9840955a 100644
  28 --- a/lib/ipmi_fru.c
  29 +++ b/lib/ipmi_fru.c
  30 @@ -3062,7 +3062,7 @@ ipmi_fru_print(struct ipmi_intf * intf, struct sdr_record_fru_locator * fru)
  31                 return 0;
  32
  33         memset(desc, 0, sizeof(desc));
  34 -       memcpy(desc, fru->id_string, fru->id_code & 0x01f);
  35 +       memcpy(desc, fru->id_string, __min(fru->id_code & 0x01f, sizeof(desc)));
  36         desc[fru->id_code & 0x01f] = 0;
  37         printf("FRU Device Description : %s (ID %d)\n", desc, fru->device_id);
  38
  39 diff --git a/lib/ipmi_sdr.c b/lib/ipmi_sdr.c
  40 index 2a9cbe3087af..62aac08a9002 100644
  41 --- a/lib/ipmi_sdr.c
  42 +++ b/lib/ipmi_sdr.c
  43 @@ -2084,7 +2084,7 @@ ipmi_sdr_print_sensor_eventonly(struct ipmi_intf *intf,
  44                 return -1;
  45
  46         memset(desc, 0, sizeof (desc));
  47 -       snprintf(desc, (sensor->id_code & 0x1f) + 1, "%s", sensor->id_string);
  48 +       snprintf(desc, sizeof(desc), "%.*s", (sensor->id_code & 0x1f) + 1, sensor->id_string);
  49
  50         if (verbose) {
  51                 printf("Sensor ID              : %s (0x%x)\n",
  52 @@ -2135,7 +2135,7 @@ ipmi_sdr_print_sensor_mc_locator(struct ipmi_intf *intf,
  53                 return -1;
  54
  55         memset(desc, 0, sizeof (desc));
  56 -       snprintf(desc, (mc->id_code & 0x1f) + 1, "%s", mc->id_string);
  57 +       snprintf(desc, sizeof(desc), "%.*s", (mc->id_code & 0x1f) + 1, mc->id_string);
  58
  59         if (verbose == 0) {
  60                 if (csv_output)
  61 @@ -2228,7 +2228,7 @@ ipmi_sdr_print_sensor_generic_locator(struct ipmi_intf *intf,
  62         char desc[17];
  63
  64         memset(desc, 0, sizeof (desc));
  65 -       snprintf(desc, (dev->id_code & 0x1f) + 1, "%s", dev->id_string);
  66 +       snprintf(desc, sizeof(desc), "%.*s", (dev->id_code & 0x1f) + 1, dev->id_string);
  67
  68         if (!verbose) {
  69                 if (csv_output)
  70 @@ -2285,7 +2285,7 @@ ipmi_sdr_print_sensor_fru_locator(struct ipmi_intf *intf,
  71         char desc[17];
  72
  73         memset(desc, 0, sizeof (desc));
  74 -       snprintf(desc, (fru->id_code & 0x1f) + 1, "%s", fru->id_string);
  75 +       snprintf(desc, sizeof(desc), "%.*s", (fru->id_code & 0x1f) + 1, fru->id_string);
  76
  77         if (!verbose) {
  78                 if (csv_output)
  79 @@ -2489,35 +2489,43 @@ ipmi_sdr_print_name_from_rawentry(struct ipmi_intf *intf, uint16_t id,
  80
  81     int rc =0;
  82     char desc[17];
  83 +   const char *id_string;
  84 +   uint8_t id_code;
  85     memset(desc, ' ', sizeof (desc));
  86
  87     switch ( type) {
  88        case SDR_RECORD_TYPE_FULL_SENSOR:
  89        record.full = (struct sdr_record_full_sensor *) raw;
  90 -      snprintf(desc, (record.full->id_code & 0x1f) +1, "%s",
  91 -               (const char *)record.full->id_string);
  92 +      id_code = record.full->id_code;
  93 +      id_string = record.full->id_string;
  94        break;
  95 +
  96        case SDR_RECORD_TYPE_COMPACT_SENSOR:
  97        record.compact = (struct sdr_record_compact_sensor *) raw        ;
  98 -      snprintf(desc, (record.compact->id_code & 0x1f)  +1, "%s",
  99 -               (const char *)record.compact->id_string);
 100 +      id_code = record.compact->id_code;
 101 +      id_string = record.compact->id_string;
 102        break;
 103 +
 104        case SDR_RECORD_TYPE_EVENTONLY_SENSOR:
 105        record.eventonly  = (struct sdr_record_eventonly_sensor *) raw ;
 106 -      snprintf(desc, (record.eventonly->id_code & 0x1f)  +1, "%s",
 107 -               (const char *)record.eventonly->id_string);
 108 -      break;
 109 +      id_code = record.eventonly->id_code;
 110 +      id_string = record.eventonly->id_string;
 111 +      break;
 112 +
 113        case SDR_RECORD_TYPE_MC_DEVICE_LOCATOR:
 114        record.mcloc  = (struct sdr_record_mc_locator *) raw ;
 115 -      snprintf(desc, (record.mcloc->id_code & 0x1f)  +1, "%s",
 116 -               (const char *)record.mcloc->id_string);
 117 +      id_code = record.mcloc->id_code;
 118 +      id_string = record.mcloc->id_string;
 119        break;
 120 +
 121        default:
 122        rc = -1;
 123 -      break;
 124 -   }
 125 +   }
 126 +   if (!rc) {
 127 +       snprintf(desc, sizeof(desc), "%.*s", (id_code & 0x1f) + 1, id_string);
 128 +   }
 129
 130 -      lprintf(LOG_INFO, "ID: 0x%04x , NAME: %-16s", id, desc);
 131 +   lprintf(LOG_INFO, "ID: 0x%04x , NAME: %-16s", id, desc);
 132     return rc;
 133  }
 134
 135 --
 136 2.27.0
 137