On Mon, Nov 20, 2023 at 1:12 PM James Carter <jwcart2@xxxxxxxxx> wrote:
>
> On Mon, Nov 20, 2023 at 10:48 AM Christian Göttsche
> <cgzones@xxxxxxxxxxxxxx> wrote:
> >
> > 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@xxxxxxxxxxxxxx>
>
> For these two patches:
> Acked-by: James Carter <jwcart2@xxxxxxxxx>
>
These two patches have been merged.
Thanks,
Jim
> > ---
> > v2:
> > reset in_range on retry
> > Signed-off-by: Christian Göttsche <cgzones@xxxxxxxxxxxxxx>
> > ---
> > checkpolicy/test/dismod.c | 9 +++++-
> > checkpolicy/test/dispol.c | 10 +++++-
> > libsepol/src/assertion.c | 7 ++++-
> > libsepol/src/kernel_to_conf.c | 9 +++---
> > libsepol/src/util.c | 57 +++++++++++++++++++++++------------
> > 5 files changed, 66 insertions(+), 26 deletions(-)
> >
> > diff --git a/checkpolicy/test/dismod.c b/checkpolicy/test/dismod.c
> > index fa7117f5..9f4a669b 100644
> > --- a/checkpolicy/test/dismod.c
> > +++ b/checkpolicy/test/dismod.c
> > @@ -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)
> > @@ -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");
> > diff --git a/checkpolicy/test/dispol.c b/checkpolicy/test/dispol.c
> > index b567ce77..944ef7ec 100644
> > --- a/checkpolicy/test/dispol.c
> > +++ b/checkpolicy/test/dispol.c
> > @@ -196,6 +196,8 @@ 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)
> > @@ -203,7 +205,13 @@ static int render_av_rule(avtab_key_t * key, avtab_datum_t * datum, uint32_t wha
> > 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;
> > diff --git a/libsepol/src/assertion.c b/libsepol/src/assertion.c
> > index b6ac4cfe..6de7d031 100644
> > --- a/libsepol/src/assertion.c
> > +++ b/libsepol/src/assertion.c
> > @@ -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++;
> > }
> > }
> > diff --git a/libsepol/src/kernel_to_conf.c b/libsepol/src/kernel_to_conf.c
> > index b0ae16d9..b5b530d6 100644
> > --- a/libsepol/src/kernel_to_conf.c
> > +++ b/libsepol/src/kernel_to_conf.c
> > @@ -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:
> > @@ -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];
> >
> > diff --git a/libsepol/src/util.c b/libsepol/src/util.c
> > index 0a2edc85..2f877920 100644
> > --- a/libsepol/src/util.c
> > +++ b/libsepol/src/util.c
> > @@ -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;
> > @@ -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;
> > }
> >
> > /*
> > --
> > 2.42.0
> >
