On Wed, Nov 16, 2016 at 4:51 PM, Stephen Smalley <sds@xxxxxxxxxxxxx> wrote:
On 11/16/2016 10:29 AM, Nicolas Iooss wrote:
> On Wed, Nov 16, 2016 at 3:12 PM, Stephen Smalley <sds@xxxxxxxxxxxxx
So, don't we need to also check that i < pdb->symtab[sym].nprim before> <mailto:sds@xxxxxxxxxxxxx>> wrote:
>
> On 11/15/2016 06:07 PM, Nicolas Iooss wrote:
> > When hll/pp loads a policy file which has been modified so that the
> > nprim field of one of its non-empty symbol table was changed to zero, it
> > crashes with a segmentation fault. A quick analysis leads to
> > "p->sym_val_to_name[i] = (char **)alloc(p->symtab[i].nprim, sizeof(char
> > *));" in policydb_index_others(), which is not executed when
> > p->symtab[i].nprim is zero even though there are items in
> > p->symtab[i].table.
> >
> > Detect such an oddity in the policy file early to exit with a clean
> > error message.
>
> I'll apply this but I'd like to know where the segmentation fault
> occurred. The index functions already check whether the value exceeds
> the nprim value, and therefore shouldn't try to set the _val_to_name[]
> entry.
>
>
> Thanks. The segfault occurs in required_scopes_to_cil()
> in module_to_cil.c. Here is a gdb session.
>
> % gdb-run ./DESTDIR/usr/libexec/selinux/hll/pp
> afl-out/pp/crashes.2016-11-15-16:49:07/id:000000,sig:11,*
>
> Reading symbols from ./DESTDIR/usr/libexec/selinux/hll/pp...done.
> Starting program: ./DESTDIR/usr/libexec/selinux/hll/pp
> afl-out/pp/crashes.2016-11-15-16:49:07/id:000000,sig:11,src: 000000,op:flip1,pos:109
>
> Program received signal SIGSEGV, Segmentation fault.
> required_scopes_to_cil (decl_stack=0x6030b0, block=0x606780,
> pdb=0x6032e0, indent=0) at module_to_cil.c:3476
> 3476 key = pdb->sym_val_to_name[sym][i];
> => 0x00007ffff7b6c708 <block_to_cil+1224>: 4a 8b 04 e8 mov
> (%rax,%r13,8),%rax
> (gdb) bt
> #0 required_scopes_to_cil (decl_stack=0x6030b0, block=0x606780,
> pdb=0x6032e0, indent=0) at module_to_cil.c:3476
> #1 block_to_cil (pdb=0x6032e0, block=0x606780, stack=0x6030b0,
> indent=0) at module_to_cil.c:3622
> #2 0x00007ffff7b6e0c6 in blocks_to_cil (pdb=0x6032e0) at
> module_to_cil.c:3760
> #3 sepol_module_policydb_to_cil (fp=fp@entry=0x7ffff7b405e0
> <_IO_2_1_stdout_>, pdb=0x6032e0, linked=linked@entry=0) at
> module_to_cil.c:4047
> #4 0x00007ffff7b6e3c6 in sepol_module_package_to_cil
> (fp=fp@entry=0x7ffff7b405e0 <_IO_2_1_stdout_>, mod_pkg=0x603280) at
> module_to_cil.c:4076
> #5 0x0000000000400e76 in main (argc=<optimized out>, argv=<optimized
> out>) at pp.c:150
> (gdb) list
> 3471 map = decl->required.scope[sym];
> 3472 ebitmap_for_each_bit(&map, node, i) {
> 3473 if (!ebitmap_get_bit(&map, i)) {
> 3474 continue;
> 3475 }
> 3476 key = pdb->sym_val_to_name[sym][i];
> 3477
> 3478 scope_datum =
> hashtab_search(pdb->scope[sym].table, key);
> 3479 for (j = 0; j <
> scope_datum->decl_ids_len; j++) {
> 3480 if (scope_datum->decl_ids[j] ==
> decl->decl_id) {
line 3476?
As i is an iterator on decl->required.scope[sym] bitmap, it makes more sense to compare the bitmap high bit value with pdb->symtab[sym].nprim when it is loaded in policydb.c. More precisely, from an API perspective, it makes sense for sepol_module_policydb_to_cil() to expect a well-formed policy database as input. I see this issue as the fact that policydb_read() can load an invalid policy file and still returns a successful return value.
Right now I have no time to investigate/write a patch, so I am sending to the mailing list the files I am using to reproduce the issue (basic.pp is the file I gave to the fuzzer and the other attachment is the invalid policy which made pp crash before the patch I sent yesterday).
Nicolas
Attachment:
basic.pp
Description: Binary data
Attachment:
id:000000,sig:11,src:000000,op:flip1,pos:109
Description: Binary data
_______________________________________________ Selinux mailing list Selinux@xxxxxxxxxxxxx To unsubscribe, send email to Selinux-leave@xxxxxxxxxxxxx. To get help, send an email containing "help" to Selinux-request@xxxxxxxxxxxxx.
