String-overflow with long product identifiers Hi! I have a no-name USB multi-card reader, and found a string-overflow in hddtemp 0.3-beta15. # 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.") When a device reports a full 16-byte Product ID, the string won't get zero-terminated. 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". Here's a patch: --- 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); } } I have moved the scsi_fixstring() into scsi_model (where it belongs), and truncate the model string to 24 bytes (Vendor + Product ID). greetings, -- Thomas Kindler