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> --- 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 fc8a8a40..b1c2270e 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[], int len) { - int rc = SEPOL_ERR; - int num_extras = 0; struct cil_tree_node *c = parse_current; int 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