From fdb536f38de2a891a49fd015ccbf46a08f6c680c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Mon, 20 Nov 2023 16:47:35 +0100 Subject: [PATCH] libsepol: avoid fixed sized format buffer for xperms MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Acked-by: James Carter --- 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 fa7117f582..9f4a669b23 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 b567ce7754..944ef7ecc1 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 b6ac4cfe94..6de7d0315a 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 ?: ""); + free(permstring); errors++; } } diff --git a/libsepol/src/kernel_to_conf.c b/libsepol/src/kernel_to_conf.c index b0ae16d97c..b5b530d6e6 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 0a2edc852b..2f87792016 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; } /*