Skip to content

Commit

Permalink
libsepol: avoid fixed sized format buffer for xperms
Browse files Browse the repository at this point in the history
An extended access vector rule can consist of many individual ranges of
permissions.  Use a dynamically growing sized buffer for formatting such
rules instead of a static buffer to avoid write failures due to
truncations.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
  • Loading branch information
cgzones authored and jwcart2 committed Nov 21, 2023
1 parent 285d7cc commit fdb536f
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 26 deletions.
9 changes: 8 additions & 1 deletion checkpolicy/test/dismod.c
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,7 @@ static int display_avrule(avrule_t * avrule, policydb_t * policy,
display_id(policy, fp, SYM_TYPES, avrule->perms->data - 1, "");
} else if (avrule->specified & AVRULE_XPERMS) {
avtab_extended_perms_t xperms;
char *perms;
int i;

if (avrule->xperms->specified == AVRULE_XPERMS_IOCTLFUNCTION)
Expand All @@ -362,7 +363,13 @@ static int display_avrule(avrule_t * avrule, policydb_t * policy,
for (i = 0; i < EXTENDED_PERMS_LEN; i++)
xperms.perms[i] = avrule->xperms->perms[i];

fprintf(fp, "%s", sepol_extended_perms_to_string(&xperms));
perms = sepol_extended_perms_to_string(&xperms);
if (!perms) {
fprintf(fp, " ERROR: failed to format xperms\n");
return -1;
}
fprintf(fp, "%s", perms);
free(perms);
}

fprintf(fp, ";\n");
Expand Down
10 changes: 9 additions & 1 deletion checkpolicy/test/dispol.c
Original file line number Diff line number Diff line change
Expand Up @@ -196,14 +196,22 @@ static int render_av_rule(avtab_key_t * key, avtab_datum_t * datum, uint32_t wha
fprintf(fp, ";\n");
}
} else if (key->specified & AVTAB_XPERMS) {
char *perms;

if (key->specified & AVTAB_XPERMS_ALLOWED)
fprintf(fp, "allowxperm ");
else if (key->specified & AVTAB_XPERMS_AUDITALLOW)
fprintf(fp, "auditallowxperm ");
else if (key->specified & AVTAB_XPERMS_DONTAUDIT)
fprintf(fp, "dontauditxperm ");
render_key(key, p, fp);
fprintf(fp, "%s;\n", sepol_extended_perms_to_string(datum->xperms));
perms = sepol_extended_perms_to_string(datum->xperms);
if (!perms) {
fprintf(fp, " ERROR: failed to format xperms\n");
return -1;
}
fprintf(fp, "%s;\n", perms);
free(perms);
} else {
fprintf(fp, " ERROR: no valid rule type specified\n");
return -1;
Expand Down
7 changes: 6 additions & 1 deletion libsepol/src/assertion.c
Original file line number Diff line number Diff line change
Expand Up @@ -178,15 +178,20 @@ static int report_assertion_extended_permissions(sepol_handle_t *handle,
rc = check_extended_permissions(avrule->xperms, xperms);
/* failure on the extended permission check_extended_permissions */
if (rc) {
char *permstring;

extended_permissions_violated(&error, avrule->xperms, xperms);
permstring = sepol_extended_perms_to_string(&error);

ERR(handle, "neverallowxperm on line %lu of %s (or line %lu of %s) violated by\n"
"allowxperm %s %s:%s %s;",
avrule->source_line, avrule->source_filename, avrule->line, policy_name(p),
p->p_type_val_to_name[i],
p->p_type_val_to_name[j],
p->p_class_val_to_name[curperm->tclass - 1],
sepol_extended_perms_to_string(&error));
permstring ?: "<format-failure>");

free(permstring);
errors++;
}
}
Expand Down
9 changes: 5 additions & 4 deletions libsepol/src/kernel_to_conf.c
Original file line number Diff line number Diff line change
Expand Up @@ -1683,7 +1683,7 @@ static char *avtab_node_to_str(struct policydb *pdb, avtab_key_t *key, avtab_dat
uint32_t data = datum->data;
type_datum_t *type;
const char *flavor, *src, *tgt, *class, *perms, *new;
char *rule = NULL;
char *rule = NULL, *permstring;

switch (0xFFF & key->specified) {
case AVTAB_ALLOWED:
Expand Down Expand Up @@ -1738,13 +1738,14 @@ static char *avtab_node_to_str(struct policydb *pdb, avtab_key_t *key, avtab_dat
rule = create_str("%s %s %s:%s { %s };", 5,
flavor, src, tgt, class, perms+1);
} else if (key->specified & AVTAB_XPERMS) {
perms = sepol_extended_perms_to_string(datum->xperms);
if (perms == NULL) {
permstring = sepol_extended_perms_to_string(datum->xperms);
if (permstring == NULL) {
ERR(NULL, "Failed to generate extended permission string");
goto exit;
}

rule = create_str("%s %s %s:%s %s;", 5, flavor, src, tgt, class, perms);
rule = create_str("%s %s %s:%s %s;", 5, flavor, src, tgt, class, permstring);
free(permstring);
} else {
new = pdb->p_type_val_to_name[data - 1];

Expand Down
57 changes: 38 additions & 19 deletions libsepol/src/util.c
Original file line number Diff line number Diff line change
Expand Up @@ -132,21 +132,32 @@ char *sepol_extended_perms_to_string(avtab_extended_perms_t *xperms)
uint16_t low_bit;
uint16_t low_value;
unsigned int bit;
unsigned int in_range = 0;
static char xpermsbuf[2048];
char *p;
int len, xpermslen = 0;
xpermsbuf[0] = '\0';
p = xpermsbuf;
unsigned int in_range;
char *buffer = NULL, *p;
int len;
size_t remaining, size = 128;

if ((xperms->specified != AVTAB_XPERMS_IOCTLFUNCTION)
&& (xperms->specified != AVTAB_XPERMS_IOCTLDRIVER))
return NULL;

len = snprintf(p, sizeof(xpermsbuf) - xpermslen, "ioctl { ");
retry:
size *= 2;
if (size == 0)
goto err;
p = realloc(buffer, size);
if (!p)
goto err;
buffer = p;
remaining = size;

len = snprintf(p, remaining, "ioctl { ");
if (len < 0 || (size_t)len >= remaining)
goto err;
p += len;
xpermslen += len;
remaining -= len;

in_range = 0;
for (bit = 0; bit < sizeof(xperms->perms)*8; bit++) {
if (!xperm_test(bit, xperms->perms))
continue;
Expand All @@ -165,35 +176,43 @@ char *sepol_extended_perms_to_string(avtab_extended_perms_t *xperms)
value = xperms->driver<<8 | bit;
if (in_range) {
low_value = xperms->driver<<8 | low_bit;
len = snprintf(p, sizeof(xpermsbuf) - xpermslen, "0x%hx-0x%hx ", low_value, value);
len = snprintf(p, remaining, "0x%hx-0x%hx ", low_value, value);
} else {
len = snprintf(p, sizeof(xpermsbuf) - xpermslen, "0x%hx ", value);
len = snprintf(p, remaining, "0x%hx ", value);
}
} else if (xperms->specified & AVTAB_XPERMS_IOCTLDRIVER) {
value = bit << 8;
if (in_range) {
low_value = low_bit << 8;
len = snprintf(p, sizeof(xpermsbuf) - xpermslen, "0x%hx-0x%hx ", low_value, (uint16_t) (value|0xff));
len = snprintf(p, remaining, "0x%hx-0x%hx ", low_value, (uint16_t) (value|0xff));
} else {
len = snprintf(p, sizeof(xpermsbuf) - xpermslen, "0x%hx-0x%hx ", value, (uint16_t) (value|0xff));
len = snprintf(p, remaining, "0x%hx-0x%hx ", value, (uint16_t) (value|0xff));
}

}

if (len < 0 || (size_t) len >= (sizeof(xpermsbuf) - xpermslen))
return NULL;
if (len < 0)
goto err;
if ((size_t) len >= remaining)
goto retry;

p += len;
xpermslen += len;
remaining -= len;
if (in_range)
in_range = 0;
}

len = snprintf(p, sizeof(xpermsbuf) - xpermslen, "}");
if (len < 0 || (size_t) len >= (sizeof(xpermsbuf) - xpermslen))
return NULL;
len = snprintf(p, remaining, "}");
if (len < 0)
goto err;
if ((size_t) len >= remaining)
goto retry;

return buffer;

return xpermsbuf;
err:
free(buffer);
return NULL;
}

/*
Expand Down

0 comments on commit fdb536f

Please sign in to comment.