Skip to content

Commit

Permalink
libsepol: check scope index bitmap bounds
Browse files Browse the repository at this point in the history
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
crashed with a segmentation fault before commit 02a7d77 ("libsepol:
make parsing symbol table headers more robust").

More precisely, the segmentation fault occurred in
required_scopes_to_cil() (in module_to_cil.c):

    map = decl->required.scope[sym];
    ebitmap_for_each_bit(&map, node, i) {
        if (!ebitmap_get_bit(&map, i)) {
                continue;
        }
        key = pdb->sym_val_to_name[sym][i]; // the program crashed here

The segmentation fault occurred because pdb->sym_val_to_name[sym] was
NULL (it is not allocated when pdb->symtab[sym].nprim is 0). Anyway if
decl->required.scope[sym] bitmap is crafted so that a bit higher than
nprim is enabled, required_scopes_to_cil() still reads a value outside
of pdb->sym_val_to_name[sym].

Detect such a malformed policy file when loaded decl->required.scope in
scope_index_read().

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
  • Loading branch information
fishilico committed Jun 28, 2022
1 parent 3f6ba1e commit 8d942f5
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 4 deletions.
1 change: 1 addition & 0 deletions libsepol/include/sepol/policydb/ebitmap.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ extern int ebitmap_set_bit(ebitmap_t * e, unsigned int bit, int value);
extern unsigned int ebitmap_highest_set_bit(const ebitmap_t * e);
extern void ebitmap_destroy(ebitmap_t * e);
extern int ebitmap_read(ebitmap_t * e, void *fp);
extern int ebitmap_read_bounded(ebitmap_t * e, uint32_t bound, void *fp);

#ifdef __cplusplus
}
Expand Down
33 changes: 33 additions & 0 deletions libsepol/src/ebitmap.c
Original file line number Diff line number Diff line change
Expand Up @@ -493,4 +493,37 @@ int ebitmap_read(ebitmap_t * e, void *fp)
goto out;
}

int ebitmap_read_bounded(ebitmap_t * e, uint32_t bound, void *fp)
{
int rc;
struct ebitmap_node *n;
unsigned int bit;

rc = ebitmap_read(e, fp);
if (rc < 0)
return rc;

/* Go to the last node, to compare its bits with bound */
n = e->node;
if (!n)
return 0;
while (n->next) {
n = n->next;
}
if (n->startbit + MAPSIZE <= bound) {
/* All bits are below the bound. No overflow is possible */
return 0;
}
/* There may be bits enabled over the bound */
for (bit = 0; bit < MAPSIZE; bit++) {
if (n->startbit + bit >= bound && (n->map & (MAPBIT << bit ))) {
printf
("security: ebitmap: bitmap bit %u over the bound %u\n",
n->startbit + bit, bound);
ebitmap_destroy(e);
return -EINVAL;
}
}
return 0;
}
/* FLASK */
8 changes: 4 additions & 4 deletions libsepol/src/policydb.c
Original file line number Diff line number Diff line change
Expand Up @@ -3979,15 +3979,15 @@ static int range_trans_rule_read(range_trans_rule_t ** r,
return 0;
}

static int scope_index_read(scope_index_t * scope_index,
static int scope_index_read(policydb_t * p, scope_index_t * scope_index,
unsigned int num_scope_syms, struct policy_file *fp)
{
unsigned int i;
uint32_t buf[1];
int rc;

for (i = 0; i < num_scope_syms; i++) {
if (ebitmap_read(scope_index->scope + i, fp) < 0) {
if (ebitmap_read_bounded(scope_index->scope + i, p->symtab[i].nprim, fp) < 0) {
return -1;
}
}
Expand Down Expand Up @@ -4041,8 +4041,8 @@ static int avrule_decl_read(policydb_t * p, avrule_decl_t * decl,
range_trans_rule_read(&decl->range_tr_rules, fp) == -1) {
return -1;
}
if (scope_index_read(&decl->required, num_scope_syms, fp) == -1 ||
scope_index_read(&decl->declared, num_scope_syms, fp) == -1) {
if (scope_index_read(p, &decl->required, num_scope_syms, fp) == -1 ||
scope_index_read(p, &decl->declared, num_scope_syms, fp) == -1) {
return -1;
}

Expand Down

0 comments on commit 8d942f5

Please sign in to comment.