On Wed, Sep 9, 2020 at 3:43 PM James Carter <jwcart2@xxxxxxxxx> wrote:
>
> CIL was not correctly determining the depth of conditional expressions
> which prevented it from giving an error when the max depth was exceeded.
> This allowed invalid policy binaries to be created.
>
> Validate the conditional expression using the same logic that is used
> when evaluating a conditional expression. This includes checking the
> depth of the expression.
>
> Signed-off-by: James Carter <jwcart2@xxxxxxxxx>
> ---
> libsepol/cil/src/cil_binary.c | 43 ++++++++++++++++++++++++++++++++
> libsepol/cil/src/cil_build_ast.c | 26 ++++++-------------
> 2 files changed, 50 insertions(+), 19 deletions(-)
>
> diff --git a/libsepol/cil/src/cil_binary.c b/libsepol/cil/src/cil_binary.c
> index 77266858..d30233c4 100644
> --- a/libsepol/cil/src/cil_binary.c
> +++ b/libsepol/cil/src/cil_binary.c
> @@ -2176,6 +2176,44 @@ static int __cil_cond_expr_to_sepol_expr(policydb_t *pdb, struct cil_list *cil_e
> return SEPOL_OK;
> }
>
> +int __cil_validate_cond_expr(cond_expr_t *cond_expr)
> +{
> + cond_expr_t *e;
> + int depth = -1;
> +
> + for (e = cond_expr; e != NULL; e = e->next) {
> + switch (e->expr_type) {
> + case COND_BOOL:
> + if (depth == (COND_EXPR_MAXDEPTH - 1)) {
> + cil_log(CIL_ERR,"Conditional expression exceeded max allowable depth\n");
> + return -1;
> + }
> + depth++;
> + break;
> + case COND_NOT:
> + if (depth < 0) {
> + cil_log(CIL_ERR,"Invalid conditional expression\n");
> + return -1;
> + }
> + break;
> + case COND_OR:
> + case COND_AND:
> + case COND_XOR:
> + case COND_EQ:
> + case COND_NEQ:
> + if (depth < 1) {
> + cil_log(CIL_ERR,"Invalid conditional expression\n");
> + return -1;
> + }
> + depth--;
> + break;
> + default:
> + cil_log(CIL_ERR,"Invalid conditional expression\n");
> + return -1;
> + }
> + }
Missing a return here.
../cil/src/cil_binary.c: In function ‘__cil_validate_cond_expr’:
../cil/src/cil_binary.c:2215:1: error: control reaches end of non-void
function [-Werror=return-type]
2215 | }
| ^
