[flashrom] [commit] r1824 - trunk

[repository service]

Author: stefanct
Date: Sun Jul 13 14:52:15 2014
New Revision: 1824
URL: http://flashrom.org/trac/flashrom/changeset/1824

Log:
Fix garbage handling in DMI strings.

Previously we tried to replace garbage characters with <space> directly in
the read-only memory-mapped SMBIOS area(!). This could never have
worked for any DMI strings with garbage and results in a segfault on
machines with such strings.

Thanks to Brian Rak (Supermicro X10SLE-F) and John Pohlman (HP XW9400)
for reporting this issue.

With this patch the strings are duplicated within dmi_string() already,
just before we sanitize them. Also, the limit variable used everywhere
points to the first invalid byte address. Refine respective checks
accordingly.

Signed-off-by: Stefan Tauner 
Acked-by: Carl-Daniel Hailfinger

Modified:
   trunk/dmi.c

Modified: trunk/dmi.c
==============================================================================
--- trunk/dmi.c	Sun Jul 13 02:05:07 2014	(r1823)
+++ trunk/dmi.c	Sun Jul 13 14:52:15 2014	(r1824)
@@ -23,6 +23,7 @@

#include <strings.h>
 #include <string.h>
+#include <ctype.h>
 #include <stdio.h>
 #include <stdlib.h>

@@ -105,7 +106,16 @@
    return (sum == 0);
 }

-static char *dmi_string(uint8_t *buf, uint8_t string_id, uint8_t *limit)
+/** Retrieve a DMI string.
+ *
+ * See SMBIOS spec. section 6.1.3 "Text strings".
+ * The table will be unmapped ASAP, hence return a duplicated & sanitized string that needs to be freed later.
+ *
+ * \param buf		the buffer to search through (usually appended directly to a DMI structure)
+ * \param string_id	index of the string to look for
+ * \param limit		pointer to the first byte beyond \em buf
+ */
+static char *dmi_string(const char *buf, uint8_t string_id, const char *limit)
 {
    size_t i, len;

@@ -113,26 +123,33 @@
    	return "Not Specified";

while (string_id > 1 && string_id--) {
-		if (buf > limit) {
+		if (buf >= limit) {
    		msg_perr("DMI table is broken (string portion out of bounds)!\n");
    		return "<OUT OF BOUNDS>";
    	}
-		buf += strnlen((char *)buf, limit - buf) + 1;
+		buf += strnlen(buf, limit - buf) + 1;
    }

if (!*buf) /* as long as the current byte we're on isn't null */
    	return "<BAD INDEX>";

-	len = strnlen((char *)buf, limit - buf);
-	if (len > DMI_MAX_ANSWER_LEN)
-		len = DMI_MAX_ANSWER_LEN;
+	len = strnlen(buf, limit - buf);
+	char *newbuf = malloc(len + 1);
+	if (newbuf == NULL) {
+		msg_perr("Out of memory!\n");
+		return NULL;
+	}

/* fix junk bytes in the string */
-	for (i = 0; i < len; i++)
-		if (buf[i] < 32 || buf[i] >= 127)
-			buf[i] = ' ';
+	for (i = 0; i < len && buf[i] != '\0'; i++) {
+		if (isprint(buf[i]))
+			newbuf[i] = buf[i];
+		else
+			newbuf[i] = ' ';
+	}
+	newbuf[i] = '\0';

-	return (char *)buf;
+	return newbuf;
 }

static void dmi_chassis_type(uint8_t code)
@@ -167,7 +184,7 @@
     *  - uint8_t length;	// data section w/ header w/o strings
     *  - uint16_t handle;
     */
-	while (i < num && data + 4 <= limit) {
+	while (i < num && data + 4 < limit) {
    	/* - If a short entry is found (less than 4 bytes), not only it
    	 *   is invalid, but we cannot reliably locate the next entry.
    	 * - If the length value indicates that this structure spreads
@@ -175,15 +192,16 @@
    	 * Better stop at this point, and let the user know his/her
    	 * table is broken.
    	 */
-		if (data[1] < 4 || data + data[1] > limit) {
+		if (data[1] < 4 || data + data[1] >= limit) {
    		msg_perr("DMI table is broken (bogus header)!\n");
    		break;
    	}

if(data[0] == 3) {
-			if (data + 5 <= limit)
+			if (data + 5 < limit)
    			dmi_chassis_type(data[5]);
-			/* else the table is broken, but laptop detection is optional, hence continue. */
+			else /* the table is broken, but laptop detection is optional, hence continue. */
+				msg_pwarn("DMI table is broken (chassis_type out of bounds)!\n");
    	} else
    		for (j = 0; j < ARRAY_SIZE(dmi_strings); j++) {
    			uint8_t offset = dmi_strings[j].offset;
@@ -192,13 +210,13 @@
    			if (data[0] != type)
    				continue;

-				if (data[1] <= offset || data+offset > limit) {
+				if (data[1] <= offset || data + offset >= limit) {
    				msg_perr("DMI table is broken (offset out of bounds)!\n");
    				goto out;
    			}

-				/* Table will be unmapped, hence fill the struct with duplicated strings. */
-				dmi_strings[j].value = strdup(dmi_string(data + data[1], data[offset], limit));
+				dmi_strings[j].value = dmi_string((const char *)(data + data[1]), data[offset],
+								  (const char *)limit);
    		}
    	/* Find next structure by skipping data and string sections */
    	data += data[1];