misc/84797: State engine in the libutils properties_read is not
safe over incremental reads
Spencer Minear
minear at securecomputing.com
Thu Aug 11 12:40:31 GMT 2005
>Number: 84797
>Category: misc
>Synopsis: State engine in the libutils properties_read is not safe over incremental reads
>Confidential: no
>Severity: non-critical
>Priority: low
>Responsible: freebsd-bugs
>State: open
>Quarter:
>Keywords:
>Date-Required:
>Class: sw-bug
>Submitter-Id: current-users
>Arrival-Date: Thu Aug 11 12:40:20 GMT 2005
>Closed-Date:
>Last-Modified:
>Originator: Spencer Minear
>Release: 5.4
>Organization:
Securecomputing Corp.
>Environment:
FreeBSD nowind2.scur.com 5.4-RELEASE FreeBSD 5.4-RELEASE #0: Fri Jun 24 07:43:40 CDT 2005 mkarels at nowind2.scur.com:/usr/src/sys/i386/compile/SMP i386
>Description:
If the data fead to the properties_read function in libustil is coming over a network, and results in a partial read, the parsing state engine will loose information from the last partial line when a subsequent read is done.
>How-To-Repeat:
Build build a program that uses the property funtions to parse a property file like a FTP dist.inf file and feed the information in "chunks" with a shell script like the following, where base.inf.1 and base.inf.2 are a base.inf file that gets cut in the middle of a line.
#!/bin/sh
( cat base.inf.1; sleep 2; cat base.inf.2 ) | ./proptest
>Fix:
The following is the context diff of the changes I made that appear to work. The approach is to separate the buffer read state from the parsing state, and not change the parse state just becuase the next block of data is read.
Index: property.c
===================================================================
RCS file: /projects/fusion/cvs/src/freebsd/lib/libutil/property.c,v
retrieving revision 1.2
diff -c -r1.2 property.c
*** property.c 2005/01/04 18:14:22 1.2
--- property.c 2005/08/11 11:40:56
***************
*** 75,218 ****
char hold_v[PROPERTY_MAX_VALUE + 1];
char buf[BUFSIZ * 4];
int bp, n, v, max;
! enum { LOOK, COMMENT, NAME, VALUE, MVALUE, COMMIT, FILL, STOP } state;
int ch = 0, blevel = 0;
n = v = bp = max = 0;
head = ptr = NULL;
! state = LOOK;
! while (state != STOP) {
! if (state != COMMIT) {
if (bp == max)
! state = FILL;
else
ch = buf[bp++];
}
! switch(state) {
case FILL:
if ((max = read(fd, buf, sizeof buf)) < 0) {
properties_free(head);
return (NULL);
}
if (max == 0) {
! state = STOP;
break;
} else {
! state = LOOK;
ch = buf[0];
bp = 1;
}
! /* FALLTHROUGH deliberately since we already have a character and st
ate == LOOK */
! case LOOK:
! if (isspace((unsigned char)ch))
! continue;
! /* Allow shell or lisp style comments */
! else if (ch == '#' || ch == ';') {
! state = COMMENT;
! continue;
! }
! else if (isalnum((unsigned char)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;
!
! case NAME:
! if (ch == '\n' || !ch) {
! hold_n[n] = '\0';
! hold_v[0] = '\0';
! v = n = 0;
! state = COMMIT;
! }
! else if (isspace((unsigned char)ch))
! continue;
! else if (ch == '=') {
! hold_n[n] = '\0';
! v = n = 0;
! state = VALUE;
! }
! else
! hold_n[n++] = ch;
! break;
! case VALUE:
! if (v == 0 && ch == '\n') {
! hold_v[v] = '\0';
! v = n = 0;
! state = COMMIT;
! }
! else if (v == 0 && isspace((unsigned char)ch))
! continue;
! else if (ch == '{') {
! state = MVALUE;
! ++blevel;
! }
! else if (ch == '\n' || !ch) {
! hold_v[v] = '\0';
! v = n = 0;
! state = COMMIT;
! }
! else {
if (v >= PROPERTY_MAX_VALUE) {
! state = COMMENT;
v = n = 0;
! break;
}
! 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;
! }
! else if (ch == '}' && !--blevel) {
! hold_v[v] = '\0';
v = n = 0;
! state = COMMIT;
! }
! else {
! hold_v[v++] = ch;
! if (ch == '{')
! ++blevel;
! }
! break;
- case COMMIT:
- if (head == NULL) {
- if ((head = ptr = property_alloc(hold_n, hold_v)) == NULL)
- return (NULL);
- } else {
- if ((ptr->next = property_alloc(hold_n, hold_v)) == NULL) {
- properties_free(head);
- return (NULL);
- }
- ptr = ptr->next;
}
- state = LOOK;
- v = n = 0;
- break;
-
case STOP:
! /* we don't handle this here, but this prevents warnings */
break;
}
}
--- 75,225 ----
char hold_v[PROPERTY_MAX_VALUE + 1];
char buf[BUFSIZ * 4];
int bp, n, v, max;
! enum { LOOK, COMMENT, NAME, VALUE, MVALUE, COMMIT } parse_state;
! enum { FILL, DATA, STOP } buffer_state;
int ch = 0, blevel = 0;
n = v = bp = max = 0;
head = ptr = NULL;
! buffer_state = FILL;
! parse_state = LOOK;
!
! while (buffer_state != STOP) {
!
! if (parse_state != COMMIT) {
if (bp == max)
! buffer_state = FILL;
else
ch = buf[bp++];
}
!
! switch(buffer_state) {
case FILL:
if ((max = read(fd, buf, sizeof buf)) < 0) {
properties_free(head);
return (NULL);
}
if (max == 0) {
! buffer_state = STOP;
break;
} else {
! buffer_state = DATA;
ch = buf[0];
bp = 1;
}
! /* Fall through since we now have data to process */
! case DATA:
! switch(parse_state) {
! case LOOK:
! if (isspace((unsigned char)ch))
! continue;
! /* Allow shell or lisp style comments */
! else if (ch == '#' || ch == ';') {
! parse_state = COMMENT;
! continue;
! }
! else if (isalnum((unsigned char)ch) || ch == '_') {
! if (n >= PROPERTY_MAX_NAME) {
! n = 0;
! parse_state = COMMENT;
! }
! else {
! hold_n[n++] = ch;
! parse_state = NAME;
! }
! }
! else
! parse_state = COMMENT; /* Ignore the rest of the line */
! break;
! case COMMENT:
! if (ch == '\n')
! parse_state = LOOK;
! break;
!
! case NAME:
! if (ch == '\n' || !ch) {
! hold_n[n] = '\0';
! hold_v[0] = '\0';
! v = n = 0;
! parse_state = COMMIT;
}
! else if (isspace((unsigned char)ch))
! continue;
! else if (ch == '=') {
! hold_n[n] = '\0';
! v = n = 0;
! parse_state = VALUE;
}
! else
! hold_n[n++] = ch;
! break;
! case VALUE:
! if (v == 0 && ch == '\n') {
! hold_v[v] = '\0';
! v = n = 0;
! parse_state = COMMIT;
! }
! else if (v == 0 && isspace((unsigned char)ch))
! continue;
! else if (ch == '{') {
! parse_state = MVALUE;
! ++blevel;
! }
! else if (ch == '\n' || !ch) {
! hold_v[v] = '\0';
! v = n = 0;
! parse_state = COMMIT;
! }
! else {
! if (v >= PROPERTY_MAX_VALUE) {
! parse_state = COMMENT;
! v = n = 0;
! break;
! }
! else
! hold_v[v++] = ch;
! }
! break;
! case MVALUE:
! /* multiline value */
if (v >= PROPERTY_MAX_VALUE) {
! warn("properties_read: value exceeds max length");
! parse_state = COMMENT;
! n = v = 0;
! }
! else if (ch == '}' && !--blevel) {
! hold_v[v] = '\0';
v = n = 0;
! parse_state = COMMIT;
}
! else {
hold_v[v++] = ch;
! if (ch == '{')
! ++blevel;
! }
! break;
! case COMMIT:
! if (head == NULL) {
! if ((head = ptr = property_alloc(hold_n, hold_v)) == NULL)
! return (NULL);
! } else {
! if ((ptr->next = property_alloc(hold_n, hold_v)) == NULL) {
! properties_free(head);
! return (NULL);
! }
! ptr = ptr->next;
! }
! parse_state = LOOK;
v = n = 0;
! break;
}
case STOP:
! /* we don't handle these here, but this prevents warnings */
break;
}
}
>Release-Note:
>Audit-Trail:
>Unformatted:
More information about the freebsd-bugs
mailing list