* [PATCH 1/5] checkpolicy/fuzz: drop redundant notdefined check
@ 2024-03-11 14:57 Christian Göttsche
2024-03-11 14:57 ` [PATCH 2/5] checkpolicy: clone level only once Christian Göttsche
` (4 more replies)
0 siblings, 5 replies; 7+ messages in thread
From: Christian Göttsche @ 2024-03-11 14:57 UTC (permalink / raw
To: selinux
The level_datum_t member notdefined is checked to be 1 during validation
and the fuzzer calls policydb_validate().
Drop the redundant check (as announced in the TODO).
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
checkpolicy/fuzz/checkpolicy-fuzzer.c | 17 -----------------
1 file changed, 17 deletions(-)
diff --git a/checkpolicy/fuzz/checkpolicy-fuzzer.c b/checkpolicy/fuzz/checkpolicy-fuzzer.c
index a3da0b57..f3a17cce 100644
--- a/checkpolicy/fuzz/checkpolicy-fuzzer.c
+++ b/checkpolicy/fuzz/checkpolicy-fuzzer.c
@@ -130,21 +130,6 @@ static int read_source_policy(policydb_t *p, const uint8_t *data, size_t size)
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->notdefined) {
- 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;
@@ -198,8 +183,6 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
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;
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/5] checkpolicy: clone level only once
2024-03-11 14:57 [PATCH 1/5] checkpolicy/fuzz: drop redundant notdefined check Christian Göttsche
@ 2024-03-11 14:57 ` Christian Göttsche
2024-03-11 14:57 ` [PATCH 3/5] checkpolicy: return YYerror on invalid character Christian Göttsche
` (3 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Christian Göttsche @ 2024-03-11 14:57 UTC (permalink / raw
To: selinux
In case of aliases clone the level only once to avoid leaking the fist
one.
Example policy:
class p sid h class p{d}sensitivity d alias s0;dominance{s0}level d;level s0;
Reported-by: oss-fuzz (issue #67308)
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
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 614b7706..0cf938ea 100644
--- a/checkpolicy/policy_define.c
+++ b/checkpolicy/policy_define.c
@@ -1038,7 +1038,7 @@ static int clone_level(hashtab_key_t key __attribute__ ((unused)), hashtab_datum
level_datum_t *levdatum = (level_datum_t *) datum;
mls_level_t *level = (mls_level_t *) arg, *newlevel;
- if (levdatum->level == level) {
+ if (levdatum->notdefined && levdatum->level == level) {
if (!levdatum->isalias) {
levdatum->notdefined = FALSE;
return 0;
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/5] checkpolicy: return YYerror on invalid character
2024-03-11 14:57 [PATCH 1/5] checkpolicy/fuzz: drop redundant notdefined check Christian Göttsche
2024-03-11 14:57 ` [PATCH 2/5] checkpolicy: clone level only once Christian Göttsche
@ 2024-03-11 14:57 ` Christian Göttsche
2024-03-11 14:57 ` [PATCH 4/5] libsepol: reject MLS support in pre-MLS policies Christian Göttsche
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Christian Göttsche @ 2024-03-11 14:57 UTC (permalink / raw
To: selinux
Inform bison about an invalid character by returning YYerror, so the
parser can cleanup internal state and return the failure via yyparse().
Currently the error is only observable via the global variable
policydb_errors, which needs to be checked separately.
Reported-by: oss-fuzz (issue #67270)
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
Should also fix issue #67327 (leak) due to the now performed cleanup.
Also fixes issue #67272 for me, but this one might resurface.
---
checkpolicy/policy_scan.l | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/checkpolicy/policy_scan.l b/checkpolicy/policy_scan.l
index 19c05a58..1926129c 100644
--- a/checkpolicy/policy_scan.l
+++ b/checkpolicy/policy_scan.l
@@ -308,7 +308,7 @@ GLBLUB { return(GLBLUB); }
"]" |
"~" |
"*" { return(yytext[0]); }
-. { yyerror("unrecognized character");}
+. { yyerror("unrecognized character"); return YYerror; }
%%
int yyerror(const char *msg)
{
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 4/5] libsepol: reject MLS support in pre-MLS policies
2024-03-11 14:57 [PATCH 1/5] checkpolicy/fuzz: drop redundant notdefined check Christian Göttsche
2024-03-11 14:57 ` [PATCH 2/5] checkpolicy: clone level only once Christian Göttsche
2024-03-11 14:57 ` [PATCH 3/5] checkpolicy: return YYerror on invalid character Christian Göttsche
@ 2024-03-11 14:57 ` Christian Göttsche
2024-03-11 14:57 ` [PATCH 5/5] checkpolicy/fuzz: scan Xen policies Christian Göttsche
2024-03-14 13:19 ` [PATCH 1/5] checkpolicy/fuzz: drop redundant notdefined check James Carter
4 siblings, 0 replies; 7+ messages in thread
From: Christian Göttsche @ 2024-03-11 14:57 UTC (permalink / raw
To: selinux
If MLS support is enabled check the policy version supports MLS.
Reported-by: oss-fuzz (issue #67322)
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
libsepol/src/policydb_validate.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/libsepol/src/policydb_validate.c b/libsepol/src/policydb_validate.c
index 6e46f426..e987d8da 100644
--- a/libsepol/src/policydb_validate.c
+++ b/libsepol/src/policydb_validate.c
@@ -1554,11 +1554,15 @@ static int validate_properties(sepol_handle_t *handle, const policydb_t *p)
case POLICY_KERN:
if (p->policyvers < POLICYDB_VERSION_MIN || p->policyvers > POLICYDB_VERSION_MAX)
goto bad;
+ if (p->mls && p->policyvers < POLICYDB_VERSION_MLS)
+ goto bad;
break;
case POLICY_BASE:
case POLICY_MOD:
if (p->policyvers < MOD_POLICYDB_VERSION_MIN || p->policyvers > MOD_POLICYDB_VERSION_MAX)
goto bad;
+ if (p->mls && p->policyvers < MOD_POLICYDB_VERSION_MLS)
+ goto bad;
break;
default:
goto bad;
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 5/5] checkpolicy/fuzz: scan Xen policies
2024-03-11 14:57 [PATCH 1/5] checkpolicy/fuzz: drop redundant notdefined check Christian Göttsche
` (2 preceding siblings ...)
2024-03-11 14:57 ` [PATCH 4/5] libsepol: reject MLS support in pre-MLS policies Christian Göttsche
@ 2024-03-11 14:57 ` Christian Göttsche
2024-03-14 13:19 ` [PATCH 1/5] checkpolicy/fuzz: drop redundant notdefined check James Carter
4 siblings, 0 replies; 7+ messages in thread
From: Christian Göttsche @ 2024-03-11 14:57 UTC (permalink / raw
To: selinux
In addition to standard SELinux platform policies also check Xen ones.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
Note: this will break all current reproducers and corpuses due to the
changed input format.
---
checkpolicy/fuzz/checkpolicy-fuzzer.c | 43 ++++++++++++++++++---------
1 file changed, 29 insertions(+), 14 deletions(-)
diff --git a/checkpolicy/fuzz/checkpolicy-fuzzer.c b/checkpolicy/fuzz/checkpolicy-fuzzer.c
index f3a17cce..ab1a6bb8 100644
--- a/checkpolicy/fuzz/checkpolicy-fuzzer.c
+++ b/checkpolicy/fuzz/checkpolicy-fuzzer.c
@@ -147,15 +147,28 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
policydb_t *finalpolicydb;
sidtab_t sidtab = {};
FILE *devnull = NULL;
- int mls, policyvers;
+ int mls, platform, policyvers;
sepol_debug(VERBOSE);
- /* Take the first byte whether to parse as MLS policy
- * and the second byte as policy version. */
- if (size < 2)
+ /*
+ * Take the first byte whether to generate a SELinux or Xen policy,
+ * the second byte whether to parse as MLS policy,
+ * and the second byte as policy version.
+ */
+ if (size < 3)
return 0;
switch (data[0]) {
+ case 'S':
+ platform = SEPOL_TARGET_SELINUX;
+ break;
+ case 'X':
+ platform = SEPOL_TARGET_XEN;
+ break;
+ default:
+ return 0;
+ }
+ switch (data[1]) {
case '0':
mls = 0;
break;
@@ -166,11 +179,11 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
return 0;
}
static_assert(0x7F - 'A' >= POLICYDB_VERSION_MAX, "Max policy version should be representable");
- policyvers = data[1] - 'A';
+ policyvers = data[2] - 'A';
if (policyvers < POLICYDB_VERSION_MIN || policyvers > POLICYDB_VERSION_MAX)
return 0;
- data += 2;
- size -= 2;
+ data += 3;
+ size -= 3;
if (policydb_init(&parsepolicydb))
goto exit;
@@ -178,7 +191,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
parsepolicydb.policy_type = POLICY_BASE;
parsepolicydb.mls = mls;
parsepolicydb.handle_unknown = DENY_UNKNOWN;
- policydb_set_target_platform(&parsepolicydb, SEPOL_TARGET_SELINUX);
+ policydb_set_target_platform(&parsepolicydb, platform);
if (read_source_policy(&parsepolicydb, data, size))
goto exit;
@@ -198,15 +211,17 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
kernpolicydb.policyvers = policyvers;
- assert(kernpolicydb.policy_type == POLICY_KERN);
- assert(kernpolicydb.handle_unknown == SEPOL_DENY_UNKNOWN);
- assert(kernpolicydb.mls == mls);
+ assert(kernpolicydb.policy_type == POLICY_KERN);
+ assert(kernpolicydb.handle_unknown == SEPOL_DENY_UNKNOWN);
+ assert(kernpolicydb.mls == mls);
+ assert(kernpolicydb.target_platform == platform);
finalpolicydb = &kernpolicydb;
} else {
- assert(parsepolicydb.policy_type == POLICY_MOD);
- assert(parsepolicydb.handle_unknown == SEPOL_DENY_UNKNOWN);
- assert(parsepolicydb.mls == mls);
+ assert(parsepolicydb.policy_type == POLICY_MOD);
+ assert(parsepolicydb.handle_unknown == SEPOL_DENY_UNKNOWN);
+ assert(parsepolicydb.mls == mls);
+ assert(parsepolicydb.target_platform == platform);
finalpolicydb = &parsepolicydb;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/5] checkpolicy/fuzz: drop redundant notdefined check
2024-03-11 14:57 [PATCH 1/5] checkpolicy/fuzz: drop redundant notdefined check Christian Göttsche
` (3 preceding siblings ...)
2024-03-11 14:57 ` [PATCH 5/5] checkpolicy/fuzz: scan Xen policies Christian Göttsche
@ 2024-03-14 13:19 ` James Carter
2024-03-20 20:05 ` James Carter
4 siblings, 1 reply; 7+ messages in thread
From: James Carter @ 2024-03-14 13:19 UTC (permalink / raw
To: Christian Göttsche; +Cc: selinux
On Mon, Mar 11, 2024 at 10:59 AM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> The level_datum_t member notdefined is checked to be 1 during validation
> and the fuzzer calls policydb_validate().
> Drop the redundant check (as announced in the TODO).
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
For these 5 patches:
Acked-by: James Carter <jwcart2@gmail.com>
> ---
> checkpolicy/fuzz/checkpolicy-fuzzer.c | 17 -----------------
> 1 file changed, 17 deletions(-)
>
> diff --git a/checkpolicy/fuzz/checkpolicy-fuzzer.c b/checkpolicy/fuzz/checkpolicy-fuzzer.c
> index a3da0b57..f3a17cce 100644
> --- a/checkpolicy/fuzz/checkpolicy-fuzzer.c
> +++ b/checkpolicy/fuzz/checkpolicy-fuzzer.c
> @@ -130,21 +130,6 @@ static int read_source_policy(policydb_t *p, const uint8_t *data, size_t size)
> 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->notdefined) {
> - 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;
> @@ -198,8 +183,6 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
> 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;
> --
> 2.43.0
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/5] checkpolicy/fuzz: drop redundant notdefined check
2024-03-14 13:19 ` [PATCH 1/5] checkpolicy/fuzz: drop redundant notdefined check James Carter
@ 2024-03-20 20:05 ` James Carter
0 siblings, 0 replies; 7+ messages in thread
From: James Carter @ 2024-03-20 20:05 UTC (permalink / raw
To: Christian Göttsche; +Cc: selinux
On Thu, Mar 14, 2024 at 9:19 AM James Carter <jwcart2@gmail.com> wrote:
>
> On Mon, Mar 11, 2024 at 10:59 AM Christian Göttsche
> <cgzones@googlemail.com> wrote:
> >
> > The level_datum_t member notdefined is checked to be 1 during validation
> > and the fuzzer calls policydb_validate().
> > Drop the redundant check (as announced in the TODO).
> >
> > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
>
> For these 5 patches:
> Acked-by: James Carter <jwcart2@gmail.com>
>
These 5 patches have been merged.
Thanks,
Jim
> > ---
> > checkpolicy/fuzz/checkpolicy-fuzzer.c | 17 -----------------
> > 1 file changed, 17 deletions(-)
> >
> > diff --git a/checkpolicy/fuzz/checkpolicy-fuzzer.c b/checkpolicy/fuzz/checkpolicy-fuzzer.c
> > index a3da0b57..f3a17cce 100644
> > --- a/checkpolicy/fuzz/checkpolicy-fuzzer.c
> > +++ b/checkpolicy/fuzz/checkpolicy-fuzzer.c
> > @@ -130,21 +130,6 @@ static int read_source_policy(policydb_t *p, const uint8_t *data, size_t size)
> > 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->notdefined) {
> > - 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;
> > @@ -198,8 +183,6 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
> > 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;
> > --
> > 2.43.0
> >
> >
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-03-20 20:05 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-11 14:57 [PATCH 1/5] checkpolicy/fuzz: drop redundant notdefined check Christian Göttsche
2024-03-11 14:57 ` [PATCH 2/5] checkpolicy: clone level only once Christian Göttsche
2024-03-11 14:57 ` [PATCH 3/5] checkpolicy: return YYerror on invalid character Christian Göttsche
2024-03-11 14:57 ` [PATCH 4/5] libsepol: reject MLS support in pre-MLS policies Christian Göttsche
2024-03-11 14:57 ` [PATCH 5/5] checkpolicy/fuzz: scan Xen policies Christian Göttsche
2024-03-14 13:19 ` [PATCH 1/5] checkpolicy/fuzz: drop redundant notdefined check James Carter
2024-03-20 20:05 ` James Carter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).