[PATCH 3/3 v2] libsepol/cil: Fix syntax checking in __cil_verify_syntax()

[James Carter <jwcart2@xxxxxxxxx>]

The function __cil_verify_syntax() is used to check the syntax of
CIL rules (and a few other common things like contexts and class
permissions). It does not correctly check the syntax combination
"CIL_SYN_STRING | CIL_SYN_N_LISTS, CIL_SYN_N_LISTS | CIL_SYN_END".
This should mean either a string followed by any number of lists
or any number of lists followed by the end of the rule. Instead,
while allowing the correct syntax, it allows any number of lists
followed by a string followed by any number of more lists followed
by the end of the rule and, also, any number of lists followed by a
string followed by the end of the rule.

Refactor the function to make it clearer to follow and so that once
checking begins for CIL_SYN_N_LISTS or CIL_SYN_N_STRINGS, then only
strings or lists are allowed until the end of the rule is found. In
addition, always check for CIL_SYN_END at the end.

Signed-off-by: James Carter <jwcart2@xxxxxxxxx>
---
v2: Put parenthesis around bitwise operations.
    Using size_t for len and i as changed by patch 2.

 libsepol/cil/src/cil_verify.c | 71 ++++++++++++-----------------------
 1 file changed, 23 insertions(+), 48 deletions(-)

diff --git a/libsepol/cil/src/cil_verify.c b/libsepol/cil/src/cil_verify.c
index 6438ceba..5502c4d5 100644
--- a/libsepol/cil/src/cil_verify.c
+++ b/libsepol/cil/src/cil_verify.c
@@ -146,68 +146,43 @@ exit:
 
 int __cil_verify_syntax(struct cil_tree_node *parse_current, enum cil_syntax s[], size_t len)
 {
-	int rc = SEPOL_ERR;
-	int num_extras = 0;
 	struct cil_tree_node *c = parse_current;
 	size_t i = 0;
-	while (i < len) {
-		if ((s[i] & CIL_SYN_END) && c == NULL) {
-			break;
-		}
 
-		if (s[i] & CIL_SYN_N_LISTS || s[i] & CIL_SYN_N_STRINGS) {
-			if (c == NULL) {
-				if (num_extras > 0) {
-					i++;
-					continue;
+	while (i < len && c != NULL) {
+		if ((s[i] & CIL_SYN_STRING) && c->data != NULL && c->cl_head == NULL) {
+			c = c->next;
+			i++;
+		} else if ((s[i] & CIL_SYN_LIST) && c->data == NULL && c->cl_head != NULL) {
+			c = c->next;
+			i++;
+		} else if ((s[i] & CIL_SYN_EMPTY_LIST) && c->data == NULL && c->cl_head == NULL) {
+			c = c->next;
+			i++;
+		} else if ((s[i] & CIL_SYN_N_LISTS) || (s[i] & CIL_SYN_N_STRINGS)) {
+			while (c != NULL) {
+				if ((s[i] & CIL_SYN_N_LISTS) && c->data == NULL && c->cl_head != NULL) {
+					c = c->next;
+				} else if ((s[i] & CIL_SYN_N_STRINGS) && c->data != NULL && c->cl_head == NULL) {
+					c = c->next;
 				} else {
 					goto exit;
 				}
-			} else if ((s[i] & CIL_SYN_N_LISTS) && (c->data == NULL && c->cl_head != NULL)) {
-				c = c->next;
-				num_extras++;
-				continue;
-			} else if ((s[i] & CIL_SYN_N_STRINGS) && (c->data != NULL && c->cl_head == NULL)) {
-				c = c->next;
-				num_extras++;
-				continue;
 			}
-		}
-
-		if (c == NULL) {
+			i++;
+			break; /* Only CIL_SYN_END allowed after these */
+		} else {
 			goto exit;
 		}
+	}
 
-		if (s[i] & CIL_SYN_STRING) {
-			if (c->data != NULL && c->cl_head == NULL) {
-				c = c->next;
-				i++;
-				continue;
-			}
-		}
-
-		if (s[i] & CIL_SYN_LIST) {
-			if (c->data == NULL && c->cl_head != NULL) {
-				c = c->next;
-				i++;
-				continue;
-			}
-		}
-
-		if (s[i] & CIL_SYN_EMPTY_LIST) {
-			if (c->data == NULL && c->cl_head == NULL) {
-				c = c->next;
-				i++;
-				continue;
-			}
-		}
-		goto exit;
+	if (i < len && (s[i] & CIL_SYN_END) && c == NULL) {
+		return SEPOL_OK;
 	}
-	return SEPOL_OK;
 
 exit:
 	cil_log(CIL_ERR, "Invalid syntax\n");
-	return rc;
+	return SEPOL_ERR;
 }
 
 int cil_verify_expr_syntax(struct cil_tree_node *current, enum cil_flavor op, enum cil_flavor expr_flavor)
-- 
2.31.1