| Message ID | 20201230100746.2549568-1-nicolas.iooss@m4x.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/6] libsepol: do not decode out-of-bound rolebounds | expand |
On Wed, Dec 30, 2020 at 5:11 AM Nicolas Iooss <nicolas.iooss@m4x.org> wrote: > > While fuzzing /usr/libexec/hll/pp, a policy module was generated with a > role->bounds larger that the number of roles in the policy. > > This issue has been found while fuzzing hll/pp with the American Fuzzy > Lop. > > Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org> > --- > libsepol/src/module_to_cil.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/libsepol/src/module_to_cil.c b/libsepol/src/module_to_cil.c > index a87bc15e7610..c99790eb76e7 100644 > --- a/libsepol/src/module_to_cil.c > +++ b/libsepol/src/module_to_cil.c > @@ -2165,7 +2165,9 @@ static int role_to_cil(int indent, struct policydb *pdb, struct avrule_block *UN > } > } > > - if (role->bounds > 0) { > + if (role->bounds >= pdb->p_roles.nprim) { > + log_err("Warning: role %s defines an out-of-bound rolebounds", key); > + } else if (role->bounds > 0) { > cil_println(indent, "(rolebounds %s %s)", key, pdb->p_role_val_to_name[role->bounds - 1]); > } > break; > -- > 2.29.2 > There are other places where the bounds value is used as an index and type datums also have bounds that are used in the same way. Correct me if I am wrong, but I think that this can only occur by crafting a binary (and not as a result of a policy being compiled). So I think the correct place for the check would be when the binary file is read. I'll have to test to be sure, but I think the attached patch might do the proper checking. Jim
On Mon, Jan 4, 2021 at 10:48 AM James Carter <jwcart2@gmail.com> wrote: > > On Wed, Dec 30, 2020 at 5:11 AM Nicolas Iooss <nicolas.iooss@m4x.org> wrote: > > > > While fuzzing /usr/libexec/hll/pp, a policy module was generated with a > > role->bounds larger that the number of roles in the policy. > > > > This issue has been found while fuzzing hll/pp with the American Fuzzy > > Lop. > > > > Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org> > > --- > > libsepol/src/module_to_cil.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/libsepol/src/module_to_cil.c b/libsepol/src/module_to_cil.c > > index a87bc15e7610..c99790eb76e7 100644 > > --- a/libsepol/src/module_to_cil.c > > +++ b/libsepol/src/module_to_cil.c > > @@ -2165,7 +2165,9 @@ static int role_to_cil(int indent, struct policydb *pdb, struct avrule_block *UN > > } > > } > > > > - if (role->bounds > 0) { > > + if (role->bounds >= pdb->p_roles.nprim) { > > + log_err("Warning: role %s defines an out-of-bound rolebounds", key); > > + } else if (role->bounds > 0) { > > cil_println(indent, "(rolebounds %s %s)", key, pdb->p_role_val_to_name[role->bounds - 1]); > > } > > break; > > -- > > 2.29.2 > > > > There are other places where the bounds value is used as an index and > type datums also have bounds that are used in the same way. > > Correct me if I am wrong, but I think that this can only occur by > crafting a binary (and not as a result of a policy being compiled). So > I think the correct place for the check would be when the binary file > is read. > > I'll have to test to be sure, but I think the attached patch might do > the proper checking. > Oops, that patch had typos. This one. > Jim
On Mon, Jan 4, 2021 at 4:51 PM James Carter <jwcart2@gmail.com> wrote: > > On Mon, Jan 4, 2021 at 10:48 AM James Carter <jwcart2@gmail.com> wrote: > > > > On Wed, Dec 30, 2020 at 5:11 AM Nicolas Iooss <nicolas.iooss@m4x.org> wrote: > > > > > > While fuzzing /usr/libexec/hll/pp, a policy module was generated with a > > > role->bounds larger that the number of roles in the policy. > > > > > > This issue has been found while fuzzing hll/pp with the American Fuzzy > > > Lop. > > > > > > Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org> > > > --- > > > libsepol/src/module_to_cil.c | 4 +++- > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > diff --git a/libsepol/src/module_to_cil.c b/libsepol/src/module_to_cil.c > > > index a87bc15e7610..c99790eb76e7 100644 > > > --- a/libsepol/src/module_to_cil.c > > > +++ b/libsepol/src/module_to_cil.c > > > @@ -2165,7 +2165,9 @@ static int role_to_cil(int indent, struct policydb *pdb, struct avrule_block *UN > > > } > > > } > > > > > > - if (role->bounds > 0) { > > > + if (role->bounds >= pdb->p_roles.nprim) { > > > + log_err("Warning: role %s defines an out-of-bound rolebounds", key); > > > + } else if (role->bounds > 0) { > > > cil_println(indent, "(rolebounds %s %s)", key, pdb->p_role_val_to_name[role->bounds - 1]); > > > } > > > break; > > > -- > > > 2.29.2 > > > > > > > There are other places where the bounds value is used as an index and > > type datums also have bounds that are used in the same way. > > > > Correct me if I am wrong, but I think that this can only occur by > > crafting a binary (and not as a result of a policy being compiled). So > > I think the correct place for the check would be when the binary file > > is read. > > > > I'll have to test to be sure, but I think the attached patch might do > > the proper checking. > > > > Oops, that patch had typos. This one. I agree, and I confirm your patch fixed the crash I experienced. Thanks! Nicolas
diff --git a/libsepol/src/module_to_cil.c b/libsepol/src/module_to_cil.c index a87bc15e7610..c99790eb76e7 100644 --- a/libsepol/src/module_to_cil.c +++ b/libsepol/src/module_to_cil.c @@ -2165,7 +2165,9 @@ static int role_to_cil(int indent, struct policydb *pdb, struct avrule_block *UN } } - if (role->bounds > 0) { + if (role->bounds >= pdb->p_roles.nprim) { + log_err("Warning: role %s defines an out-of-bound rolebounds", key); + } else if (role->bounds > 0) { cil_println(indent, "(rolebounds %s %s)", key, pdb->p_role_val_to_name[role->bounds - 1]); } break;
While fuzzing /usr/libexec/hll/pp, a policy module was generated with a role->bounds larger that the number of roles in the policy. This issue has been found while fuzzing hll/pp with the American Fuzzy Lop. Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org> --- libsepol/src/module_to_cil.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)