On Wed, Sep 9, 2020 at 4:09 PM Stephen Smalley
<stephen.smalley.work@xxxxxxxxx> wrote:
>
> 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 | }
> | ^
Thanks, V2 coming up.
Jim
