Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 22 Oct 2002 14:20:41 +0000
From:      mm <meadele@nerim.net>
To:        freebsd-bugs@freebsd.org
Subject:   libutil: property.c bug (properties_read)
Message-ID:  <3DB55EB9.5000900@nerim.net>

next in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format.
--------------040306070003060106090801
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit

Hi,

There is a bad boundary check in properties_read() in libutil when 
parsing 'name=value'.

I patched property.c and added some corrections:
   - corrected bad boundary check.
   - ignore characters after space unless value is enclosed with brackets.
   - ignore characters after terminating bracket.
   - check for malloc/strdup return value.

The attached path applies on /usr/src/lib/libutil/property.c shipped 
with FreeBSD 4.7-STABLE.


--------------040306070003060106090801
Content-Type: text/plain;
 name="property.c.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="property.c.patch"

--- property.c	Mon Oct 21 18:50:44 2002
+++ property.c	Tue Oct 22 13:45:20 2002
@@ -47,9 +47,23 @@
     properties n;
 
     n = (properties)malloc(sizeof(struct _property));
+    if (n == NULL)
+        return NULL;
     n->next = NULL;
-    n->name = name ? strdup(name) : NULL;
-    n->value = value ? strdup(value) : NULL;
+    if (name) {
+        n->name = strdup(name);
+        if (n->name == NULL)
+            return NULL;
+    }
+    else
+       n->name = NULL;
+    if (value) {
+        n->value = strdup(value);
+        if (n->value == NULL)
+            return NULL;
+    }
+    else
+       n->value = NULL;
     return n;
 }
 
@@ -62,7 +76,7 @@
     char buf[BUFSIZ * 4];
     int bp, n, v, max;
     enum { LOOK, COMMENT, NAME, VALUE, MVALUE, COMMIT, FILL, STOP } state;
-    int ch = 0, blevel = 0;
+    int ch = 0, blevel = 0, ign = 0;
 
     n = v = bp = max = 0;
     head = ptr = NULL;
@@ -91,38 +105,28 @@
 	    if (isspace(ch))
 		continue;
 	    /* Allow shell or lisp style comments */
-	    else if (ch == '#' || ch == ';') {
+	    else if (ch == '#' || ch == ';' ||
+ 		     (!isalnum(ch) && ch != '_')) {
 		state = COMMENT;
 		continue;
 	    }
-	    else if (isalnum(ch) || ch == '_') {
-		if (n >= PROPERTY_MAX_NAME) {
-		    n = 0;
-		    state = COMMENT;
-		}
-		else {
-		    hold_n[n++] = ch;
-		    state = NAME;
-		}
-	    }
-	    else
-		state = COMMENT;	/* Ignore the rest of the line */
-	    break;
-
-	case COMMENT:
-	    if (ch == '\n')
-		state = LOOK;
-	    break;
+	    state = NAME;
+            /* fall through */
 
 	case NAME:
-	    if (ch == '\n' || !ch) {
+	    if (ch == '\n') {
 		hold_n[n] = '\0';
 		hold_v[0] = '\0';
 		v = n = 0;
 		state = COMMIT;
 	    }
-	    else if (isspace(ch))
-		continue;
+	    else if (n == PROPERTY_MAX_NAME) {
+	        warnx("properties_read: name exceeds max length");
+	        state = COMMENT;
+	        n = v = 0;
+	    }
+	    else if (isspace(ch)) 
+	        continue;
 	    else if (ch == '=') {
 		hold_n[n] = '\0';
 		v = n = 0;
@@ -133,44 +137,45 @@
 	    break;
 
 	case VALUE:
-	    if (v == 0 && ch == '\n') {
+	    if (ch == '\n') {
 	        hold_v[v] = '\0';
-	        v = n = 0;
+	        ign = v = n = 0;
 	        state = COMMIT;
 	    } 
-	    else if (v == 0 && isspace(ch))
-		continue;
-	    else if (ch == '{') {
-		state = MVALUE;
-		++blevel;
+	    else if (isspace(ch) || ign) {
+		if (v != 0) 
+		    ign = 1;  /* skip trailing characters */
+	        continue;
 	    }
-	    else if (ch == '\n' || !ch) {
-		hold_v[v] = '\0';
+	    else if (v == PROPERTY_MAX_VALUE) {
+		warnx("properties_read: value exceeds max length");
+		state = COMMENT;
 		v = n = 0;
-		state = COMMIT;
 	    }
-	    else {
-		if (v >= PROPERTY_MAX_VALUE) {
-		    state = COMMENT;
-		    v = n = 0;
-		    break;
-		}
-		else
-		    hold_v[v++] = ch;
+	    else if (v == 0 && ch == '{') {
+		    state = MVALUE;
+		    ++blevel;
 	    }
+	    else 
+		hold_v[v++] = ch;
 	    break;
 
 	case MVALUE:
 	    /* multiline value */
-	    if (v >= PROPERTY_MAX_VALUE) {
-		warn("properties_read: value exceeds max length");
-		state = COMMENT;
-		n = v = 0;
-	    }
+            if (blevel == 0) {      /* skip trailing characters */
+	        if (ch == '\n')
+	            state = COMMIT;
+		else
+		    continue;
+            }
 	    else if (ch == '}' && !--blevel) {
 		hold_v[v] = '\0';
-		v = n = 0;
-		state = COMMIT;
+		blevel = v = n = 0;
+	    }
+	    else if (v == PROPERTY_MAX_VALUE) {
+		warnx("properties_read: value exceeds max length");
+		state = COMMENT;
+		blevel = n = v = 0;
 	    }
 	    else {
 		hold_v[v++] = ch;
@@ -180,14 +185,26 @@
 	    break;
 
 	case COMMIT:
-	    if (!head)
+	    if (!head) {
 		head = ptr = property_alloc(hold_n, hold_v);
+                if (head == NULL)
+                    return NULL;
+                }
 	    else {
 		ptr->next = property_alloc(hold_n, hold_v);
-		ptr = ptr->next;
+		if (ptr->next == NULL) {
+		    properties_free(head);
+		    return NULL;
+		}
+	    ptr = ptr->next;
 	    }
 	    state = LOOK;
 	    v = n = 0;
+	    break;
+
+        case COMMENT:
+	    if (ch == '\n')
+		state = LOOK;
 	    break;
 
 	case STOP:

--------------040306070003060106090801--


To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-bugs" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3DB55EB9.5000900>