diff mbox series

[RFC,v2,30/36] libsepol: validate ocontexts

Message ID 20211105154542.38434-31-cgzones@googlemail.com (mailing list archive)
State Superseded
Headers show
Series libsepol: add fuzzer for reading binary policies | expand

Commit Message

Christian Göttsche Nov. 5, 2021, 3:45 p.m. UTC
Check the literal contexts in ocontext statements are defined.

    ==91274==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f60b0afe8c6 bp 0x7ffd42edc990 sp 0x7ffd42edc148 T0)
    ==91274==The signal is caused by a READ memory access.
    ==91274==Hint: address points to the zero page.
        #0 0x7f60b0afe8c6  string/../sysdeps/x86_64/multiarch/../strlen.S:120
        #1 0x4bd128 in __interceptor_strlen (./out/binpolicy-fuzzer+0x4bd128)
        #2 0x5eb387 in create_str_helper ./libsepol/src/kernel_to_common.c:69:10
        #3 0x5eb11e in create_str ./libsepol/src/kernel_to_common.c:99:8
        #4 0x56ad7b in context_to_str ./libsepol/src/kernel_to_conf.c:2408:9
        #5 0x56a717 in write_sid_context_rules_to_conf ./libsepol/src/kernel_to_conf.c:2441:9
        #6 0x55b26c in write_selinux_isid_rules_to_conf ./libsepol/src/kernel_to_conf.c:2476:9
        #7 0x55b26c in sepol_kernel_policydb_to_conf ./libsepol/src/kernel_to_conf.c:3206:8
        #8 0x55a34f in LLVMFuzzerTestOneInput ./libsepol/fuzz/binpolicy-fuzzer.c:38:9
        #9 0x45aed3 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) fuzzer.o
        #10 0x446a12 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) fuzzer.o
        #11 0x44c93b in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) fuzzer.o
        #12 0x475dd2 in main (./out/binpolicy-fuzzer+0x475dd2)
        #13 0x7f60b0a887ec in __libc_start_main csu/../csu/libc-start.c:332:16
        #14 0x423689 in _start (./out/binpolicy-fuzzer+0x423689)

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>

---
v2:
   also check in base modules

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 libsepol/src/policydb_validate.c | 44 ++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

Comments

James Carter Nov. 9, 2021, 7:04 p.m. UTC | #1
On Fri, Nov 5, 2021 at 12:11 PM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> Check the literal contexts in ocontext statements are defined.
>
>     ==91274==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f60b0afe8c6 bp 0x7ffd42edc990 sp 0x7ffd42edc148 T0)
>     ==91274==The signal is caused by a READ memory access.
>     ==91274==Hint: address points to the zero page.
>         #0 0x7f60b0afe8c6  string/../sysdeps/x86_64/multiarch/../strlen.S:120
>         #1 0x4bd128 in __interceptor_strlen (./out/binpolicy-fuzzer+0x4bd128)
>         #2 0x5eb387 in create_str_helper ./libsepol/src/kernel_to_common.c:69:10
>         #3 0x5eb11e in create_str ./libsepol/src/kernel_to_common.c:99:8
>         #4 0x56ad7b in context_to_str ./libsepol/src/kernel_to_conf.c:2408:9
>         #5 0x56a717 in write_sid_context_rules_to_conf ./libsepol/src/kernel_to_conf.c:2441:9
>         #6 0x55b26c in write_selinux_isid_rules_to_conf ./libsepol/src/kernel_to_conf.c:2476:9
>         #7 0x55b26c in sepol_kernel_policydb_to_conf ./libsepol/src/kernel_to_conf.c:3206:8
>         #8 0x55a34f in LLVMFuzzerTestOneInput ./libsepol/fuzz/binpolicy-fuzzer.c:38:9
>         #9 0x45aed3 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) fuzzer.o
>         #10 0x446a12 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) fuzzer.o
>         #11 0x44c93b in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) fuzzer.o
>         #12 0x475dd2 in main (./out/binpolicy-fuzzer+0x475dd2)
>         #13 0x7f60b0a887ec in __libc_start_main csu/../csu/libc-start.c:332:16
>         #14 0x423689 in _start (./out/binpolicy-fuzzer+0x423689)
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
>
> ---
> v2:
>    also check in base modules
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
>  libsepol/src/policydb_validate.c | 44 ++++++++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
>
> diff --git a/libsepol/src/policydb_validate.c b/libsepol/src/policydb_validate.c
> index 57eb2550..96f133c9 100644
> --- a/libsepol/src/policydb_validate.c
> +++ b/libsepol/src/policydb_validate.c
> @@ -736,6 +736,47 @@ static int validate_filename_trans_hashtab(sepol_handle_t *handle, hashtab_t fil
>         return 0;
>  }
>
> +static int validate_context(context_struct_t *con, validate_t flavors[], int mls)
> +{
> +       if (validate_value(con->user, &flavors[SYM_USERS]))
> +               return -1;
> +       if (validate_value(con->role, &flavors[SYM_ROLES]))
> +               return -1;
> +       if (validate_value(con->type, &flavors[SYM_TYPES]))
> +               return -1;
> +       if (mls && validate_mls_range(&con->range, &flavors[SYM_LEVELS], &flavors[SYM_CATS]))
> +               return -1;
> +
> +       return 0;
> +}
> +
> +static int validate_ocontexts(sepol_handle_t *handle, policydb_t *p, validate_t flavors[])
> +{
> +       ocontext_t *octx;
> +       unsigned int i;
> +
> +       for (i = 0; i < OCON_NUM; i++) {
> +               for (octx = p->ocontexts[i]; octx; octx = octx->next) {
> +                       if (validate_context(&octx->context[0], flavors, p->mls))
> +                               goto bad;
> +
> +                       switch (i) {
> +                       case OCON_FS:
> +                       case OCON_NETIF:
> +                               if (validate_context(&octx->context[1], flavors, p->mls))
> +                                       goto bad;
> +                               break;

Should use a default case.

The bigger issue, which I didn't catch the first time, is that this
doesn't work for a XEN policy.
In policydb.c there is an ocontext_read_xen() and an
ocontext_read_selinux(), which is called depends on whether
p->target_platform is SEPOL_TARGET_SELINUX or SEPOL_TARGET_XEN.

Jim


> +                       }
> +               }
> +       }
> +
> +       return 0;
> +
> +bad:
> +       ERR(handle, "Invalid ocontext");
> +       return -1;
> +}
> +
>  /*
>   * Functions to validate a module policydb
>   */
> @@ -936,6 +977,9 @@ int validate_policydb(sepol_handle_t *handle, policydb_t *p)
>                         goto bad;
>         }
>
> +       if (validate_ocontexts(handle, p, flavors))
> +               goto bad;
> +
>         if (validate_scopes(handle, p->scope, p->global))
>                 goto bad;
>
> --
> 2.33.1
>
diff mbox series

Patch

diff --git a/libsepol/src/policydb_validate.c b/libsepol/src/policydb_validate.c
index 57eb2550..96f133c9 100644
--- a/libsepol/src/policydb_validate.c
+++ b/libsepol/src/policydb_validate.c
@@ -736,6 +736,47 @@  static int validate_filename_trans_hashtab(sepol_handle_t *handle, hashtab_t fil
 	return 0;
 }
 
+static int validate_context(context_struct_t *con, validate_t flavors[], int mls)
+{
+	if (validate_value(con->user, &flavors[SYM_USERS]))
+		return -1;
+	if (validate_value(con->role, &flavors[SYM_ROLES]))
+		return -1;
+	if (validate_value(con->type, &flavors[SYM_TYPES]))
+		return -1;
+	if (mls && validate_mls_range(&con->range, &flavors[SYM_LEVELS], &flavors[SYM_CATS]))
+		return -1;
+
+	return 0;
+}
+
+static int validate_ocontexts(sepol_handle_t *handle, policydb_t *p, validate_t flavors[])
+{
+	ocontext_t *octx;
+	unsigned int i;
+
+	for (i = 0; i < OCON_NUM; i++) {
+		for (octx = p->ocontexts[i]; octx; octx = octx->next) {
+			if (validate_context(&octx->context[0], flavors, p->mls))
+				goto bad;
+
+			switch (i) {
+			case OCON_FS:
+			case OCON_NETIF:
+				if (validate_context(&octx->context[1], flavors, p->mls))
+					goto bad;
+				break;
+			}
+		}
+	}
+
+	return 0;
+
+bad:
+	ERR(handle, "Invalid ocontext");
+	return -1;
+}
+
 /*
  * Functions to validate a module policydb
  */
@@ -936,6 +977,9 @@  int validate_policydb(sepol_handle_t *handle, policydb_t *p)
 			goto bad;
 	}
 
+	if (validate_ocontexts(handle, p, flavors))
+		goto bad;
+
 	if (validate_scopes(handle, p->scope, p->global))
 		goto bad;