[hddtemp-dev] hddtemp: String-overflow with long product identifiers

[Thomas Kindler]

Package: hddtemp
Version: 0.3-beta15-44
Severity: important
Tags: patch

*** Please type your report below this line ***

Hi!

I have an USB multi-card reader and found a string-overflow in hddtemp.
When a device reports a full 16-byte Product ID, the string won't get
zero-terminated.

Example:
  # hddtemp /dev/sdi
  /dev/sdi: Generic STORAGE DEVICE-A9727::::<@: S.M.A.R.T. not available
                                    ^^^^^^^^^^ overflow!!

(lsusb identifies the device as "ID 05e3:0716 Genesys Logic, Inc.")

A hex dump of the INQUIRY packet shows the following:

dumping 36 bytes from 0x7fffd9847e50
       0  1  2  3  4  5  6  7  8  9  A  B  C  D  E  F  0123456789ABCDEF
0000:  00-80 00-00 29-00 00-00 47-65 6E-65 72-69 63-20 ....)...Generic
0010:  53-54 4F-52 41-47 45-20 44-45 56-49 43-45 2D-41 STORAGE DEVICE-A
0020:  39-37 32-37   -     -     -     -     -     -   9727

According to SPC-2, the vendor is "Generic ", product "STORAGE DEVICE-A" 
and the revision level is "9727".

I've attached a patch that moves scsi_fixstring() into scsi_model (where 
it belongs), and truncates the model string to 24 bytes (Vendor+Product ID).


-- System Information:
Debian Release: 5.0
  APT prefers stable
  APT policy: (500, 'stable')
Architecture: amd64 (x86_64)

Kernel: Linux 2.6.26-1-amd64 (SMP w/2 CPU cores)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/bash

Versions of packages hddtemp depends on:
ii  debconf [debconf-2.0]         1.5.24     Debian configuration 
management sy
ii  libc6                         2.7-18     GNU C Library: Shared libraries
ii  lsb-base                      3.2-20     Linux Standard Base 3.2 
init scrip

hddtemp recommends no packages.

Versions of packages hddtemp suggests:
pn  ksensors                      <none>     (no description available)

-- debconf information excluded

--- scsicmds.c.orig     2009-02-23 00:16:15.000000000 +0100
+++ scsicmds.c  2009-02-22 23:49:50.000000000 +0100
@@ -44,27 +44,6 @@
 // Application specific includes
 #include "scsicmds.h"
 
-static void scsi_fixstring(unsigned char *s, int bytecount)
-{
-  unsigned char *p;
-  unsigned char *end;
-
-  p = s;
-  end = s + bytecount;
-
-  /* strip leading blanks */
-  while (s != end && *s == ' ')
-    ++s;
-  /* compress internal blanks and strip trailing blanks */
-  while (s != end && *s) {
-    if (*s++ != ' ' || (s != end && *s && *s != ' '))
-      *p++ = *(s-1);
-  }
-  /* wipe out trailing garbage */
-  while (p != end)
-    *p++ = '\0';
-}
-
 int scsi_SG_IO(int device, unsigned char *cdb, int cdb_len, unsigned char 
*buffer, int buffer_len, unsigned char *sense, unsigned char sense_len, int 
dxfer_direction) {
   struct sg_io_hdr io_hdr;
 
@@ -146,7 +125,6 @@
   if (scsi_command(device, cdb, sizeof(cdb), buffer, cdb[4], 
SG_DXFER_FROM_DEV) != 0)
     return 1;
   else {
-    scsi_fixstring(buffer + 8, 24);
     return 0;
   }
 }
--- scsi.c.orig 2009-02-23 00:16:06.000000000 +0100
+++ scsi.c      2009-02-22 23:52:17.000000000 +0100
@@ -57,12 +57,35 @@
     return 1;
 }
 
+static void scsi_fixstring(unsigned char *s, int bytecount)
+{
+  unsigned char *p;
+  unsigned char *end;
+
+  p = s;
+  end = s + bytecount;
+
+  /* strip leading blanks */
+  while (s != end && *s == ' ')
+    ++s;
+  /* compress internal blanks and strip trailing blanks */
+  while (s != end && *s) {
+    if (*s++ != ' ' || (s != end && *s && *s != ' '))
+      *p++ = *(s-1);
+  }
+  /* wipe out trailing garbage */
+  while (p != end)
+    *p++ = '\0';
+}
+
 static const char *scsi_model (int device) {
   unsigned char buf[36];
 
   if (scsi_inquiry(device, buf) != 0)
     return strdup(_("unknown"));
   else {
+    scsi_fixstring(buf + 8, 24);
+    buf[32] = '\0';
     return strdup(buf + 8);
   }
 }