From 8aed8807f7c1ac974c016f8b424baffaef50fddb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Mon, 22 Jan 2024 14:23:49 +0100 Subject: [PATCH 01/15] checkpolicy: add libfuzz based fuzzer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce a libfuzz[1] based fuzzer testing the parsing and policy generation code used within checkpolicy(8) and checkmodule(8), similar to the fuzzer for secilc(8). The fuzzer will work on generated source policy input and try to parse, link, expand, optimize, sort and output it. This fuzzer will also ensure policy validation is not too strict by checking compilable source policies are valid. Build the fuzzer in the oss-fuzz script. [1]: https://llvm.org/docs/LibFuzzer.html Signed-off-by: Christian Göttsche --- checkpolicy/Makefile | 3 + checkpolicy/fuzz/checkpolicy-fuzzer.c | 274 +++++++++++++++++++++++ checkpolicy/fuzz/checkpolicy-fuzzer.dict | 101 +++++++++ checkpolicy/fuzz/min_pol.conf | 60 +++++ checkpolicy/fuzz/min_pol.mls.conf | 65 ++++++ checkpolicy/module_compiler.c | 11 + checkpolicy/module_compiler.h | 4 + checkpolicy/policy_scan.l | 8 + scripts/oss-fuzz.sh | 14 ++ 9 files changed, 540 insertions(+) create mode 100644 checkpolicy/fuzz/checkpolicy-fuzzer.c create mode 100644 checkpolicy/fuzz/checkpolicy-fuzzer.dict create mode 100644 checkpolicy/fuzz/min_pol.conf create mode 100644 checkpolicy/fuzz/min_pol.mls.conf diff --git a/checkpolicy/Makefile b/checkpolicy/Makefile index 036ab90523..6e8008e328 100644 --- a/checkpolicy/Makefile +++ b/checkpolicy/Makefile @@ -54,6 +54,9 @@ lex.yy.c: policy_scan.l y.tab.c test: checkpolicy ./tests/test_roundtrip.sh +# helper target for fuzzing +checkobjects: $(CHECKOBJS) + install: all -mkdir -p $(DESTDIR)$(BINDIR) -mkdir -p $(DESTDIR)$(MANDIR)/man8 diff --git a/checkpolicy/fuzz/checkpolicy-fuzzer.c b/checkpolicy/fuzz/checkpolicy-fuzzer.c new file mode 100644 index 0000000000..0d749a0299 --- /dev/null +++ b/checkpolicy/fuzz/checkpolicy-fuzzer.c @@ -0,0 +1,274 @@ +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include + +#include "module_compiler.h" +#include "queue.h" + +extern int policydb_validate(sepol_handle_t *handle, const policydb_t *p); +extern int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size); + +extern int mlspol; +extern policydb_t *policydbp; +extern queue_t id_queue; +extern unsigned int policydb_errors; + +extern int yynerrs; +extern FILE *yyin; +extern void init_parser(int); +extern int yyparse(void); +extern void yyrestart(FILE *); +extern int yylex_destroy(void); +extern void set_source_file(const char *name); + + +// Set to 1 for verbose libsepol logging +#define VERBOSE 0 + +static ssize_t full_write(int fd, const void *buf, size_t count) +{ + ssize_t written = 0; + + while (count > 0) { + ssize_t ret = write(fd, buf, count); + if (ret < 0) { + if (errno == EINTR) + continue; + + return ret; + } + + if (ret == 0) + break; + + written += ret; + buf = (const unsigned char *)buf + (size_t)ret; + count -= (size_t)ret; + } + + return written; +} + +static int read_source_policy(policydb_t *p, const uint8_t *data, size_t size) +{ + int fd, rc; + ssize_t wr; + + fd = memfd_create("fuzz-input", MFD_CLOEXEC); + if (fd < 0) + return -1; + + wr = full_write(fd, data, size); + if (wr < 0 || (size_t)wr != size) { + close(fd); + return -1; + } + + fsync(fd); + + yynerrs = 0; + + yyin = fdopen(fd, "r"); + if (!yyin) { + close(fd); + return -1; + } + + rewind(yyin); + + set_source_file("fuzz-input"); + + id_queue = queue_create(); + if (id_queue == NULL) { + fclose(yyin); + yylex_destroy(); + return -1; + } + + policydbp = p; + mlspol = p->mls; + + init_parser(1); + + rc = yyparse(); + // TODO: drop global variable policydb_errors if proven to be redundant + assert(rc || !policydb_errors); + if (rc || policydb_errors) { + queue_destroy(id_queue); + fclose(yyin); + yylex_destroy(); + return -1; + } + + rewind(yyin); + init_parser(2); + set_source_file("fuzz-input"); + yyrestart(yyin); + + rc = yyparse(); + assert(rc || !policydb_errors); + if (rc || policydb_errors) { + queue_destroy(id_queue); + fclose(yyin); + yylex_destroy(); + return -1; + } + + queue_destroy(id_queue); + fclose(yyin); + yylex_destroy(); + + return 0; +} + +static int check_level(hashtab_key_t key, hashtab_datum_t datum, void *arg __attribute__ ((unused))) +{ + const level_datum_t *levdatum = (level_datum_t *) datum; + + // TODO: drop member defined if proven to be always set + if (!levdatum->isalias && !levdatum->defined) { + fprintf(stderr, + "Error: sensitivity %s was not used in a level definition!\n", + key); + abort(); + } + + return 0; +} + +static int write_binary_policy(FILE *outfp, policydb_t *p) +{ + struct policy_file pf; + + policy_file_init(&pf); + pf.type = PF_USE_STDIO; + pf.fp = outfp; + return policydb_write(p, &pf); +} + +int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) +{ + policydb_t parsepolicydb = {}; + policydb_t kernpolicydb = {}; + policydb_t *finalpolicydb; + sidtab_t sidtab = {}; + FILE *devnull = NULL; + int mls, policyvers; + + sepol_debug(VERBOSE); + + /* Take the first byte whether to parse as MLS policy + * and the second byte as policy version. */ + if (size < 2) + return 0; + switch (data[0]) { + case '0': + mls = 0; + break; + case '1': + mls = 1; + break; + default: + return 0; + } + static_assert(0x7F - 'A' >= POLICYDB_VERSION_MAX, "Max policy version should be representable"); + policyvers = data[1] - 'A'; + if (policyvers < POLICYDB_VERSION_MIN || policyvers > POLICYDB_VERSION_MAX) + return 0; + data += 2; + size -= 2; + + if (policydb_init(&parsepolicydb)) + goto exit; + + parsepolicydb.policy_type = POLICY_BASE; + parsepolicydb.mls = mls; + parsepolicydb.handle_unknown = DENY_UNKNOWN; + policydb_set_target_platform(&parsepolicydb, SEPOL_TARGET_SELINUX); + + if (read_source_policy(&parsepolicydb, data, size)) + goto exit; + + (void) hashtab_map(parsepolicydb.p_levels.table, check_level, NULL); + + if (parsepolicydb.policy_type == POLICY_BASE) { + if (link_modules(NULL, &parsepolicydb, NULL, 0, VERBOSE)) + goto exit; + + if (policydb_init(&kernpolicydb)) + goto exit; + + if (expand_module(NULL, &parsepolicydb, &kernpolicydb, VERBOSE, /*check_assertions=*/0)) + goto exit; + + (void) check_assertions(NULL, &kernpolicydb, kernpolicydb.global->branch_list->avrules); + (void) hierarchy_check_constraints(NULL, &kernpolicydb); + + kernpolicydb.policyvers = policyvers; + + assert(kernpolicydb.policy_type == POLICY_KERN); + assert(kernpolicydb.handle_unknown == SEPOL_DENY_UNKNOWN); + assert(kernpolicydb.mls == mls); + + finalpolicydb = &kernpolicydb; + } else { + assert(parsepolicydb.policy_type == POLICY_MOD); + assert(parsepolicydb.handle_unknown == SEPOL_DENY_UNKNOWN); + assert(parsepolicydb.mls == mls); + + finalpolicydb = &parsepolicydb; + } + + if (policydb_load_isids(finalpolicydb, &sidtab)) + goto exit; + + if (finalpolicydb->policy_type == POLICY_KERN && policydb_optimize(finalpolicydb)) + goto exit; + + if (policydb_sort_ocontexts(finalpolicydb)) + goto exit; + + if (policydb_validate(NULL, finalpolicydb)) + /* never generate an invalid policy */ + abort(); + + devnull = fopen("/dev/null", "we"); + if (devnull == NULL) + goto exit; + + if (write_binary_policy(devnull, finalpolicydb)) + abort(); + + if (finalpolicydb->policy_type == POLICY_KERN && sepol_kernel_policydb_to_conf(devnull, finalpolicydb)) + abort(); + + if (finalpolicydb->policy_type == POLICY_KERN && sepol_kernel_policydb_to_cil(devnull, finalpolicydb)) + abort(); + + if (finalpolicydb->policy_type == POLICY_MOD && sepol_module_policydb_to_cil(devnull, finalpolicydb, /*linked=*/0)) + abort(); + +exit: + if (devnull != NULL) + fclose(devnull); + + sepol_sidtab_destroy(&sidtab); + policydb_destroy(&kernpolicydb); + policydb_destroy(&parsepolicydb); + + id_queue = NULL; + policydbp = NULL; + module_compiler_reset(); + + /* Non-zero return values are reserved for future use. */ + return 0; +} diff --git a/checkpolicy/fuzz/checkpolicy-fuzzer.dict b/checkpolicy/fuzz/checkpolicy-fuzzer.dict new file mode 100644 index 0000000000..fb7e66c062 --- /dev/null +++ b/checkpolicy/fuzz/checkpolicy-fuzzer.dict @@ -0,0 +1,101 @@ +# Keyword dictionary + +"clone" +"common" +"class" +"constrain" +"validatetrans" +"inherits" +"sid" +"role" +"roles" +"roleattribute" +"attribute_role" +"types" +"typealias" +"typeattribute" +"typebounds" +"type" +"bool" +"tunable" +"if" +"else" +"alias" +"attribute" +"expandattribute" +"type_transition" +"type_member" +"type_change" +"role_transition" +"range_transition" +"sensitivity" +"dominance" +"category" +"level" +"range" +"mlsconstrain" +"mlsvalidatetrans" +"user" +"neverallow" +"allow" +"auditallow" +"auditdeny" +"dontaudit" +"allowxperm" +"auditallowxperm" +"dontauditxperm" +"neverallowxperm" +"source" +"target" +"sameuser" +"module" +"require" +"optional" +"or" +"and" +"not" +"xor" +"eq" +"true" +"false" +"dom" +"domby" +"incomp" +"fscon" +"ibpkeycon" +"ibendportcon" +"portcon" +"netifcon" +"nodecon" +"pirqcon" +"iomemcon" +"ioportcon" +"pcidevicecon" +"devicetreecon" +"fs_use_xattr" +"fs_use_task" +"fs_use_trans" +"genfscon" +"r1" +"r2" +"r3" +"u1" +"u2" +"u3" +"t1" +"t2" +"t3" +"l1" +"l2" +"h1" +"h2" +"policycap" +"permissive" +"default_user" +"default_role" +"default_type" +"default_range" +"low-high" +"high" +"low" +"glblub" diff --git a/checkpolicy/fuzz/min_pol.conf b/checkpolicy/fuzz/min_pol.conf new file mode 100644 index 0000000000..ff6d50e5ec --- /dev/null +++ b/checkpolicy/fuzz/min_pol.conf @@ -0,0 +1,60 @@ +class process +class blk_file +class chr_file +class dir +class fifo_file +class file +class lnk_file +class sock_file +sid kernel +sid security +sid unlabeled +sid fs +sid file +sid file_labels +sid init +sid any_socket +sid port +sid netif +sid netmsg +sid node +sid igmp_packet +sid icmp_socket +sid tcp_socket +sid sysctl_modprobe +sid sysctl +sid sysctl_fs +sid sysctl_kernel +sid sysctl_net +sid sysctl_net_unix +sid sysctl_vm +sid sysctl_dev +sid kmod +sid policy +sid scmp_packet +sid devnull +class process { dyntransition transition } +default_role { blk_file } source; +default_role { chr_file } source; +default_role { dir } source; +default_role { fifo_file } source; +default_role { file } source; +default_role { lnk_file } source; +default_role { sock_file } source; +type sys_isid; +typealias sys_isid alias { dpkg_script_t rpm_script_t }; +allow sys_isid self : process { dyntransition transition }; +role sys_role; +role sys_role types { sys_isid }; +user sys_user roles sys_role; +sid kernel sys_user:sys_role:sys_isid +sid security sys_user:sys_role:sys_isid +sid unlabeled sys_user:sys_role:sys_isid +sid file sys_user:sys_role:sys_isid +sid port sys_user:sys_role:sys_isid +sid netif sys_user:sys_role:sys_isid +sid netmsg sys_user:sys_role:sys_isid +sid node sys_user:sys_role:sys_isid +sid devnull sys_user:sys_role:sys_isid +fs_use_trans devpts sys_user:sys_role:sys_isid; +fs_use_trans devtmpfs sys_user:sys_role:sys_isid; diff --git a/checkpolicy/fuzz/min_pol.mls.conf b/checkpolicy/fuzz/min_pol.mls.conf new file mode 100644 index 0000000000..6d15846bd4 --- /dev/null +++ b/checkpolicy/fuzz/min_pol.mls.conf @@ -0,0 +1,65 @@ +class process +class blk_file +class chr_file +class dir +class fifo_file +class file +class lnk_file +class sock_file +sid kernel +sid security +sid unlabeled +sid fs +sid file +sid file_labels +sid init +sid any_socket +sid port +sid netif +sid netmsg +sid node +sid igmp_packet +sid icmp_socket +sid tcp_socket +sid sysctl_modprobe +sid sysctl +sid sysctl_fs +sid sysctl_kernel +sid sysctl_net +sid sysctl_net_unix +sid sysctl_vm +sid sysctl_dev +sid kmod +sid policy +sid scmp_packet +sid devnull +class process { dyntransition transition } +default_role { blk_file } source; +default_role { chr_file } source; +default_role { dir } source; +default_role { fifo_file } source; +default_role { file } source; +default_role { lnk_file } source; +default_role { sock_file } source; +sensitivity s0; +dominance { s0 } +category c0; +level s0:c0; +mlsconstrain process transition t1 eq t2; +type sys_isid; +typealias sys_isid alias { dpkg_script_t rpm_script_t }; +allow sys_isid self : process { dyntransition transition }; +role sys_role; +role sys_role types { sys_isid }; +user sys_user roles sys_role level s0 range s0 - s0:c0; +sid kernel sys_user:sys_role:sys_isid:s0 +sid security sys_user:sys_role:sys_isid:s0 +sid unlabeled sys_user:sys_role:sys_isid:s0 +sid file sys_user:sys_role:sys_isid:s0 +sid port sys_user:sys_role:sys_isid:s0 +sid netif sys_user:sys_role:sys_isid:s0 +sid netmsg sys_user:sys_role:sys_isid:s0 +sid node sys_user:sys_role:sys_isid:s0 +sid devnull sys_user:sys_role:sys_isid:s0 +fs_use_trans devpts sys_user:sys_role:sys_isid:s0; +fs_use_trans devtmpfs sys_user:sys_role:sys_isid:s0; diff --git a/checkpolicy/module_compiler.c b/checkpolicy/module_compiler.c index 3188af892a..74a9f93c09 100644 --- a/checkpolicy/module_compiler.c +++ b/checkpolicy/module_compiler.c @@ -1492,3 +1492,14 @@ static void pop_stack(void) free(stack_top); stack_top = parent; } + +#ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION +void module_compiler_reset(void) +{ + while (stack_top) + pop_stack(); + + last_block = NULL; + next_decl_id = 1; +} +#endif diff --git a/checkpolicy/module_compiler.h b/checkpolicy/module_compiler.h index 29b824b425..e43bc6c0d3 100644 --- a/checkpolicy/module_compiler.h +++ b/checkpolicy/module_compiler.h @@ -106,4 +106,8 @@ int begin_optional_else(int pass); * return -1. */ int end_avrule_block(int pass); +#ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION +void module_compiler_reset(void); +#endif + #endif diff --git a/checkpolicy/policy_scan.l b/checkpolicy/policy_scan.l index c998ff8b8a..19c05a5888 100644 --- a/checkpolicy/policy_scan.l +++ b/checkpolicy/policy_scan.l @@ -312,6 +312,7 @@ GLBLUB { return(GLBLUB); } %% int yyerror(const char *msg) { +#ifndef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION if (source_file[0]) fprintf(stderr, "%s:%lu:", source_file, source_lineno); @@ -322,6 +323,10 @@ int yyerror(const char *msg) yytext, policydb_lineno, linebuf[0], linebuf[1]); +#else + (void)msg; +#endif + policydb_errors++; return -1; } @@ -331,6 +336,7 @@ int yywarn(const char *msg) if (werror) return yyerror(msg); +#ifndef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION if (source_file[0]) fprintf(stderr, "%s:%lu:", source_file, source_lineno); @@ -341,6 +347,8 @@ int yywarn(const char *msg) yytext, policydb_lineno, linebuf[0], linebuf[1]); +#endif + return 0; } diff --git a/scripts/oss-fuzz.sh b/scripts/oss-fuzz.sh index 72d275e8d8..069f130aed 100755 --- a/scripts/oss-fuzz.sh +++ b/scripts/oss-fuzz.sh @@ -70,3 +70,17 @@ $CC $CFLAGS -c -o binpolicy-fuzzer.o libsepol/fuzz/binpolicy-fuzzer.c $CXX $CXXFLAGS $LIB_FUZZING_ENGINE binpolicy-fuzzer.o "$DESTDIR/usr/lib/libsepol.a" -o "$OUT/binpolicy-fuzzer" zip -j "$OUT/binpolicy-fuzzer_seed_corpus.zip" libsepol/fuzz/policy.bin + +## checkpolicy fuzzer ## + +make -C checkpolicy clean +make -C checkpolicy V=1 -j"$(nproc)" checkobjects +# CFLAGS, CXXFLAGS and LIB_FUZZING_ENGINE have to be split to be accepted by +# the compiler/linker so they shouldn't be quoted +# shellcheck disable=SC2086 +$CC $CFLAGS -Icheckpolicy/ -c -o checkpolicy-fuzzer.o checkpolicy/fuzz/checkpolicy-fuzzer.c +# shellcheck disable=SC2086 +$CXX $CXXFLAGS $LIB_FUZZING_ENGINE checkpolicy-fuzzer.o checkpolicy/*.o "$DESTDIR/usr/lib/libsepol.a" -o "$OUT/checkpolicy-fuzzer" + +zip -j "$OUT/checkpolicy-fuzzer_seed_corpus.zip" checkpolicy/fuzz/min_pol.mls.conf +cp checkpolicy/fuzz/checkpolicy-fuzzer.dict "$OUT/" From e7ba55fbd3f29d94ed4a1be55b50581f503bdbca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Mon, 22 Jan 2024 14:24:09 +0100 Subject: [PATCH 02/15] checkpolicy: cleanup resources on parse error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Close the input file and free all memory by the queue and lexer on a syntax or parse error. Signed-off-by: Christian Göttsche --- checkpolicy/parse_util.c | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/checkpolicy/parse_util.c b/checkpolicy/parse_util.c index f2d1e04d7e..eda814e15f 100644 --- a/checkpolicy/parse_util.c +++ b/checkpolicy/parse_util.c @@ -26,6 +26,7 @@ extern FILE *yyin; extern void init_parser(int); extern int yyparse(void); extern void yyrestart(FILE *); +extern int yylex_destroy(void); extern queue_t id_queue; extern unsigned int policydb_errors; extern policydb_t *policydbp; @@ -34,6 +35,8 @@ extern void set_source_file(const char *name); int read_source_policy(policydb_t * p, const char *file, const char *progname) { + int rc = -1; + yyin = fopen(file, "r"); if (!yyin) { fprintf(stderr, "%s: unable to open %s: %s\n", progname, file, strerror(errno)); @@ -41,21 +44,26 @@ int read_source_policy(policydb_t * p, const char *file, const char *progname) } set_source_file(file); - if ((id_queue = queue_create()) == NULL) { + id_queue = queue_create(); + if (id_queue == NULL) { fprintf(stderr, "%s: out of memory!\n", progname); - return -1; + goto cleanup; } + mlspol = p->mls; policydbp = p; policydbp->name = strdup(file); - mlspol = p->mls; + if (!policydbp->name) { + fprintf(stderr, "%s: out of memory!\n", progname); + goto cleanup; + } init_parser(1); if (yyparse() || policydb_errors) { fprintf(stderr, "%s: error(s) encountered while parsing configuration\n", progname); - return -1; + goto cleanup; } rewind(yyin); init_parser(2); @@ -65,11 +73,15 @@ int read_source_policy(policydb_t * p, const char *file, const char *progname) fprintf(stderr, "%s: error(s) encountered while parsing configuration\n", progname); - return -1; + goto cleanup; } - queue_destroy(id_queue); + rc = 0; + +cleanup: + queue_destroy(id_queue); fclose(yyin); + yylex_destroy(); - return 0; + return rc; } From 748bbafcc039d9e01f6f187cb91cae911e300335 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Mon, 22 Jan 2024 14:24:11 +0100 Subject: [PATCH 03/15] checkpolicy: cleanup identifiers on error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Free identifiers removed from the queue but not yet owned by the policy on errors. Signed-off-by: Christian Göttsche --- checkpolicy/policy_define.c | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c index 260e609d19..db7e9d0e10 100644 --- a/checkpolicy/policy_define.c +++ b/checkpolicy/policy_define.c @@ -342,6 +342,7 @@ static int read_classes(ebitmap_t *e_classes) while ((id = queue_remove(id_queue))) { if (!is_id_in_scope(SYM_CLASSES, id)) { yyerror2("class %s is not within scope", id); + free(id); return -1; } cladatum = hashtab_search(policydbp->p_classes.table, id); @@ -373,15 +374,18 @@ int define_default_user(int which) while ((id = queue_remove(id_queue))) { if (!is_id_in_scope(SYM_CLASSES, id)) { yyerror2("class %s is not within scope", id); + free(id); return -1; } cladatum = hashtab_search(policydbp->p_classes.table, id); if (!cladatum) { yyerror2("unknown class %s", id); + free(id); return -1; } if (cladatum->default_user && cladatum->default_user != which) { yyerror2("conflicting default user information for class %s", id); + free(id); return -1; } cladatum->default_user = which; @@ -405,15 +409,18 @@ int define_default_role(int which) while ((id = queue_remove(id_queue))) { if (!is_id_in_scope(SYM_CLASSES, id)) { yyerror2("class %s is not within scope", id); + free(id); return -1; } cladatum = hashtab_search(policydbp->p_classes.table, id); if (!cladatum) { yyerror2("unknown class %s", id); + free(id); return -1; } if (cladatum->default_role && cladatum->default_role != which) { yyerror2("conflicting default role information for class %s", id); + free(id); return -1; } cladatum->default_role = which; @@ -437,15 +444,18 @@ int define_default_type(int which) while ((id = queue_remove(id_queue))) { if (!is_id_in_scope(SYM_CLASSES, id)) { yyerror2("class %s is not within scope", id); + free(id); return -1; } cladatum = hashtab_search(policydbp->p_classes.table, id); if (!cladatum) { yyerror2("unknown class %s", id); + free(id); return -1; } if (cladatum->default_type && cladatum->default_type != which) { yyerror2("conflicting default type information for class %s", id); + free(id); return -1; } cladatum->default_type = which; @@ -469,15 +479,18 @@ int define_default_range(int which) while ((id = queue_remove(id_queue))) { if (!is_id_in_scope(SYM_CLASSES, id)) { yyerror2("class %s is not within scope", id); + free(id); return -1; } cladatum = hashtab_search(policydbp->p_classes.table, id); if (!cladatum) { yyerror2("unknown class %s", id); + free(id); return -1; } if (cladatum->default_range && cladatum->default_range != which) { yyerror2("conflicting default range information for class %s", id); + free(id); return -1; } cladatum->default_range = which; @@ -508,6 +521,7 @@ int define_common_perms(void) comdatum = hashtab_search(policydbp->p_commons.table, id); if (comdatum) { yyerror2("duplicate declaration for common %s\n", id); + free(id); return -1; } comdatum = (common_datum_t *) malloc(sizeof(common_datum_t)); @@ -770,12 +784,14 @@ int define_sens(void) while ((id = queue_remove(id_queue))) { if (id_has_dot(id)) { yyerror("sensitivity aliases may not contain periods"); - goto bad_alias; + free(id); + return -1; } aliasdatum = (level_datum_t *) malloc(sizeof(level_datum_t)); if (!aliasdatum) { yyerror("out of memory"); - goto bad_alias; + free(id); + return -1; } level_datum_init(aliasdatum); aliasdatum->isalias = TRUE; @@ -940,12 +956,14 @@ int define_category(void) while ((id = queue_remove(id_queue))) { if (id_has_dot(id)) { yyerror("category aliases may not contain periods"); - goto bad_alias; + free(id); + return -1; } aliasdatum = (cat_datum_t *) malloc(sizeof(cat_datum_t)); if (!aliasdatum) { yyerror("out of memory"); - goto bad_alias; + free(id); + return -1; } cat_datum_init(aliasdatum); aliasdatum->isalias = TRUE; @@ -3722,6 +3740,7 @@ uintptr_t define_cexpr(uint32_t expr_type, uintptr_t arg1, uintptr_t arg2) if (!is_id_in_scope(SYM_USERS, id)) { yyerror2("user %s is not within scope", id); + free(id); constraint_expr_destroy(expr); return 0; } @@ -3733,6 +3752,7 @@ uintptr_t define_cexpr(uint32_t expr_type, uintptr_t arg1, uintptr_t arg2) id); if (!user) { yyerror2("unknown user %s", id); + free(id); constraint_expr_destroy(expr); return 0; } @@ -3742,6 +3762,7 @@ uintptr_t define_cexpr(uint32_t expr_type, uintptr_t arg1, uintptr_t arg2) yyerror2("role %s is not within scope", id); constraint_expr_destroy(expr); + free(id); return 0; } role = @@ -3753,6 +3774,7 @@ uintptr_t define_cexpr(uint32_t expr_type, uintptr_t arg1, uintptr_t arg2) if (!role) { yyerror2("unknown role %s", id); constraint_expr_destroy(expr); + free(id); return 0; } val = role->s.value; @@ -3765,11 +3787,13 @@ uintptr_t define_cexpr(uint32_t expr_type, uintptr_t arg1, uintptr_t arg2) } else { yyerror("invalid constraint expression"); constraint_expr_destroy(expr); + free(id); return 0; } if (ebitmap_set_bit(&expr->names, val - 1, TRUE)) { yyerror("out of memory"); ebitmap_destroy(&expr->names); + free(id); constraint_expr_destroy(expr); return 0; } From 7a38093e31810bf1e3e2ddd63e816b87beff93d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Mon, 22 Jan 2024 14:24:12 +0100 Subject: [PATCH 04/15] checkpolicy: free ebitmap on error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Christian Göttsche --- checkpolicy/policy_define.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c index db7e9d0e10..053156df6a 100644 --- a/checkpolicy/policy_define.c +++ b/checkpolicy/policy_define.c @@ -2544,6 +2544,8 @@ static int define_te_avtab_helper(int which, avrule_t ** rule) int add = 1, ret = 0; int suppress = 0; + ebitmap_init(&tclasses); + avrule = (avrule_t *) malloc(sizeof(avrule_t)); if (!avrule) { yyerror("memory error"); @@ -2607,7 +2609,6 @@ static int define_te_avtab_helper(int which, avrule_t ** rule) } } - ebitmap_init(&tclasses); ret = read_classes(&tclasses); if (ret) goto out; @@ -2693,8 +2694,6 @@ static int define_te_avtab_helper(int which, avrule_t ** rule) free(id); } - ebitmap_destroy(&tclasses); - avrule->perms = perms; *rule = avrule; @@ -2703,6 +2702,9 @@ static int define_te_avtab_helper(int which, avrule_t ** rule) avrule_destroy(avrule); free(avrule); } + + ebitmap_destroy(&tclasses); + return ret; } From 8ec207898dfa68adc11447e41a0895e49592b0c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Mon, 22 Jan 2024 14:24:13 +0100 Subject: [PATCH 05/15] checkpolicy: check allocation and free memory on error at type definition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Christian Göttsche --- checkpolicy/policy_define.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c index 053156df6a..ec19da9df3 100644 --- a/checkpolicy/policy_define.c +++ b/checkpolicy/policy_define.c @@ -1399,7 +1399,7 @@ int define_typeattribute(void) return 0; } -static int define_typebounds_helper(char *bounds_id, char *type_id) +static int define_typebounds_helper(const char *bounds_id, const char *type_id) { type_datum_t *bounds, *type; @@ -1482,15 +1482,26 @@ int define_type(int alias) * old name based hierarchy. */ if ((id = queue_remove(id_queue))) { - char *bounds, *delim; + const char *delim; + + if ((delim = strrchr(id, '.'))) { + int ret; + char *bounds = strdup(id); + if (!bounds) { + yyerror("out of memory"); + free(id); + return -1; + } - if ((delim = strrchr(id, '.')) - && (bounds = strdup(id))) { bounds[(size_t)(delim - id)] = '\0'; - if (define_typebounds_helper(bounds, id)) - return -1; + ret = define_typebounds_helper(bounds, id); free(bounds); + if (ret) { + free(id); + return -1; + } + } free(id); } From cf8fcbc48b4f5d73a2e63c12e095ad9a0a48a483 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Mon, 22 Jan 2024 14:24:14 +0100 Subject: [PATCH 06/15] checkpolicy: clean expression on error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The passed expression needs to be transferred into the policy or free'd by the sink functions define_constraint() and define_validatetrans(). Signed-off-by: Christian Göttsche --- checkpolicy/policy_define.c | 68 ++++++++++++++++++++++--------------- 1 file changed, 40 insertions(+), 28 deletions(-) diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c index ec19da9df3..975826308e 100644 --- a/checkpolicy/policy_define.c +++ b/checkpolicy/policy_define.c @@ -3428,20 +3428,22 @@ int define_constraint(constraint_expr_t * expr) return 0; } + ebitmap_init(&classmap); + depth = -1; for (e = expr; e; e = e->next) { switch (e->expr_type) { case CEXPR_NOT: if (depth < 0) { yyerror("illegal constraint expression"); - return -1; + goto bad; } break; case CEXPR_AND: case CEXPR_OR: if (depth < 1) { yyerror("illegal constraint expression"); - return -1; + goto bad; } depth--; break; @@ -3449,51 +3451,48 @@ int define_constraint(constraint_expr_t * expr) case CEXPR_NAMES: if (e->attr & CEXPR_XTARGET) { yyerror("illegal constraint expression"); - return -1; /* only for validatetrans rules */ + goto bad; /* only for validatetrans rules */ } if (depth == (CEXPR_MAXDEPTH - 1)) { yyerror("constraint expression is too deep"); - return -1; + goto bad; } depth++; break; default: yyerror("illegal constraint expression"); - return -1; + goto bad; } } if (depth != 0) { yyerror("illegal constraint expression"); - return -1; + goto bad; } - ebitmap_init(&classmap); while ((id = queue_remove(id_queue))) { if (!is_id_in_scope(SYM_CLASSES, id)) { yyerror2("class %s is not within scope", id); free(id); - return -1; + goto bad; } cladatum = (class_datum_t *) hashtab_search(policydbp->p_classes.table, (hashtab_key_t) id); if (!cladatum) { yyerror2("class %s is not defined", id); - ebitmap_destroy(&classmap); free(id); - return -1; + goto bad; } if (ebitmap_set_bit(&classmap, cladatum->s.value - 1, TRUE)) { yyerror("out of memory"); - ebitmap_destroy(&classmap); free(id); - return -1; + goto bad; } node = malloc(sizeof(struct constraint_node)); if (!node) { yyerror("out of memory"); free(node); - return -1; + goto bad; } memset(node, 0, sizeof(constraint_node_t)); if (useexpr) { @@ -3505,7 +3504,7 @@ int define_constraint(constraint_expr_t * expr) if (!node->expr) { yyerror("out of memory"); free(node); - return -1; + goto bad; } node->permissions = 0; @@ -3557,8 +3556,7 @@ int define_constraint(constraint_expr_t * expr) yyerror2("permission %s is not" " defined for class %s", id, policydbp->p_class_val_to_name[i]); free(id); - ebitmap_destroy(&classmap); - return -1; + goto bad; } } node->permissions |= (UINT32_C(1) << (perdatum->s.value - 1)); @@ -3569,6 +3567,13 @@ int define_constraint(constraint_expr_t * expr) ebitmap_destroy(&classmap); return 0; + +bad: + ebitmap_destroy(&classmap); + if (useexpr) + constraint_expr_destroy(expr); + + return -1; } int define_validatetrans(constraint_expr_t * expr) @@ -3587,20 +3592,22 @@ int define_validatetrans(constraint_expr_t * expr) return 0; } + ebitmap_init(&classmap); + depth = -1; for (e = expr; e; e = e->next) { switch (e->expr_type) { case CEXPR_NOT: if (depth < 0) { yyerror("illegal validatetrans expression"); - return -1; + goto bad; } break; case CEXPR_AND: case CEXPR_OR: if (depth < 1) { yyerror("illegal validatetrans expression"); - return -1; + goto bad; } depth--; break; @@ -3608,47 +3615,45 @@ int define_validatetrans(constraint_expr_t * expr) case CEXPR_NAMES: if (depth == (CEXPR_MAXDEPTH - 1)) { yyerror("validatetrans expression is too deep"); - return -1; + goto bad; } depth++; break; default: yyerror("illegal validatetrans expression"); - return -1; + goto bad; } } if (depth != 0) { yyerror("illegal validatetrans expression"); - return -1; + goto bad; } - ebitmap_init(&classmap); while ((id = queue_remove(id_queue))) { if (!is_id_in_scope(SYM_CLASSES, id)) { yyerror2("class %s is not within scope", id); free(id); - return -1; + goto bad; } cladatum = (class_datum_t *) hashtab_search(policydbp->p_classes.table, (hashtab_key_t) id); if (!cladatum) { yyerror2("class %s is not defined", id); - ebitmap_destroy(&classmap); free(id); - return -1; + goto bad; } if (ebitmap_set_bit(&classmap, (cladatum->s.value - 1), TRUE)) { yyerror("out of memory"); - ebitmap_destroy(&classmap); free(id); - return -1; + goto bad; } node = malloc(sizeof(struct constraint_node)); if (!node) { yyerror("out of memory"); - return -1; + free(id); + goto bad; } memset(node, 0, sizeof(constraint_node_t)); if (useexpr) { @@ -3668,6 +3673,13 @@ int define_validatetrans(constraint_expr_t * expr) ebitmap_destroy(&classmap); return 0; + +bad: + ebitmap_destroy(&classmap); + if (useexpr) + constraint_expr_destroy(expr); + + return -1; } uintptr_t define_cexpr(uint32_t expr_type, uintptr_t arg1, uintptr_t arg2) From b8d7c36b92e111e1cc5c5fa4359e9ee90f3c69b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Mon, 22 Jan 2024 14:24:15 +0100 Subject: [PATCH 07/15] checkpolicy: call YYABORT on parse errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Calling the parser macro YYABORT allows the parser to cleanup up any allocated resources before returning. Signed-off-by: Christian Göttsche --- checkpolicy/policy_parse.y | 400 ++++++++++++++++++------------------- 1 file changed, 200 insertions(+), 200 deletions(-) diff --git a/checkpolicy/policy_parse.y b/checkpolicy/policy_parse.y index 356626e2dc..f5025f284d 100644 --- a/checkpolicy/policy_parse.y +++ b/checkpolicy/policy_parse.y @@ -163,26 +163,26 @@ typedef int (* require_func_t)(int pass); policy : base_policy | module_policy ; -base_policy : { if (define_policy(pass, 0) == -1) return -1; } +base_policy : { if (define_policy(pass, 0) == -1) YYABORT; } classes initial_sids access_vectors - { if (pass == 1) { if (policydb_index_classes(policydbp)) return -1; } - else if (pass == 2) { if (policydb_index_others(NULL, policydbp, 0)) return -1; }} + { if (pass == 1) { if (policydb_index_classes(policydbp)) YYABORT; } + else if (pass == 2) { if (policydb_index_others(NULL, policydbp, 0)) YYABORT; }} opt_default_rules opt_mls te_rbac users opt_constraints - { if (pass == 1) { if (policydb_index_bools(policydbp)) return -1;} - else if (pass == 2) { if (policydb_index_others(NULL, policydbp, 0)) return -1;}} + { if (pass == 1) { if (policydb_index_bools(policydbp)) YYABORT; } + else if (pass == 2) { if (policydb_index_others(NULL, policydbp, 0)) YYABORT; }} initial_sid_contexts opt_fs_contexts opt_fs_uses opt_genfs_contexts net_contexts opt_dev_contexts opt_ibpkey_contexts opt_ibendport_contexts ; classes : class_def | classes class_def ; class_def : CLASS identifier - {if (define_class()) return -1;} + {if (define_class()) YYABORT;} ; initial_sids : initial_sid_def | initial_sids initial_sid_def ; initial_sid_def : SID identifier - {if (define_initial_sid()) return -1;} + {if (define_initial_sid()) YYABORT;} ; access_vectors : opt_common_perms av_perms ; @@ -193,17 +193,17 @@ common_perms : common_perms_def | common_perms common_perms_def ; common_perms_def : COMMON identifier '{' identifier_list '}' - {if (define_common_perms()) return -1;} + {if (define_common_perms()) YYABORT;} ; av_perms : av_perms_def | av_perms av_perms_def ; av_perms_def : CLASS identifier '{' identifier_list '}' - {if (define_av_perms(FALSE)) return -1;} + {if (define_av_perms(FALSE)) YYABORT;} | CLASS identifier INHERITS identifier - {if (define_av_perms(TRUE)) return -1;} + {if (define_av_perms(TRUE)) YYABORT;} | CLASS identifier INHERITS identifier '{' identifier_list '}' - {if (define_av_perms(TRUE)) return -1;} + {if (define_av_perms(TRUE)) YYABORT;} ; opt_default_rules : default_rules | @@ -218,34 +218,34 @@ default_rules : default_user_def | default_rules default_range_def ; default_user_def : DEFAULT_USER names SOURCE ';' - {if (define_default_user(DEFAULT_SOURCE)) return -1; } + {if (define_default_user(DEFAULT_SOURCE)) YYABORT; } | DEFAULT_USER names TARGET ';' - {if (define_default_user(DEFAULT_TARGET)) return -1; } + {if (define_default_user(DEFAULT_TARGET)) YYABORT; } ; default_role_def : DEFAULT_ROLE names SOURCE ';' - {if (define_default_role(DEFAULT_SOURCE)) return -1; } + {if (define_default_role(DEFAULT_SOURCE)) YYABORT; } | DEFAULT_ROLE names TARGET ';' - {if (define_default_role(DEFAULT_TARGET)) return -1; } + {if (define_default_role(DEFAULT_TARGET)) YYABORT; } ; default_type_def : DEFAULT_TYPE names SOURCE ';' - {if (define_default_type(DEFAULT_SOURCE)) return -1; } + {if (define_default_type(DEFAULT_SOURCE)) YYABORT;; } | DEFAULT_TYPE names TARGET ';' - {if (define_default_type(DEFAULT_TARGET)) return -1; } + {if (define_default_type(DEFAULT_TARGET)) YYABORT; } ; default_range_def : DEFAULT_RANGE names SOURCE LOW ';' - {if (define_default_range(DEFAULT_SOURCE_LOW)) return -1; } + {if (define_default_range(DEFAULT_SOURCE_LOW)) YYABORT; } | DEFAULT_RANGE names SOURCE HIGH ';' - {if (define_default_range(DEFAULT_SOURCE_HIGH)) return -1; } + {if (define_default_range(DEFAULT_SOURCE_HIGH)) YYABORT; } | DEFAULT_RANGE names SOURCE LOW_HIGH ';' - {if (define_default_range(DEFAULT_SOURCE_LOW_HIGH)) return -1; } + {if (define_default_range(DEFAULT_SOURCE_LOW_HIGH)) YYABORT; } | DEFAULT_RANGE names TARGET LOW ';' - {if (define_default_range(DEFAULT_TARGET_LOW)) return -1; } + {if (define_default_range(DEFAULT_TARGET_LOW)) YYABORT; } | DEFAULT_RANGE names TARGET HIGH ';' - {if (define_default_range(DEFAULT_TARGET_HIGH)) return -1; } + {if (define_default_range(DEFAULT_TARGET_HIGH)) YYABORT; } | DEFAULT_RANGE names TARGET LOW_HIGH ';' - {if (define_default_range(DEFAULT_TARGET_LOW_HIGH)) return -1; } + {if (define_default_range(DEFAULT_TARGET_LOW_HIGH)) YYABORT; } | DEFAULT_RANGE names GLBLUB';' - {if (define_default_range(DEFAULT_GLBLUB)) return -1; } + {if (define_default_range(DEFAULT_GLBLUB)) YYABORT; } ; opt_mls : mls | @@ -256,16 +256,16 @@ sensitivities : sensitivity_def | sensitivities sensitivity_def ; sensitivity_def : SENSITIVITY identifier alias_def ';' - {if (define_sens()) return -1;} + {if (define_sens()) YYABORT;} | SENSITIVITY identifier ';' - {if (define_sens()) return -1;} + {if (define_sens()) YYABORT;} ; alias_def : ALIAS names ; dominance : DOMINANCE identifier - {if (define_dominance()) return -1;} + {if (define_dominance()) YYABORT;} | DOMINANCE '{' identifier_list '}' - {if (define_dominance()) return -1;} + {if (define_dominance()) YYABORT;} ; opt_categories : categories | @@ -274,17 +274,17 @@ categories : category_def | categories category_def ; category_def : CATEGORY identifier alias_def ';' - {if (define_category()) return -1;} + {if (define_category()) YYABORT;} | CATEGORY identifier ';' - {if (define_category()) return -1;} + {if (define_category()) YYABORT;} ; levels : level_def | levels level_def ; level_def : LEVEL identifier ':' id_comma_list ';' - {if (define_level()) return -1;} + {if (define_level()) YYABORT;} | LEVEL identifier ';' - {if (define_level()) return -1;} + {if (define_level()) YYABORT;} ; mlspolicy : mlspolicy_decl | mlspolicy mlspolicy_decl @@ -293,10 +293,10 @@ mlspolicy_decl : mlsconstraint_def | mlsvalidatetrans_def ; mlsconstraint_def : MLSCONSTRAIN names names cexpr ';' - { if (define_constraint((constraint_expr_t*)$4)) return -1; } + { if (define_constraint((constraint_expr_t*)$4)) YYABORT; } ; mlsvalidatetrans_def : MLSVALIDATETRANS names cexpr ';' - { if (define_validatetrans((constraint_expr_t*)$3)) return -1; } + { if (define_validatetrans((constraint_expr_t*)$3)) YYABORT; } ; te_rbac : te_rbac_decl | te_rbac te_rbac_decl @@ -329,41 +329,41 @@ te_decl : attribute_def | permissive_def ; attribute_def : ATTRIBUTE identifier ';' - { if (define_attrib()) return -1;} + { if (define_attrib()) YYABORT;} ; expandattribute_def : EXPANDATTRIBUTE names bool_val ';' - { if (expand_attrib()) return -1;} + { if (expand_attrib()) YYABORT;} ; type_def : TYPE identifier alias_def opt_attr_list ';' - {if (define_type(1)) return -1;} + {if (define_type(1)) YYABORT;} | TYPE identifier opt_attr_list ';' - {if (define_type(0)) return -1;} + {if (define_type(0)) YYABORT;} ; typealias_def : TYPEALIAS identifier alias_def ';' - {if (define_typealias()) return -1;} + {if (define_typealias()) YYABORT;} ; typeattribute_def : TYPEATTRIBUTE identifier id_comma_list ';' - {if (define_typeattribute()) return -1;} + {if (define_typeattribute()) YYABORT;} ; typebounds_def : TYPEBOUNDS identifier id_comma_list ';' - {if (define_typebounds()) return -1;} + {if (define_typebounds()) YYABORT;} ; opt_attr_list : ',' id_comma_list | ; bool_def : BOOL identifier bool_val ';' - { if (define_bool_tunable(0)) return -1; } + { if (define_bool_tunable(0)) YYABORT; } ; tunable_def : TUNABLE identifier bool_val ';' - { if (define_bool_tunable(1)) return -1; } + { if (define_bool_tunable(1)) YYABORT; } ; bool_val : CTRUE - { if (insert_id("T",0)) return -1; } + { if (insert_id("T",0)) YYABORT; } | CFALSE - { if (insert_id("F",0)) return -1; } + { if (insert_id("F",0)) YYABORT; } ; cond_stmt_def : IF cond_expr '{' cond_pol_list '}' cond_else - { if (pass == 2) { if (define_conditional((cond_expr_t*)$2, (avrule_t*)$4, (avrule_t*)$6) < 0) return -1; }} + { if (pass == 2) { if (define_conditional((cond_expr_t*)$2, (avrule_t*)$4, (avrule_t*)$6) < 0) YYABORT; }} ; cond_else : ELSE '{' cond_pol_list '}' { $$ = $3; } @@ -374,28 +374,28 @@ cond_expr : '(' cond_expr ')' { $$ = $2;} | NOT cond_expr { $$ = define_cond_expr(COND_NOT, $2, 0); - if ($$ == 0) return -1; } + if ($$ == 0) YYABORT; } | cond_expr AND cond_expr { $$ = define_cond_expr(COND_AND, $1, $3); - if ($$ == 0) return -1; } + if ($$ == 0) YYABORT; } | cond_expr OR cond_expr { $$ = define_cond_expr(COND_OR, $1, $3); - if ($$ == 0) return -1; } + if ($$ == 0) YYABORT; } | cond_expr XOR cond_expr { $$ = define_cond_expr(COND_XOR, $1, $3); - if ($$ == 0) return -1; } + if ($$ == 0) YYABORT; } | cond_expr EQUALS cond_expr { $$ = define_cond_expr(COND_EQ, $1, $3); - if ($$ == 0) return -1; } + if ($$ == 0) YYABORT; } | cond_expr NOTEQUAL cond_expr { $$ = define_cond_expr(COND_NEQ, $1, $3); - if ($$ == 0) return -1; } + if ($$ == 0) YYABORT; } | cond_expr_prim { $$ = $1; } ; cond_expr_prim : identifier { $$ = define_cond_expr(COND_BOOL,0, 0); - if ($$ == COND_ERR) return -1; } + if ($$ == COND_ERR) YYABORT; } ; cond_pol_list : cond_pol_list cond_rule_def { $$ = define_cond_pol_list((avrule_t *)$1, (avrule_t *)$2); } @@ -411,16 +411,16 @@ cond_rule_def : cond_transition_def ; cond_transition_def : TYPE_TRANSITION names names ':' names identifier filename ';' { $$ = define_cond_filename_trans() ; - if ($$ == COND_ERR) return -1;} + if ($$ == COND_ERR) YYABORT;} | TYPE_TRANSITION names names ':' names identifier ';' { $$ = define_cond_compute_type(AVRULE_TRANSITION) ; - if ($$ == COND_ERR) return -1;} + if ($$ == COND_ERR) YYABORT;} | TYPE_MEMBER names names ':' names identifier ';' { $$ = define_cond_compute_type(AVRULE_MEMBER) ; - if ($$ == COND_ERR) return -1;} + if ($$ == COND_ERR) YYABORT;} | TYPE_CHANGE names names ':' names identifier ';' { $$ = define_cond_compute_type(AVRULE_CHANGE) ; - if ($$ == COND_ERR) return -1;} + if ($$ == COND_ERR) YYABORT;} ; cond_te_avtab_def : cond_allow_def { $$ = $1; } @@ -433,34 +433,34 @@ cond_te_avtab_def : cond_allow_def ; cond_allow_def : ALLOW names names ':' names names ';' { $$ = define_cond_te_avtab(AVRULE_ALLOWED) ; - if ($$ == COND_ERR) return -1; } + if ($$ == COND_ERR) YYABORT; } ; cond_auditallow_def : AUDITALLOW names names ':' names names ';' { $$ = define_cond_te_avtab(AVRULE_AUDITALLOW) ; - if ($$ == COND_ERR) return -1; } + if ($$ == COND_ERR) YYABORT; } ; cond_auditdeny_def : AUDITDENY names names ':' names names ';' { $$ = define_cond_te_avtab(AVRULE_AUDITDENY) ; - if ($$ == COND_ERR) return -1; } + if ($$ == COND_ERR) YYABORT; } ; cond_dontaudit_def : DONTAUDIT names names ':' names names ';' { $$ = define_cond_te_avtab(AVRULE_DONTAUDIT); - if ($$ == COND_ERR) return -1; } + if ($$ == COND_ERR) YYABORT; } ; ; transition_def : TYPE_TRANSITION names names ':' names identifier filename ';' - {if (define_filename_trans()) return -1; } + {if (define_filename_trans()) YYABORT; } | TYPE_TRANSITION names names ':' names identifier ';' - {if (define_compute_type(AVRULE_TRANSITION)) return -1;} + {if (define_compute_type(AVRULE_TRANSITION)) YYABORT;} | TYPE_MEMBER names names ':' names identifier ';' - {if (define_compute_type(AVRULE_MEMBER)) return -1;} + {if (define_compute_type(AVRULE_MEMBER)) YYABORT;} | TYPE_CHANGE names names ':' names identifier ';' - {if (define_compute_type(AVRULE_CHANGE)) return -1;} + {if (define_compute_type(AVRULE_CHANGE)) YYABORT;} ; range_trans_def : RANGE_TRANSITION names names mls_range_def ';' - { if (define_range_trans(0)) return -1; } + { if (define_range_trans(0)) YYABORT; } | RANGE_TRANSITION names names ':' names mls_range_def ';' - { if (define_range_trans(1)) return -1; } + { if (define_range_trans(1)) YYABORT; } ; te_avtab_def : allow_def | auditallow_def @@ -473,51 +473,51 @@ te_avtab_def : allow_def | xperm_neverallow_def ; allow_def : ALLOW names names ':' names names ';' - {if (define_te_avtab(AVRULE_ALLOWED)) return -1; } + {if (define_te_avtab(AVRULE_ALLOWED)) YYABORT; } ; auditallow_def : AUDITALLOW names names ':' names names ';' - {if (define_te_avtab(AVRULE_AUDITALLOW)) return -1; } + {if (define_te_avtab(AVRULE_AUDITALLOW)) YYABORT; } ; auditdeny_def : AUDITDENY names names ':' names names ';' - {if (define_te_avtab(AVRULE_AUDITDENY)) return -1; } + {if (define_te_avtab(AVRULE_AUDITDENY)) YYABORT; } ; dontaudit_def : DONTAUDIT names names ':' names names ';' - {if (define_te_avtab(AVRULE_DONTAUDIT)) return -1; } + {if (define_te_avtab(AVRULE_DONTAUDIT)) YYABORT; } ; neverallow_def : NEVERALLOW names names ':' names names ';' - {if (define_te_avtab(AVRULE_NEVERALLOW)) return -1; } + {if (define_te_avtab(AVRULE_NEVERALLOW)) YYABORT; } ; xperm_allow_def : ALLOWXPERM names names ':' names identifier xperms ';' - {if (define_te_avtab_extended_perms(AVRULE_XPERMS_ALLOWED)) return -1; } + {if (define_te_avtab_extended_perms(AVRULE_XPERMS_ALLOWED)) YYABORT; } ; xperm_auditallow_def : AUDITALLOWXPERM names names ':' names identifier xperms ';' - {if (define_te_avtab_extended_perms(AVRULE_XPERMS_AUDITALLOW)) return -1; } + {if (define_te_avtab_extended_perms(AVRULE_XPERMS_AUDITALLOW)) YYABORT; } ; xperm_dontaudit_def : DONTAUDITXPERM names names ':' names identifier xperms ';' - {if (define_te_avtab_extended_perms(AVRULE_XPERMS_DONTAUDIT)) return -1; } + {if (define_te_avtab_extended_perms(AVRULE_XPERMS_DONTAUDIT)) YYABORT; } ; xperm_neverallow_def : NEVERALLOWXPERM names names ':' names identifier xperms ';' - {if (define_te_avtab_extended_perms(AVRULE_XPERMS_NEVERALLOW)) return -1; } + {if (define_te_avtab_extended_perms(AVRULE_XPERMS_NEVERALLOW)) YYABORT; } ; attribute_role_def : ATTRIBUTE_ROLE identifier ';' - {if (define_attrib_role()) return -1; } + {if (define_attrib_role()) YYABORT; } ; role_type_def : ROLE identifier TYPES names ';' - {if (define_role_types()) return -1;} + {if (define_role_types()) YYABORT;} ; role_attr_def : ROLE identifier opt_attr_list ';' - {if (define_role_attr()) return -1;} + {if (define_role_attr()) YYABORT;} ; role_trans_def : ROLE_TRANSITION names names identifier ';' - {if (define_role_trans(0)) return -1; } + {if (define_role_trans(0)) YYABORT; } | ROLE_TRANSITION names names ':' names identifier ';' - {if (define_role_trans(1)) return -1;} + {if (define_role_trans(1)) YYABORT;} ; role_allow_def : ALLOW names names ';' - {if (define_role_allow()) return -1; } + {if (define_role_allow()) YYABORT; } ; roleattribute_def : ROLEATTRIBUTE identifier id_comma_list ';' - {if (define_roleattribute()) return -1;} + {if (define_roleattribute()) YYABORT;} ; opt_constraints : constraints | @@ -529,97 +529,97 @@ constraint_decl : constraint_def | validatetrans_def ; constraint_def : CONSTRAIN names names cexpr ';' - { if (define_constraint((constraint_expr_t*)$4)) return -1; } + { if (define_constraint((constraint_expr_t*)$4)) YYABORT; } ; validatetrans_def : VALIDATETRANS names cexpr ';' - { if (define_validatetrans((constraint_expr_t*)$3)) return -1; } + { if (define_validatetrans((constraint_expr_t*)$3)) YYABORT; } ; cexpr : '(' cexpr ')' { $$ = $2; } | NOT cexpr { $$ = define_cexpr(CEXPR_NOT, $2, 0); - if ($$ == 0) return -1; } + if ($$ == 0) YYABORT; } | cexpr AND cexpr { $$ = define_cexpr(CEXPR_AND, $1, $3); - if ($$ == 0) return -1; } + if ($$ == 0) YYABORT; } | cexpr OR cexpr { $$ = define_cexpr(CEXPR_OR, $1, $3); - if ($$ == 0) return -1; } + if ($$ == 0) YYABORT; } | cexpr_prim { $$ = $1; } ; cexpr_prim : U1 op U2 { $$ = define_cexpr(CEXPR_ATTR, CEXPR_USER, $2); - if ($$ == 0) return -1; } + if ($$ == 0) YYABORT; } | R1 role_mls_op R2 { $$ = define_cexpr(CEXPR_ATTR, CEXPR_ROLE, $2); - if ($$ == 0) return -1; } + if ($$ == 0) YYABORT; } | T1 op T2 { $$ = define_cexpr(CEXPR_ATTR, CEXPR_TYPE, $2); - if ($$ == 0) return -1; } - | U1 op { if (insert_separator(1)) return -1; } names_push + if ($$ == 0) YYABORT; } + | U1 op { if (insert_separator(1)) YYABORT; } names_push { $$ = define_cexpr(CEXPR_NAMES, CEXPR_USER, $2); - if ($$ == 0) return -1; } - | U2 op { if (insert_separator(1)) return -1; } names_push + if ($$ == 0) YYABORT; } + | U2 op { if (insert_separator(1)) YYABORT; } names_push { $$ = define_cexpr(CEXPR_NAMES, (CEXPR_USER | CEXPR_TARGET), $2); - if ($$ == 0) return -1; } - | U3 op { if (insert_separator(1)) return -1; } names_push + if ($$ == 0) YYABORT; } + | U3 op { if (insert_separator(1)) YYABORT; } names_push { $$ = define_cexpr(CEXPR_NAMES, (CEXPR_USER | CEXPR_XTARGET), $2); - if ($$ == 0) return -1; } - | R1 op { if (insert_separator(1)) return -1; } names_push + if ($$ == 0) YYABORT; } + | R1 op { if (insert_separator(1)) YYABORT; } names_push { $$ = define_cexpr(CEXPR_NAMES, CEXPR_ROLE, $2); - if ($$ == 0) return -1; } - | R2 op { if (insert_separator(1)) return -1; } names_push + if ($$ == 0) YYABORT; } + | R2 op { if (insert_separator(1)) YYABORT; } names_push { $$ = define_cexpr(CEXPR_NAMES, (CEXPR_ROLE | CEXPR_TARGET), $2); - if ($$ == 0) return -1; } - | R3 op { if (insert_separator(1)) return -1; } names_push + if ($$ == 0) YYABORT; } + | R3 op { if (insert_separator(1)) YYABORT; } names_push { $$ = define_cexpr(CEXPR_NAMES, (CEXPR_ROLE | CEXPR_XTARGET), $2); - if ($$ == 0) return -1; } - | T1 op { if (insert_separator(1)) return -1; } names_push + if ($$ == 0) YYABORT; } + | T1 op { if (insert_separator(1)) YYABORT; } names_push { $$ = define_cexpr(CEXPR_NAMES, CEXPR_TYPE, $2); - if ($$ == 0) return -1; } - | T2 op { if (insert_separator(1)) return -1; } names_push + if ($$ == 0) YYABORT; } + | T2 op { if (insert_separator(1)) YYABORT; } names_push { $$ = define_cexpr(CEXPR_NAMES, (CEXPR_TYPE | CEXPR_TARGET), $2); - if ($$ == 0) return -1; } - | T3 op { if (insert_separator(1)) return -1; } names_push + if ($$ == 0) YYABORT; } + | T3 op { if (insert_separator(1)) YYABORT; } names_push { $$ = define_cexpr(CEXPR_NAMES, (CEXPR_TYPE | CEXPR_XTARGET), $2); - if ($$ == 0) return -1; } + if ($$ == 0) YYABORT; } | SAMEUSER { $$ = define_cexpr(CEXPR_ATTR, CEXPR_USER, CEXPR_EQ); - if ($$ == 0) return -1; } - | SOURCE ROLE { if (insert_separator(1)) return -1; } names_push + if ($$ == 0) YYABORT; } + | SOURCE ROLE { if (insert_separator(1)) YYABORT; } names_push { $$ = define_cexpr(CEXPR_NAMES, CEXPR_ROLE, CEXPR_EQ); - if ($$ == 0) return -1; } - | TARGET ROLE { if (insert_separator(1)) return -1; } names_push + if ($$ == 0) YYABORT; } + | TARGET ROLE { if (insert_separator(1)) YYABORT; } names_push { $$ = define_cexpr(CEXPR_NAMES, (CEXPR_ROLE | CEXPR_TARGET), CEXPR_EQ); - if ($$ == 0) return -1; } + if ($$ == 0) YYABORT; } | ROLE role_mls_op { $$ = define_cexpr(CEXPR_ATTR, CEXPR_ROLE, $2); - if ($$ == 0) return -1; } - | SOURCE TYPE { if (insert_separator(1)) return -1; } names_push + if ($$ == 0) YYABORT; } + | SOURCE TYPE { if (insert_separator(1)) YYABORT; } names_push { $$ = define_cexpr(CEXPR_NAMES, CEXPR_TYPE, CEXPR_EQ); - if ($$ == 0) return -1; } - | TARGET TYPE { if (insert_separator(1)) return -1; } names_push + if ($$ == 0) YYABORT; } + | TARGET TYPE { if (insert_separator(1)) YYABORT; } names_push { $$ = define_cexpr(CEXPR_NAMES, (CEXPR_TYPE | CEXPR_TARGET), CEXPR_EQ); - if ($$ == 0) return -1; } + if ($$ == 0) YYABORT; } | L1 role_mls_op L2 { $$ = define_cexpr(CEXPR_ATTR, CEXPR_L1L2, $2); - if ($$ == 0) return -1; } + if ($$ == 0) YYABORT; } | L1 role_mls_op H2 { $$ = define_cexpr(CEXPR_ATTR, CEXPR_L1H2, $2); - if ($$ == 0) return -1; } + if ($$ == 0) YYABORT; } | H1 role_mls_op L2 { $$ = define_cexpr(CEXPR_ATTR, CEXPR_H1L2, $2); - if ($$ == 0) return -1; } + if ($$ == 0) YYABORT; } | H1 role_mls_op H2 { $$ = define_cexpr(CEXPR_ATTR, CEXPR_H1H2, $2); - if ($$ == 0) return -1; } + if ($$ == 0) YYABORT; } | L1 role_mls_op H1 { $$ = define_cexpr(CEXPR_ATTR, CEXPR_L1H1, $2); - if ($$ == 0) return -1; } + if ($$ == 0) YYABORT; } | L2 role_mls_op H2 { $$ = define_cexpr(CEXPR_ATTR, CEXPR_L2H2, $2); - if ($$ == 0) return -1; } + if ($$ == 0) YYABORT; } ; op : EQUALS { $$ = CEXPR_EQ; } @@ -639,7 +639,7 @@ users : user_def | users user_def ; user_def : USER identifier ROLES names opt_mls_user ';' - {if (define_user()) return -1;} + {if (define_user()) YYABORT;} ; opt_mls_user : LEVEL mls_level_def RANGE mls_range_def | @@ -648,7 +648,7 @@ initial_sid_contexts : initial_sid_context_def | initial_sid_contexts initial_sid_context_def ; initial_sid_context_def : SID identifier security_context_def - {if (define_initial_sid_context()) return -1;} + {if (define_initial_sid_context()) YYABORT;} ; opt_dev_contexts : dev_contexts | ; @@ -662,23 +662,23 @@ dev_context_def : pirq_context_def | dtree_context_def ; pirq_context_def : PIRQCON number security_context_def - {if (define_pirq_context($2)) return -1;} + {if (define_pirq_context($2)) YYABORT;} ; iomem_context_def : IOMEMCON number64 security_context_def - {if (define_iomem_context($2,$2)) return -1;} + {if (define_iomem_context($2,$2)) YYABORT;} | IOMEMCON number64 '-' number64 security_context_def - {if (define_iomem_context($2,$4)) return -1;} + {if (define_iomem_context($2,$4)) YYABORT;} ; ioport_context_def : IOPORTCON number security_context_def - {if (define_ioport_context($2,$2)) return -1;} + {if (define_ioport_context($2,$2)) YYABORT;} | IOPORTCON number '-' number security_context_def - {if (define_ioport_context($2,$4)) return -1;} + {if (define_ioport_context($2,$4)) YYABORT;} ; pci_context_def : PCIDEVICECON number security_context_def - {if (define_pcidevice_context($2)) return -1;} + {if (define_pcidevice_context($2)) YYABORT;} ; dtree_context_def : DEVICETREECON path security_context_def - {if (define_devicetree_context()) return -1;} + {if (define_devicetree_context()) YYABORT;} ; opt_fs_contexts : fs_contexts | @@ -687,7 +687,7 @@ fs_contexts : fs_context_def | fs_contexts fs_context_def ; fs_context_def : FSCON number number security_context_def security_context_def - {if (define_fs_context($2,$3)) return -1;} + {if (define_fs_context($2,$3)) YYABORT;} ; net_contexts : opt_port_contexts opt_netif_contexts opt_node_contexts ; @@ -698,9 +698,9 @@ port_contexts : port_context_def | port_contexts port_context_def ; port_context_def : PORTCON identifier number security_context_def - {if (define_port_context($3,$3)) return -1;} + {if (define_port_context($3,$3)) YYABORT;} | PORTCON identifier number '-' number security_context_def - {if (define_port_context($3,$5)) return -1;} + {if (define_port_context($3,$5)) YYABORT;} ; opt_ibpkey_contexts : ibpkey_contexts | @@ -709,9 +709,9 @@ ibpkey_contexts : ibpkey_context_def | ibpkey_contexts ibpkey_context_def ; ibpkey_context_def : IBPKEYCON ipv6_addr number security_context_def - {if (define_ibpkey_context($3,$3)) return -1;} + {if (define_ibpkey_context($3,$3)) YYABORT;} | IBPKEYCON ipv6_addr number '-' number security_context_def - {if (define_ibpkey_context($3,$5)) return -1;} + {if (define_ibpkey_context($3,$5)) YYABORT;} ; opt_ibendport_contexts : ibendport_contexts | @@ -720,7 +720,7 @@ ibendport_contexts : ibendport_context_def | ibendport_contexts ibendport_context_def ; ibendport_context_def : IBENDPORTCON identifier number security_context_def - {if (define_ibendport_context($3)) return -1;} + {if (define_ibendport_context($3)) YYABORT;} ; opt_netif_contexts : netif_contexts | @@ -729,7 +729,7 @@ netif_contexts : netif_context_def | netif_contexts netif_context_def ; netif_context_def : NETIFCON identifier security_context_def security_context_def - {if (define_netif_context()) return -1;} + {if (define_netif_context()) YYABORT;} ; opt_node_contexts : node_contexts | @@ -738,9 +738,9 @@ node_contexts : node_context_def | node_contexts node_context_def ; node_context_def : NODECON ipv4_addr_def ipv4_addr_def security_context_def - {if (define_ipv4_node_context()) return -1;} + {if (define_ipv4_node_context()) YYABORT;} | NODECON ipv6_addr ipv6_addr security_context_def - {if (define_ipv6_node_context()) return -1;} + {if (define_ipv6_node_context()) YYABORT;} ; opt_fs_uses : fs_uses | @@ -749,11 +749,11 @@ fs_uses : fs_use_def | fs_uses fs_use_def ; fs_use_def : FSUSEXATTR filesystem security_context_def ';' - {if (define_fs_use(SECURITY_FS_USE_XATTR)) return -1;} + {if (define_fs_use(SECURITY_FS_USE_XATTR)) YYABORT;} | FSUSETASK identifier security_context_def ';' - {if (define_fs_use(SECURITY_FS_USE_TASK)) return -1;} + {if (define_fs_use(SECURITY_FS_USE_TASK)) YYABORT;} | FSUSETRANS identifier security_context_def ';' - {if (define_fs_use(SECURITY_FS_USE_TRANS)) return -1;} + {if (define_fs_use(SECURITY_FS_USE_TRANS)) YYABORT;} ; opt_genfs_contexts : genfs_contexts | @@ -762,36 +762,36 @@ genfs_contexts : genfs_context_def | genfs_contexts genfs_context_def ; genfs_context_def : GENFSCON filesystem path '-' identifier security_context_def - {if (define_genfs_context(1)) return -1;} + {if (define_genfs_context(1)) YYABORT;} | GENFSCON filesystem path '-' '-' {insert_id("-", 0);} security_context_def - {if (define_genfs_context(1)) return -1;} + {if (define_genfs_context(1)) YYABORT;} | GENFSCON filesystem path security_context_def - {if (define_genfs_context(0)) return -1;} + {if (define_genfs_context(0)) YYABORT;} ; ipv4_addr_def : IPV4_ADDR - { if (insert_id(yytext,0)) return -1; } + { if (insert_id(yytext,0)) YYABORT; } ; xperms : xperm - { if (insert_separator(0)) return -1; } + { if (insert_separator(0)) YYABORT; } | nested_xperm_set - { if (insert_separator(0)) return -1; } + { if (insert_separator(0)) YYABORT; } | tilde xperm - { if (insert_id("~", 0)) return -1; } + { if (insert_id("~", 0)) YYABORT; } | tilde nested_xperm_set - { if (insert_id("~", 0)) return -1; - if (insert_separator(0)) return -1; } + { if (insert_id("~", 0)) YYABORT; + if (insert_separator(0)) YYABORT; } ; nested_xperm_set : '{' nested_xperm_list '}' ; nested_xperm_list : nested_xperm_element | nested_xperm_list nested_xperm_element ; -nested_xperm_element: xperm '-' { if (insert_id("-", 0)) return -1; } xperm +nested_xperm_element: xperm '-' { if (insert_id("-", 0)) YYABORT; } xperm | xperm | nested_xperm_set ; xperm : number - { if (insert_id(yytext,0)) return -1; } + { if (insert_id(yytext,0)) YYABORT; } ; security_context_def : identifier ':' identifier ':' identifier opt_mls_range_def ; @@ -799,14 +799,14 @@ opt_mls_range_def : ':' mls_range_def | ; mls_range_def : mls_level_def '-' mls_level_def - {if (insert_separator(0)) return -1;} + {if (insert_separator(0)) YYABORT;} | mls_level_def - {if (insert_separator(0)) return -1;} + {if (insert_separator(0)) YYABORT;} ; mls_level_def : identifier ':' id_comma_list - {if (insert_separator(0)) return -1;} + {if (insert_separator(0)) YYABORT;} | identifier - {if (insert_separator(0)) return -1;} + {if (insert_separator(0)) YYABORT;} ; id_comma_list : identifier | id_comma_list ',' identifier @@ -816,26 +816,26 @@ tilde : '~' asterisk : '*' ; names : identifier - { if (insert_separator(0)) return -1; } + { if (insert_separator(0)) YYABORT; } | nested_id_set - { if (insert_separator(0)) return -1; } + { if (insert_separator(0)) YYABORT; } | asterisk - { if (insert_id("*", 0)) return -1; - if (insert_separator(0)) return -1; } + { if (insert_id("*", 0)) YYABORT; + if (insert_separator(0)) YYABORT; } | tilde identifier - { if (insert_id("~", 0)) return -1; - if (insert_separator(0)) return -1; } + { if (insert_id("~", 0)) YYABORT; + if (insert_separator(0)) YYABORT; } | tilde nested_id_set - { if (insert_id("~", 0)) return -1; - if (insert_separator(0)) return -1; } - | identifier '-' { if (insert_id("-", 0)) return -1; } identifier - { if (insert_separator(0)) return -1; } + { if (insert_id("~", 0)) YYABORT; + if (insert_separator(0)) YYABORT; } + | identifier '-' { if (insert_id("-", 0)) YYABORT; } identifier + { if (insert_separator(0)) YYABORT; } ; tilde_push : tilde - { if (insert_id("~", 1)) return -1; } + { if (insert_id("~", 1)) YYABORT; } ; asterisk_push : asterisk - { if (insert_id("*", 1)) return -1; } + { if (insert_id("*", 1)) YYABORT; } ; names_push : identifier_push | '{' identifier_list_push '}' @@ -847,7 +847,7 @@ identifier_list_push : identifier_push | identifier_list_push identifier_push ; identifier_push : IDENTIFIER - { if (insert_id(yytext, 1)) return -1; } + { if (insert_id(yytext, 1)) YYABORT; } ; identifier_list : identifier | identifier_list identifier @@ -856,33 +856,33 @@ nested_id_set : '{' nested_id_list '}' ; nested_id_list : nested_id_element | nested_id_list nested_id_element ; -nested_id_element : identifier | '-' { if (insert_id("-", 0)) return -1; } identifier | nested_id_set +nested_id_element : identifier | '-' { if (insert_id("-", 0)) YYABORT; } identifier | nested_id_set ; identifier : IDENTIFIER - { if (insert_id(yytext,0)) return -1; } + { if (insert_id(yytext,0)) YYABORT; } ; filesystem : FILESYSTEM - { if (insert_id(yytext,0)) return -1; } + { if (insert_id(yytext,0)) YYABORT; } | IDENTIFIER - { if (insert_id(yytext,0)) return -1; } + { if (insert_id(yytext,0)) YYABORT; } ; path : PATH - { if (insert_id(yytext,0)) return -1; } + { if (insert_id(yytext,0)) YYABORT; } | QPATH - { yytext[strlen(yytext) - 1] = '\0'; if (insert_id(yytext + 1,0)) return -1; } + { yytext[strlen(yytext) - 1] = '\0'; if (insert_id(yytext + 1,0)) YYABORT; } ; filename : FILENAME - { yytext[strlen(yytext) - 1] = '\0'; if (insert_id(yytext + 1,0)) return -1; } + { yytext[strlen(yytext) - 1] = '\0'; if (insert_id(yytext + 1,0)) YYABORT; } ; number : NUMBER { unsigned long x; errno = 0; x = strtoul(yytext, NULL, 0); if (errno) - return -1; + YYABORT; #if ULONG_MAX > UINT_MAX if (x > UINT_MAX) - return -1; + YYABORT; #endif $$ = (unsigned int) x; } @@ -892,33 +892,33 @@ number64 : NUMBER errno = 0; x = strtoull(yytext, NULL, 0); if (errno) - return -1; + YYABORT; $$ = (uint64_t) x; } ; ipv6_addr : IPV6_ADDR - { if (insert_id(yytext,0)) return -1; } + { if (insert_id(yytext,0)) YYABORT; } ; policycap_def : POLICYCAP identifier ';' - {if (define_polcap()) return -1;} + {if (define_polcap()) YYABORT;} ; permissive_def : PERMISSIVE identifier ';' - {if (define_permissive()) return -1;} + {if (define_permissive()) YYABORT;} /*********** module grammar below ***********/ module_policy : module_def avrules_block - { if (end_avrule_block(pass) == -1) return -1; - if (policydb_index_others(NULL, policydbp, 0)) return -1; + { if (end_avrule_block(pass) == -1) YYABORT; + if (policydb_index_others(NULL, policydbp, 0)) YYABORT; } ; module_def : MODULE identifier version_identifier ';' - { if (define_policy(pass, 1) == -1) return -1; } + { if (define_policy(pass, 1) == -1) YYABORT; } ; version_identifier : VERSION_IDENTIFIER - { if (insert_id(yytext,0)) return -1; } + { if (insert_id(yytext,0)) YYABORT; } | number - { if (insert_id(yytext,0)) return -1; } + { if (insert_id(yytext,0)) YYABORT; } | ipv4_addr_def /* version can look like ipv4 address */ ; avrules_block : avrule_decls avrule_user_defs @@ -942,7 +942,7 @@ require_decl : require_class ';' | require_decl_def require_id_list ';' ; require_class : CLASS identifier names - { if (require_class(pass)) return -1; } + { if (require_class(pass)) YYABORT; } ; require_decl_def : ROLE { $$ = require_role; } | TYPE { $$ = require_type; } @@ -955,24 +955,24 @@ require_decl_def : ROLE { $$ = require_role; } | CATEGORY { $$ = require_cat; } ; require_id_list : identifier - { if ($0 (pass)) return -1; } + { if ($0 (pass)) YYABORT; } | require_id_list ',' identifier - { if ($0 (pass)) return -1; } + { if ($0 (pass)) YYABORT; } ; optional_block : optional_decl '{' avrules_block '}' - { if (end_avrule_block(pass) == -1) return -1; } + { if (end_avrule_block(pass) == -1) YYABORT; } optional_else - { if (end_optional(pass) == -1) return -1; } + { if (end_optional(pass) == -1) YYABORT; } ; optional_else : else_decl '{' avrules_block '}' - { if (end_avrule_block(pass) == -1) return -1; } + { if (end_avrule_block(pass) == -1) YYABORT; } | /* empty */ ; optional_decl : OPTIONAL - { if (begin_optional(pass) == -1) return -1; } + { if (begin_optional(pass) == -1) YYABORT; } ; else_decl : ELSE - { if (begin_optional_else(pass) == -1) return -1; } + { if (begin_optional_else(pass) == -1) YYABORT; } ; avrule_user_defs : user_def avrule_user_defs | /* empty */ From ee32f0b20a713241eb66e1caecc2e0a9f96144fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Mon, 22 Jan 2024 14:24:16 +0100 Subject: [PATCH 08/15] checkpolicy: bail out on invalid role MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Return early on invalid roles in user definition. Signed-off-by: Christian Göttsche --- checkpolicy/policy_define.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c index 975826308e..44236797c7 100644 --- a/checkpolicy/policy_define.c +++ b/checkpolicy/policy_define.c @@ -4244,7 +4244,7 @@ int define_user(void) while ((id = queue_remove(id_queue))) { if (set_user_roles(&usrdatum->roles, id)) - continue; + return -1; } if (mlspol) { From 7a2120587d29df1c30c8cc7b846096dd9ecdc2b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Mon, 22 Jan 2024 14:24:18 +0100 Subject: [PATCH 09/15] libsepol: use typedef MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Convert the only usage of the raw type struct level_datum to use the typedef. Simplifies refactorizations on the type. Signed-off-by: Christian Göttsche --- libsepol/src/module_to_cil.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libsepol/src/module_to_cil.c b/libsepol/src/module_to_cil.c index ee22dbbdb3..417c5307aa 100644 --- a/libsepol/src/module_to_cil.c +++ b/libsepol/src/module_to_cil.c @@ -2390,7 +2390,7 @@ static int boolean_to_cil(int indent, struct policydb *UNUSED(pdb), struct avrul static int sens_to_cil(int indent, struct policydb *pdb, struct avrule_block *UNUSED(block), struct stack *UNUSED(decl_stack), char *key, void *datum, int scope) { - struct level_datum *level = datum; + level_datum_t *level = datum; if (scope == SCOPE_DECL) { if (!level->isalias) { From ce8ddafc8f3b9543496315fb0e147636e657ad8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Mon, 22 Jan 2024 14:24:19 +0100 Subject: [PATCH 10/15] libsepol: add copy member to level_datum MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a new member to the struct level_datum to indicate whether the member `level` is owned by the current instance, and free it on cleanup only then. This helps to implement a fix for a use-after-free issue in the checkpolicy(8) compiler. Signed-off-by: Christian Göttsche --- libsepol/include/sepol/policydb/policydb.h | 1 + libsepol/src/policydb.c | 6 ++++-- libsepol/src/policydb_validate.c | 3 +++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/libsepol/include/sepol/policydb/policydb.h b/libsepol/include/sepol/policydb/policydb.h index 6682069eaf..06231574da 100644 --- a/libsepol/include/sepol/policydb/policydb.h +++ b/libsepol/include/sepol/policydb/policydb.h @@ -218,6 +218,7 @@ typedef struct level_datum { mls_level_t *level; /* sensitivity and associated categories */ unsigned char isalias; /* is this sensitivity an alias for another? */ unsigned char defined; + unsigned char copy; /* whether the level is a non-owned copy (compile time only) */ } level_datum_t; /* Category attributes */ diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c index f10a8a95ac..322ab745f8 100644 --- a/libsepol/src/policydb.c +++ b/libsepol/src/policydb.c @@ -1390,8 +1390,10 @@ static int sens_destroy(hashtab_key_t key, hashtab_datum_t datum, void *p if (key) free(key); levdatum = (level_datum_t *) datum; - mls_level_destroy(levdatum->level); - free(levdatum->level); + if (!levdatum->copy) { + mls_level_destroy(levdatum->level); + free(levdatum->level); + } level_datum_destroy(levdatum); free(levdatum); return 0; diff --git a/libsepol/src/policydb_validate.c b/libsepol/src/policydb_validate.c index d86f885e4a..e3af7ccd11 100644 --- a/libsepol/src/policydb_validate.c +++ b/libsepol/src/policydb_validate.c @@ -623,6 +623,9 @@ static int validate_level_datum(__attribute__ ((unused)) hashtab_key_t k, hashta level_datum_t *level = d; validate_t *flavors = args; + if (level->copy) + return -1; + return validate_mls_level(level->level, &flavors[SYM_LEVELS], &flavors[SYM_CATS]); } From ecb67d0b097a60ceb332dbec1e3129657b0610af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Mon, 22 Jan 2024 14:24:20 +0100 Subject: [PATCH 11/15] checkpolicy: fix use-after-free on invalid sens alias MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit During compilation sensitivity aliases share the level with their prime sensitivity, until after the level has been fully defined they are deduplicated. If an error happens by that time the cleanup will free the shared level multiple times, leading to a use-after-free. Make use of the added new member of the struct level_datum. Example policy: class c sid e class c{i}sensitivity S alias L; Signed-off-by: Christian Göttsche --- checkpolicy/fuzz/checkpolicy-fuzzer.c | 7 +++++++ checkpolicy/policy_define.c | 3 +++ 2 files changed, 10 insertions(+) diff --git a/checkpolicy/fuzz/checkpolicy-fuzzer.c b/checkpolicy/fuzz/checkpolicy-fuzzer.c index 0d749a0299..d0221f3ff0 100644 --- a/checkpolicy/fuzz/checkpolicy-fuzzer.c +++ b/checkpolicy/fuzz/checkpolicy-fuzzer.c @@ -134,6 +134,13 @@ static int check_level(hashtab_key_t key, hashtab_datum_t datum, void *arg __att { const level_datum_t *levdatum = (level_datum_t *) datum; + if (levdatum->copy) { + fprintf(stderr, + "Error: sensitivity %s is still a copy!\n", + key); + abort(); + } + // TODO: drop member defined if proven to be always set if (!levdatum->isalias && !levdatum->defined) { fprintf(stderr, diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c index 44236797c7..360cff6887 100644 --- a/checkpolicy/policy_define.c +++ b/checkpolicy/policy_define.c @@ -756,6 +756,7 @@ int define_sens(void) } level_datum_init(datum); datum->isalias = FALSE; + datum->copy = FALSE; datum->level = level; ret = declare_symbol(SYM_LEVELS, id, datum, &value, &value); @@ -795,6 +796,7 @@ int define_sens(void) } level_datum_init(aliasdatum); aliasdatum->isalias = TRUE; + aliasdatum->copy = TRUE; aliasdatum->level = level; ret = declare_symbol(SYM_LEVELS, id, aliasdatum, NULL, &value); @@ -1035,6 +1037,7 @@ static int clone_level(hashtab_key_t key __attribute__ ((unused)), hashtab_datum return -1; } levdatum->level = newlevel; + levdatum->copy = FALSE; } return 0; } From 9c7b2be26a1ba7e324a45bccaf594f2a0e82ab45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Mon, 22 Jan 2024 14:24:21 +0100 Subject: [PATCH 12/15] checkpolicy: provide more descriptive error messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Provide more descriptive error messages by including the identifier or other kind of value if available. Also drop duplicate newlines at the end of messages. Signed-off-by: Christian Göttsche --- checkpolicy/module_compiler.c | 6 +- checkpolicy/policy_define.c | 123 ++++++++++++++++++++-------------- 2 files changed, 76 insertions(+), 53 deletions(-) diff --git a/checkpolicy/module_compiler.c b/checkpolicy/module_compiler.c index 74a9f93c09..119b7e3646 100644 --- a/checkpolicy/module_compiler.c +++ b/checkpolicy/module_compiler.c @@ -75,7 +75,7 @@ static void print_error_msg(int ret, uint32_t symbol_type) yyerror2("Could not declare %s here", flavor_str[symbol_type]); break; default: - yyerror("Unknown error"); + yyerror2("Unknown error %d", ret); } } @@ -86,7 +86,7 @@ int define_policy(int pass, int module_header_given) if (module_header_given) { if (policydbp->policy_type != POLICY_MOD) { yyerror - ("Module specification found while not building a policy module.\n"); + ("Module specification found while not building a policy module."); return -1; } @@ -111,7 +111,7 @@ int define_policy(int pass, int module_header_given) } else { if (policydbp->policy_type == POLICY_MOD) { yyerror - ("Building a policy module, but no module specification found.\n"); + ("Building a policy module, but no module specification found."); return -1; } } diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c index 360cff6887..cd49cae37a 100644 --- a/checkpolicy/policy_define.c +++ b/checkpolicy/policy_define.c @@ -96,6 +96,17 @@ void yyerror2(const char *fmt, ...) va_end(ap); } +__attribute__ ((format(printf, 1, 2))) +static void yywarn2(const char *fmt, ...) +{ + char warnmsg[256]; + va_list ap; + va_start(ap, fmt); + vsnprintf(warnmsg, sizeof(warnmsg), fmt, ap); + yywarn(warnmsg); + va_end(ap); +} + int insert_separator(int push) { int error; @@ -233,7 +244,7 @@ int define_permissive(void) } if (t->flavor == TYPE_ATTRIB) { - yyerror2("attributes may not be permissive: %s\n", type); + yyerror2("attributes may not be permissive: %s", type); rc = -1; goto out; } @@ -520,7 +531,7 @@ int define_common_perms(void) } comdatum = hashtab_search(policydbp->p_commons.table, id); if (comdatum) { - yyerror2("duplicate declaration for common %s\n", id); + yyerror2("duplicate declaration for common %s", id); free(id); return -1; } @@ -557,8 +568,8 @@ int define_common_perms(void) perdatum->s.value = comdatum->permissions.nprim + 1; if (perdatum->s.value > (sizeof(sepol_access_vector_t) * 8)) { - yyerror - ("too many permissions to fit in an access vector"); + yyerror2 + ("too many permissions (%d) to fit in an access vector", perdatum->s.value); goto bad_perm; } ret = hashtab_insert(comdatum->permissions.table, @@ -619,12 +630,15 @@ int define_av_perms(int inherits) yyerror2("class %s is not defined", id); goto bad; } - free(id); if (cladatum->comdatum || cladatum->permissions.nprim) { - yyerror("duplicate access vector definition"); - return -1; + yyerror2("duplicate access vector definition for class %s", id); + goto bad; } + + free(id); + id = NULL; + if (symtab_init(&cladatum->permissions, PERM_SYMTAB_SIZE)) { yyerror("out of memory"); return -1; @@ -664,8 +678,8 @@ int define_av_perms(int inherits) perdatum->s.value = ++cladatum->permissions.nprim; if (perdatum->s.value > (sizeof(sepol_access_vector_t) * 8)) { - yyerror - ("too many permissions to fit in an access vector"); + yyerror2 + ("too many permissions (%d) to fit in an access vector", perdatum->s.value); goto bad; } if (inherits) { @@ -737,7 +751,7 @@ int define_sens(void) return -1; } if (id_has_dot(id)) { - yyerror("sensitivity identifiers may not contain periods"); + yyerror2("sensitivity identifier %s may not contain periods", id); goto bad; } level = (mls_level_t *) malloc(sizeof(mls_level_t)); @@ -766,11 +780,11 @@ int define_sens(void) goto bad; } case -2:{ - yyerror("duplicate declaration of sensitivity level"); + yyerror2("duplicate declaration of sensitivity level %s", id); goto bad; } case -1:{ - yyerror("could not declare sensitivity level here"); + yyerror2("could not declare sensitivity level %s here", id); goto bad; } case 0: @@ -784,7 +798,7 @@ int define_sens(void) while ((id = queue_remove(id_queue))) { if (id_has_dot(id)) { - yyerror("sensitivity aliases may not contain periods"); + yyerror2("sensitivity alias %s may not contain periods", id); free(id); return -1; } @@ -806,13 +820,13 @@ int define_sens(void) goto bad_alias; } case -2:{ - yyerror - ("duplicate declaration of sensitivity alias"); + yyerror2 + ("duplicate declaration of sensitivity alias %s", id); goto bad_alias; } case -1:{ - yyerror - ("could not declare sensitivity alias here"); + yyerror2 + ("could not declare sensitivity alias %s here", id); goto bad_alias; } case 0: @@ -920,7 +934,7 @@ int define_category(void) return -1; } if (id_has_dot(id)) { - yyerror("category identifiers may not contain periods"); + yyerror2("category identifier %s may not contain periods", id); goto bad; } datum = (cat_datum_t *) malloc(sizeof(cat_datum_t)); @@ -938,11 +952,11 @@ int define_category(void) goto bad; } case -2:{ - yyerror("duplicate declaration of category"); + yyerror2("duplicate declaration of category %s", id); goto bad; } case -1:{ - yyerror("could not declare category here"); + yyerror2("could not declare category %s here", id); goto bad; } case 0: @@ -957,7 +971,7 @@ int define_category(void) while ((id = queue_remove(id_queue))) { if (id_has_dot(id)) { - yyerror("category aliases may not contain periods"); + yyerror2("category alias %s may not contain periods", id); free(id); return -1; } @@ -980,13 +994,13 @@ int define_category(void) goto bad_alias; } case -2:{ - yyerror - ("duplicate declaration of category aliases"); + yyerror2 + ("duplicate declaration of category alias %s", id); goto bad_alias; } case -1:{ - yyerror - ("could not declare category aliases here"); + yyerror2 + ("could not declare category alias %s here", id); goto bad_alias; } case 0: @@ -1114,7 +1128,7 @@ int define_level(void) range_end = cdatum->s.value - 1; if (range_end < range_start) { - yyerror2("category range is invalid"); + yyerror2("category range %d-%d is invalid", range_start, range_end); free(id); return -1; } @@ -1170,6 +1184,7 @@ int expand_attrib(void) ebitmap_t attrs; type_datum_t *attr; ebitmap_node_t *node; + const char *name; uint32_t i; int rc = -1; int flags = 0; @@ -1222,13 +1237,13 @@ int expand_attrib(void) } ebitmap_for_each_positive_bit(&attrs, node, i) { - attr = hashtab_search(policydbp->p_types.table, - policydbp->sym_val_to_name[SYM_TYPES][i]); + name = policydbp->sym_val_to_name[SYM_TYPES][i]; + attr = hashtab_search(policydbp->p_types.table, name); attr->flags |= flags; if ((attr->flags & TYPE_FLAGS_EXPAND_ATTR_TRUE) && (attr->flags & TYPE_FLAGS_EXPAND_ATTR_FALSE)) { - yywarn("Expandattribute option was set to both true and false. " - "Resolving to false."); + yywarn2("Expandattribute option of attribute %s was set to both true and false; " + "Resolving to false.", name); attr->flags &= ~TYPE_FLAGS_EXPAND_ATTR_TRUE; } } @@ -1247,9 +1262,9 @@ static int add_aliases_to_type(type_datum_t * type) int ret; while ((id = queue_remove(id_queue))) { if (id_has_dot(id)) { + yyerror2 + ("type alias identifier %s may not contain periods", id); free(id); - yyerror - ("type alias identifiers may not contain periods"); return -1; } aliasdatum = (type_datum_t *) malloc(sizeof(type_datum_t)); @@ -1274,7 +1289,7 @@ static int add_aliases_to_type(type_datum_t * type) goto cleanup; } case -1:{ - yyerror("could not declare alias here"); + yyerror2("could not declare alias %s here", id); goto cleanup; } case 0: break; @@ -1790,8 +1805,8 @@ int define_bool_tunable(int is_tunable) return -1; } if (id_has_dot(id)) { + yyerror2("boolean identifier %s may not contain periods", id); free(id); - yyerror("boolean identifiers may not contain periods"); return -1; } datum = (cond_bool_datum_t *) malloc(sizeof(cond_bool_datum_t)); @@ -1814,7 +1829,7 @@ int define_bool_tunable(int is_tunable) goto cleanup; } case -1:{ - yyerror("could not declare boolean here"); + yyerror2("could not declare boolean %s here", id); goto cleanup; } case 0: @@ -1957,7 +1972,8 @@ static int avrule_read_ioctls(struct av_ioctl_range_list **rangehead) id = queue_remove(id_queue); r->range.high = (uint16_t) strtoul(id,NULL,0); if (r->range.high < r->range.low) { - yyerror("Ioctl ranges must be in ascending order."); + yyerror2("Ioctl range %d-%d must be in ascending order.", + r->range.low, r->range.high); return -1; } free(id); @@ -2532,7 +2548,7 @@ int define_te_avtab_extended_perms(int which) if (strcmp(id,"ioctl") == 0) { rc = define_te_avtab_ioctl(avrule_template); } else { - yyerror("only ioctl extended permissions are supported"); + yyerror2("only ioctl extended permissions are supported, found %s", id); rc = -1; } @@ -3189,7 +3205,7 @@ int define_role_allow(void) avrule_t *define_cond_filename_trans(void) { yyerror("type transitions with a filename not allowed inside " - "conditionals\n"); + "conditionals"); return COND_ERR; } @@ -3861,7 +3877,7 @@ int define_conditional(cond_expr_t * expr, avrule_t * t, avrule_t * f) f = 0; expr = define_cond_expr(COND_NOT, expr, 0); if (!expr) { - yyerror("unable to invert"); + yyerror("unable to invert conditional expression"); return -1; } } @@ -4126,7 +4142,7 @@ static int parse_categories(char *id, level_datum_t * levdatum, ebitmap_t * cats range_end = cdatum->s.value - 1; if (range_end < range_start) { - yyerror2("category range is invalid"); + yyerror2("category range %d-%d is invalid", range_start, range_end); return -1; } } else { @@ -5102,7 +5118,7 @@ int define_ibendport_context(unsigned int port) } if (port > 0xff || port == 0) { - yyerror("Invalid ibendport port number, should be 0 < port < 256"); + yyerror2("Invalid ibendport port number %d, should be 0 < port < 256", port); return -1; } @@ -5121,7 +5137,7 @@ int define_ibendport_context(unsigned int port) } if (strlen(newc->u.ibendport.dev_name) > IB_DEVICE_NAME_MAX - 1) { - yyerror("infiniband device name exceeds max length of 63."); + yyerror2("infiniband device name %s exceeds max length of 63.", newc->u.ibendport.dev_name); rc = -1; goto out; } @@ -5248,13 +5264,14 @@ int define_ipv4_node_context(void) } rc = inet_pton(AF_INET, id, &addr); - free(id); if (rc < 1) { - yyerror("failed to parse ipv4 address"); + yyerror2("failed to parse ipv4 address %s", id); + free(id); if (rc == 0) rc = -1; goto out; } + free(id); id = queue_remove(id_queue); if (!id) { @@ -5264,14 +5281,16 @@ int define_ipv4_node_context(void) } rc = inet_pton(AF_INET, id, &mask); - free(id); if (rc < 1) { - yyerror("failed to parse ipv4 mask"); + yyerror2("failed to parse ipv4 mask %s", id); + free(id); if (rc == 0) rc = -1; goto out; } + free(id); + if (mask.s_addr != 0 && ((~mask.s_addr + 1) & ~mask.s_addr) != 0) { yywarn("ipv4 mask is not contiguous"); } @@ -5376,14 +5395,16 @@ int define_ipv6_node_context(void) } rc = inet_pton(AF_INET6, id, &addr); - free(id); if (rc < 1) { - yyerror("failed to parse ipv6 address"); + yyerror2("failed to parse ipv6 address %s", id); + free(id); if (rc == 0) rc = -1; goto out; } + free(id); + id = queue_remove(id_queue); if (!id) { yyerror("failed to read ipv6 address"); @@ -5392,14 +5413,16 @@ int define_ipv6_node_context(void) } rc = inet_pton(AF_INET6, id, &mask); - free(id); if (rc < 1) { - yyerror("failed to parse ipv6 mask"); + yyerror2("failed to parse ipv6 mask %s", id); + free(id); if (rc == 0) rc = -1; goto out; } + free(id); + if (!ipv6_is_mask_contiguous(&mask)) { yywarn("ipv6 mask is not contiguous"); } From 42fd67ededd2230b4b8ff88779e7124f80616336 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Mon, 22 Jan 2024 14:24:23 +0100 Subject: [PATCH 13/15] checkpolicy: free temporary bounds type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Free the temporary bounds type in the error branches. Signed-off-by: Christian Göttsche --- checkpolicy/module_compiler.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/checkpolicy/module_compiler.c b/checkpolicy/module_compiler.c index 119b7e3646..464897cc8b 100644 --- a/checkpolicy/module_compiler.c +++ b/checkpolicy/module_compiler.c @@ -234,6 +234,7 @@ static int role_implicit_bounds(hashtab_t roles_tab, if (!bounds) { yyerror2("role %s doesn't exist, is implicit bounds of %s", bounds_id, role_id); + free(bounds_id); return -1; } @@ -243,6 +244,7 @@ static int role_implicit_bounds(hashtab_t roles_tab, yyerror2("role %s has inconsistent bounds %s/%s", role_id, bounds_id, policydbp->p_role_val_to_name[role->bounds - 1]); + free(bounds_id); return -1; } free(bounds_id); @@ -479,6 +481,7 @@ static int user_implicit_bounds(hashtab_t users_tab, if (!bounds) { yyerror2("user %s doesn't exist, is implicit bounds of %s", bounds_id, user_id); + free(bounds_id); return -1; } @@ -488,6 +491,7 @@ static int user_implicit_bounds(hashtab_t users_tab, yyerror2("user %s has inconsistent bounds %s/%s", user_id, bounds_id, policydbp->p_role_val_to_name[user->bounds - 1]); + free(bounds_id); return -1; } free(bounds_id); From 7f429a8410ecfd004f4ccc6e97f139abb8ddef7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Mon, 22 Jan 2024 14:24:24 +0100 Subject: [PATCH 14/15] checkpolicy: avoid assigning garbage values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Only assign the computed value on success, since it is not set by declare_symbol() on failure. Reported by GCC: module_compiler.c: In function 'create_role': module_compiler.c:287:24: warning: use of uninitialized value 'value' [CWE-457] [-Wanalyzer-use-of-uninitialized-value] 287 | datum->s.value = value; | ~~~~~~~~~~~~~~~^~~~~~~ Signed-off-by: Christian Göttsche --- checkpolicy/module_compiler.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/checkpolicy/module_compiler.c b/checkpolicy/module_compiler.c index 464897cc8b..6ff91b8f94 100644 --- a/checkpolicy/module_compiler.c +++ b/checkpolicy/module_compiler.c @@ -284,9 +284,8 @@ static int create_role(uint32_t scope, unsigned char isattr, role_datum_t **role ret = require_symbol(SYM_ROLES, id, datum, &value, &value); } - datum->s.value = value; - if (ret == 0) { + datum->s.value = value; *role = datum; *key = strdup(id); if (*key == NULL) { @@ -303,6 +302,7 @@ static int create_role(uint32_t scope, unsigned char isattr, role_datum_t **role free(datum); return -1; } + datum->s.value = value; *role = datum; *key = id; } else { @@ -529,9 +529,8 @@ static int create_user(uint32_t scope, user_datum_t **user, char **key) ret = require_symbol(SYM_USERS, id, datum, &value, &value); } - datum->s.value = value; - if (ret == 0) { + datum->s.value = value; *user = datum; *key = strdup(id); if (*key == NULL) { @@ -539,6 +538,7 @@ static int create_user(uint32_t scope, user_datum_t **user, char **key) return -1; } } else if (ret == 1) { + datum->s.value = value; *user = datum; *key = id; } else { From d4bb604a6d1edec451e7912e64e1bb08686ceeaa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Mon, 22 Jan 2024 14:24:28 +0100 Subject: [PATCH 15/15] checkpolicy: misc policy_define.c cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sync function parameter names. Drop superfluous return value. The function avrule_merge_ioctls() has no failure conditions and always returns 0. Drop duplicate include. Use native type for ranges. Signed-off-by: Christian Göttsche --- checkpolicy/policy_define.c | 27 ++++++++++++--------------- checkpolicy/policy_define.h | 2 +- 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c index cd49cae37a..79d67a7895 100644 --- a/checkpolicy/policy_define.c +++ b/checkpolicy/policy_define.c @@ -44,7 +44,6 @@ #define IPPROTO_SCTP 132 #endif #include -#include #include #include #include @@ -1096,7 +1095,7 @@ int define_level(void) while ((id = queue_remove(id_queue))) { cat_datum_t *cdatum; - int range_start, range_end, i; + uint32_t range_start, range_end, i; if (id_has_dot(id)) { char *id_start = id; @@ -1932,7 +1931,7 @@ static int avrule_sort_ioctls(struct av_ioctl_range_list **rangehead) return -1; } -static int avrule_merge_ioctls(struct av_ioctl_range_list **rangehead) +static void avrule_merge_ioctls(struct av_ioctl_range_list **rangehead) { struct av_ioctl_range_list *r, *tmp; r = *rangehead; @@ -1949,7 +1948,6 @@ static int avrule_merge_ioctls(struct av_ioctl_range_list **rangehead) } r = r->next; } - return 0; } static int avrule_read_ioctls(struct av_ioctl_range_list **rangehead) @@ -2070,8 +2068,7 @@ static int avrule_ioctl_ranges(struct av_ioctl_range_list **rangelist) /* sort and merge the input ioctls */ if (avrule_sort_ioctls(&rangehead)) return -1; - if (avrule_merge_ioctls(&rangehead)) - return -1; + avrule_merge_ioctls(&rangehead); /* flip ranges if these are omitted */ if (omit) { if (avrule_omit_ioctls(&rangehead)) @@ -3854,7 +3851,7 @@ uintptr_t define_cexpr(uint32_t expr_type, uintptr_t arg1, uintptr_t arg2) return 0; } -int define_conditional(cond_expr_t * expr, avrule_t * t, avrule_t * f) +int define_conditional(cond_expr_t * expr, avrule_t * t_list, avrule_t * f_list) { cond_expr_t *e; int depth, booleans, tunables; @@ -3866,15 +3863,15 @@ int define_conditional(cond_expr_t * expr, avrule_t * t, avrule_t * f) yyerror("illegal conditional expression"); return -1; } - if (!t) { - if (!f) { + if (!t_list) { + if (!f_list) { /* empty is fine, destroy expression and return */ cond_expr_destroy(expr); return 0; } /* Invert */ - t = f; - f = 0; + t_list = f_list; + f_list = NULL; expr = define_cond_expr(COND_NOT, expr, 0); if (!expr) { yyerror("unable to invert conditional expression"); @@ -3940,8 +3937,8 @@ int define_conditional(cond_expr_t * expr, avrule_t * t, avrule_t * f) /* use tmp conditional node to partially build new node */ memset(&cn, 0, sizeof(cn)); cn.expr = expr; - cn.avtrue_list = t; - cn.avfalse_list = f; + cn.avtrue_list = t_list; + cn.avfalse_list = f_list; /* normalize/precompute expression */ if (cond_normalize_expr(policydbp, &cn) < 0) { @@ -4117,7 +4114,7 @@ static int set_user_roles(role_set_t * set, char *id) static int parse_categories(char *id, level_datum_t * levdatum, ebitmap_t * cats) { cat_datum_t *cdatum; - int range_start, range_end, i; + uint32_t range_start, range_end, i; if (id_has_dot(id)) { char *id_start = id; @@ -5527,7 +5524,7 @@ static int define_genfs_context_helper(char *fstype, int has_type) class_datum_t *cladatum; char *type = NULL; const char *sclass; - int len, len2; + size_t len, len2; if (policydbp->target_platform != SEPOL_TARGET_SELINUX) { yyerror("genfs not supported for target"); diff --git a/checkpolicy/policy_define.h b/checkpolicy/policy_define.h index 075b048dbf..bcbfe4f332 100644 --- a/checkpolicy/policy_define.h +++ b/checkpolicy/policy_define.h @@ -13,7 +13,7 @@ #define FALSE 0 avrule_t *define_cond_compute_type(int which); -avrule_t *define_cond_pol_list(avrule_t *avlist, avrule_t *stmt); +avrule_t *define_cond_pol_list(avrule_t *avlist, avrule_t *sl); avrule_t *define_cond_te_avtab(int which); avrule_t *define_cond_filename_trans(void); cond_expr_t *define_cond_expr(uint32_t expr_type, void *arg1, void* arg2);