Message ID | 20201230100746.2549568-2-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:10 AM Nicolas Iooss <nicolas.iooss@m4x.org> wrote: > > While fuzzing /usr/libexec/hll/pp, a policy module was generated which > triggered a NULL result when doing: > > key = pdb->sym_val_to_name[sym][i]; > > Detect such unexpected behavior to exit with an error instead of > crashing. > > 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, 4 insertions(+) > > diff --git a/libsepol/src/module_to_cil.c b/libsepol/src/module_to_cil.c > index c99790eb76e7..99360a9afdd0 100644 > --- a/libsepol/src/module_to_cil.c > +++ b/libsepol/src/module_to_cil.c > @@ -3459,6 +3459,10 @@ static int required_scopes_to_cil(int indent, struct policydb *pdb, struct avrul > map = decl->required.scope[sym]; > ebitmap_for_each_positive_bit(&map, node, i) { > key = pdb->sym_val_to_name[sym][i]; > + if (key == NULL) { > + rc = -1; > + goto exit; > + } > > scope_datum = hashtab_search(pdb->scope[sym].table, key); > if (scope_datum == NULL) { > -- > 2.29.2 > This is a similar case as the previous patch. There are other places where a check could go, but the check really should happen when reading the binary policy. Jim
On Mon, Jan 4, 2021 at 5:31 PM James Carter <jwcart2@gmail.com> wrote: > > On Wed, Dec 30, 2020 at 5:10 AM Nicolas Iooss <nicolas.iooss@m4x.org> wrote: > > > > While fuzzing /usr/libexec/hll/pp, a policy module was generated which > > triggered a NULL result when doing: > > > > key = pdb->sym_val_to_name[sym][i]; > > > > Detect such unexpected behavior to exit with an error instead of > > crashing. > > > > 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, 4 insertions(+) > > > > diff --git a/libsepol/src/module_to_cil.c b/libsepol/src/module_to_cil.c > > index c99790eb76e7..99360a9afdd0 100644 > > --- a/libsepol/src/module_to_cil.c > > +++ b/libsepol/src/module_to_cil.c > > @@ -3459,6 +3459,10 @@ static int required_scopes_to_cil(int indent, struct policydb *pdb, struct avrul > > map = decl->required.scope[sym]; > > ebitmap_for_each_positive_bit(&map, node, i) { > > key = pdb->sym_val_to_name[sym][i]; > > + if (key == NULL) { > > + rc = -1; > > + goto exit; > > + } > > > > scope_datum = hashtab_search(pdb->scope[sym].table, key); > > if (scope_datum == NULL) { > > -- > > 2.29.2 > > > > This is a similar case as the previous patch. There are other places > where a check could go, but the check really should happen when > reading the binary policy. I agree, thanks for your review! I have sent a new patch which introduces a check in policydb_read (https://lore.kernel.org/selinux/20210106080836.344857-1-nicolas.iooss@m4x.org/T/). Nicolas
diff --git a/libsepol/src/module_to_cil.c b/libsepol/src/module_to_cil.c index c99790eb76e7..99360a9afdd0 100644 --- a/libsepol/src/module_to_cil.c +++ b/libsepol/src/module_to_cil.c @@ -3459,6 +3459,10 @@ static int required_scopes_to_cil(int indent, struct policydb *pdb, struct avrul map = decl->required.scope[sym]; ebitmap_for_each_positive_bit(&map, node, i) { key = pdb->sym_val_to_name[sym][i]; + if (key == NULL) { + rc = -1; + goto exit; + } scope_datum = hashtab_search(pdb->scope[sym].table, key); if (scope_datum == NULL) {
While fuzzing /usr/libexec/hll/pp, a policy module was generated which triggered a NULL result when doing: key = pdb->sym_val_to_name[sym][i]; Detect such unexpected behavior to exit with an error instead of crashing. 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, 4 insertions(+)